Skip to content
This repository has been archived by the owner on Nov 11, 2024. It is now read-only.

Commit

Permalink
Align straggler keepGoingCallback() in u_device_private_cell.c to the…
Browse files Browse the repository at this point in the history 
… new pattern, align distance/time calculation in u_geofence.c, move in-line functions to be defined in a header file (u_port.h) in order that the compiler can in-line then (rather than the linker being left to do so) and add `Ms` to the end of the function names to be clear on the units of the parameters.
  • Loading branch information
@RobMeades
RobMeades committed Mar 28, 2024
1 parent 45d3be1 commit 3cf5348
Show file tree
Hide file tree
Showing 70 changed files with 222 additions and 288 deletions.
2 changes: 1 addition & 1 deletion DEVELOPMENT_PRINCIPLES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ As an aside, if `master` moves on underneath a branch **THAT YOU ALONE** are wor
# Beware The Wrap
Embedded systems usually have no better than a 32 bit millisecond tick count, i.e. a signed 32 bit counter that will wrap at `INT32_MAX`, so modulo 2^31, or 2,147,483,648, or about 25 days; the return value of the port layer `uPortGetTickTimeMs()` is an `int32_t` for this reason.

The systems that `ubxlib` is built into will need to be up for longer than 25 days, so the `ubxlib` code must behave well around such a wrap and, specifically, not unintentionally become stuck for 25 days if the tick counter happens to wrap while the code is waiting on it. Always use the `uPortTickTimeExpired()` or `uPortTickTimeBeyondStop()` functions (see [u_port.h](/port/api/u_port.h)) while waiting for a number of ticks to pass; these are designed to ensure that nothing will get stuck.
The systems that `ubxlib` is built into will need to be up for longer than 25 days, so the `ubxlib` code must behave well around such a wrap and, specifically, not unintentionally become stuck for 25 days if the tick counter happens to wrap while the code is waiting on it. Always use the `uPortTickTimeExpiredMs()` or `uPortTickTimeBeyondStopMs()` functions (see [u_port.h](/port/api/u_port.h)) while waiting for a number of ticks to pass; these are designed to ensure that nothing will get stuck.

# Be Explicit About Units
Where a number represents a quantity it will have a unit: seconds, milliseconds, nanoseconds, Volts, milliVolts, decibels, decibels relative to one milliWatt (dBm), words (as opposed to bytes), sheep, etc. You may recall the tale of the [Mars Climate Orbiter](https://en.wikipedia.org/wiki/Mars_Climate_Orbiter), a $327 million spacecraft that was lost because the NASA navigation software expected measurements in newton-seconds while their contractor was providing measurements in pound-force seconds, a factor of 4.5 different; where a number represents a quantity, **be explicit** about the unit by including it in the variable/parameter name. For instance, presented with a variable/parameter named `timeout`, you could get things wrong by three orders of magnitude or more when applying that parameter, without realising it; naming it something like `timeoutMs` or `timeoutSeconds` will make the intended usage clear.
2 changes: 1 addition & 1 deletion ble/src/u_ble_sps_intmod.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1240,7 +1240,7 @@ int32_t uBleSpsSend(uDeviceHandle_t devHandle, int32_t channel, const char *pDat
int32_t time = startTime;

// Note: this loop is constructed slightly differently to usual
// and so can't use uPortTickTimeExpired() but it
// and so can't use uPortTickTimeExpiredMs() but it
// _does_ perform tick time comparisons in a wrap-safe manner
while ((bytesLeftToSend > 0) && (time - startTimeMs < timeout)) {
int32_t bytesToSendNow = bytesLeftToSend;
Expand Down
2 changes: 1 addition & 1 deletion cell/src/u_cell_loc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1780,7 +1780,7 @@ int32_t uCellLocGet(uDeviceHandle_t cellHandle,
startTimeMs = uPortGetTickTimeMs();
while ((fixDataStorageBlock.errorCode == (int32_t) U_ERROR_COMMON_TIMEOUT) &&
(((pKeepGoingCallback == NULL) &&
!uPortTickTimeExpired(startTimeMs, U_CELL_LOC_TIMEOUT_SECONDS * 1000)) ||
!uPortTickTimeExpiredMs(startTimeMs, U_CELL_LOC_TIMEOUT_SECONDS * 1000)) ||
((pKeepGoingCallback != NULL) && pKeepGoingCallback(cellHandle)))) {
// Relax a little
uPortTaskBlock(1000);
Expand Down
40 changes: 20 additions & 20 deletions cell/src/u_cell_mqtt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1100,8 +1100,8 @@ static int32_t doSaraR4OldSyntaxUmqttQuery(const uCellPrivateInstance_t *pInstan
// and don't bother with keepGoingCallback
startTimeMs = uPortGetTickTimeMs();
while ((!checkUrcStatusField(pUrcStatus, number)) &&
!uPortTickTimeExpired(startTimeMs,
U_CELL_MQTT_LOCAL_URC_TIMEOUT_MS)) {
!uPortTickTimeExpiredMs(startTimeMs,
U_CELL_MQTT_LOCAL_URC_TIMEOUT_MS)) {
uPortTaskBlock(250);
}
if (checkUrcStatusField(pUrcStatus, number)) {
Expand Down Expand Up @@ -1347,8 +1347,8 @@ static int32_t connect(const uCellPrivateInstance_t *pInstance,
// take a little while to find out that the connection
// has actually been made and hence we wait here for
// it to be ready to connect
while (!uPortTickTimeExpired(pInstance->connectedAtMs,
U_CELL_MQTT_CONNECT_DELAY_MILLISECONDS)) {
while (!uPortTickTimeExpiredMs(pInstance->connectedAtMs,
U_CELL_MQTT_CONNECT_DELAY_MILLISECONDS)) {
uPortTaskBlock(100);
}
}
Expand Down Expand Up @@ -1401,8 +1401,8 @@ static int32_t connect(const uCellPrivateInstance_t *pInstance,
errorCode = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_CONNECT_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -1702,8 +1702,8 @@ static int32_t publish(const uCellPrivateInstance_t *pInstance,
errorCode = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_PUBLISH_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -1809,8 +1809,8 @@ static int32_t subscribe(const uCellPrivateInstance_t *pInstance,
errorCodeOrQos = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_SUBSCRIBE_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -1898,8 +1898,8 @@ static int32_t unsubscribe(const uCellPrivateInstance_t *pInstance,
errorCode = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_UNSUBSCRIBE_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -1988,8 +1988,8 @@ static int32_t readMessage(const uCellPrivateInstance_t *pInstance,
errorCode = (int32_t) U_ERROR_COMMON_EMPTY;
startTimeMs = uPortGetTickTimeMs();
while (!pUrcMessage->messageRead &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -3389,8 +3389,8 @@ int32_t uCellMqttSnRegisterNormalTopic(uDeviceHandle_t cellHandle,
errorCode = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_REGISTER_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -3659,8 +3659,8 @@ int32_t uCellMqttSnSetWillMessaage(uDeviceHandle_t cellHandle,
errorCode = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_WILL_MESSAGE_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down Expand Up @@ -3735,8 +3735,8 @@ int32_t uCellMqttSnSetWillParameters(uDeviceHandle_t cellHandle,
errorCode = (int32_t) U_ERROR_COMMON_TIMEOUT;
startTimeMs = uPortGetTickTimeMs();
while (((pUrcStatus->flagsBitmap & (1 << U_CELL_MQTT_URC_FLAG_WILL_PARAMETERS_UPDATED)) == 0) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down
12 changes: 6 additions & 6 deletions cell/src/u_cell_mux.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ static int32_t sendEvent(uCellMuxPrivateContext_t *pContext,
uPortTaskBlock(U_CFG_OS_YIELD_MS);
irqSupported = (errorCode != (int32_t) U_ERROR_COMMON_NOT_IMPLEMENTED) &&
(errorCode != (int32_t) U_ERROR_COMMON_NOT_SUPPORTED);
} while (irqSupported && !uPortTickTimeExpired(startTimeMs, delayMs));
} while (irqSupported && !uPortTickTimeExpiredMs(startTimeMs, delayMs));

if (!irqSupported) {
// If IRQ is not supported, just gotta do the normal send
Expand Down Expand Up @@ -380,8 +380,8 @@ static int32_t serialWriteInnards(struct uDeviceSerial_t *pDeviceSerial,
}
startTimeMs = uPortGetTickTimeMs();
while ((sizeWritten < sizeBytes) && (sizeOrErrorCode >= 0) &&
!uPortTickTimeExpired(startTimeMs,
U_CELL_MUX_WRITE_TIMEOUT_MS)) {
!uPortTickTimeExpiredMs(startTimeMs,
U_CELL_MUX_WRITE_TIMEOUT_MS)) {
// Encode a chunk as UIH
thisChunkSize = sizeBytes - sizeWritten;
if (thisChunkSize > U_CELL_MUX_PRIVATE_INFORMATION_LENGTH_MAX_BYTES) {
Expand All @@ -394,8 +394,8 @@ static int32_t serialWriteInnards(struct uDeviceSerial_t *pDeviceSerial,
if (sizeOrErrorCode >= 0) {
lengthWritten = 0;
while ((sizeOrErrorCode >= 0) && (lengthWritten < (size_t) sizeOrErrorCode) &&
!uPortTickTimeExpired(startTimeMs,
U_CELL_MUX_WRITE_TIMEOUT_MS)) {
!uPortTickTimeExpiredMs(startTimeMs,
U_CELL_MUX_WRITE_TIMEOUT_MS)) {
if (!pChannelContext->traffic.txIsFlowControlledOff) {
// Send the data
thisLengthWritten = uPortUartWrite(pChannelContext->pContext->underlyingStreamHandle,
Expand Down Expand Up @@ -515,7 +515,7 @@ static int32_t sendCommandCheckResponse(uDeviceSerial_t *pDeviceSerial,
// Wait for a response
startTimeMs = uPortGetTickTimeMs();
while ((pTraffic->wantedResponseFrameType != U_CELL_MUX_PRIVATE_FRAME_TYPE_NONE) &&
!uPortTickTimeExpired(startTimeMs, timeoutMs)) {
!uPortTickTimeExpiredMs(startTimeMs, timeoutMs)) {
uPortTaskBlock(10);
}
if (pTraffic->wantedResponseFrameType == U_CELL_MUX_PRIVATE_FRAME_TYPE_NONE) {
Expand Down
28 changes: 14 additions & 14 deletions cell/src/u_cell_net.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,8 +749,8 @@ static bool keepGoingLocalCb(const uCellPrivateInstance_t *pInstance)
keepGoing = pInstance->pKeepGoingCallback(pInstance->cellHandle);
} else {
if ((pInstance->startTimeMs > 0) &&
!uPortTickTimeExpired(pInstance->startTimeMs,
U_CELL_NET_CONNECT_TIMEOUT_SECONDS * 1000)) {
!uPortTickTimeExpiredMs(pInstance->startTimeMs,
U_CELL_NET_CONNECT_TIMEOUT_SECONDS * 1000)) {
keepGoing = false;
}
}
Expand All @@ -770,8 +770,8 @@ static int32_t radioOff(uCellPrivateInstance_t *pInstance)
pInstance->profileState = U_CELL_PRIVATE_PROFILE_STATE_SHOULD_BE_DOWN;
for (size_t x = 3; (x > 0) && (errorCode < 0); x--) {
// Wait for flip time to expire
while (!uPortTickTimeExpired(pInstance->lastCfunFlipTimeMs,
U_CELL_PRIVATE_AT_CFUN_FLIP_DELAY_SECONDS * 1000)) {
while (!uPortTickTimeExpiredMs(pInstance->lastCfunFlipTimeMs,
U_CELL_PRIVATE_AT_CFUN_FLIP_DELAY_SECONDS * 1000)) {
uPortTaskBlock(1000);
}
uAtClientLock(atHandle);
Expand Down Expand Up @@ -1242,8 +1242,8 @@ static int32_t registerNetwork(uCellPrivateInstance_t *pInstance,

// Come out of airplane mode and try to register
// Wait for flip time to expire first though
while (!uPortTickTimeExpired(pInstance->lastCfunFlipTimeMs,
U_CELL_PRIVATE_AT_CFUN_FLIP_DELAY_SECONDS * 1000)) {
while (!uPortTickTimeExpiredMs(pInstance->lastCfunFlipTimeMs,
U_CELL_PRIVATE_AT_CFUN_FLIP_DELAY_SECONDS * 1000)) {
uPortTaskBlock(1000);
}
// Reset the current registration status
Expand Down Expand Up @@ -1804,8 +1804,8 @@ static int32_t activateContextUpsd(const uCellPrivateInstance_t *pInstance,
deviceError.type = U_AT_CLIENT_DEVICE_ERROR_TYPE_NO_ERROR;
while (!activated && keepGoingLocalCb(pInstance) &&
(deviceError.type == U_AT_CLIENT_DEVICE_ERROR_TYPE_NO_ERROR) &&
!uPortTickTimeExpired(startTimeMs,
U_CELL_NET_UPSD_CONTEXT_ACTIVATION_TIME_SECONDS * 1000)) {
!uPortTickTimeExpiredMs(startTimeMs,
U_CELL_NET_UPSD_CONTEXT_ACTIVATION_TIME_SECONDS * 1000)) {
uAtClientClearError(atHandle);
uAtClientResponseStart(atHandle, NULL);
activated = (uAtClientErrorGet(atHandle) == 0);
Expand Down Expand Up @@ -2990,8 +2990,8 @@ int32_t uCellNetScanGetFirst(uDeviceHandle_t cellHandle,
bytesRead = -1;
innerStartTimeMs = uPortGetTickTimeMs();
while ((bytesRead <= 0) &&
!uPortTickTimeExpired(innerStartTimeMs,
U_CELL_NET_SCAN_TIME_SECONDS * 1000) &&
!uPortTickTimeExpiredMs(innerStartTimeMs,
U_CELL_NET_SCAN_TIME_SECONDS * 1000) &&
((pKeepGoingCallback == NULL) || (pKeepGoingCallback(cellHandle)))) {
uAtClientResponseStart(atHandle, "+COPS:");
// We use uAtClientReadBytes() here because the
Expand Down Expand Up @@ -3148,8 +3148,8 @@ int32_t uCellNetDeepScan(uDeviceHandle_t cellHandle,
startTimeMs = uPortGetTickTimeMs();
for (size_t x = U_CELL_NET_DEEP_SCAN_RETRIES + 1;
(x > 0) && (errorCodeOrNumber < 0) && keepGoing &&
!uPortTickTimeExpired(startTimeMs,
U_CELL_NET_DEEP_SCAN_TIME_SECONDS * 1000);
!uPortTickTimeExpiredMs(startTimeMs,
U_CELL_NET_DEEP_SCAN_TIME_SECONDS * 1000);
x--) {
number = 0;
errorCodeOrNumber = (int32_t) U_ERROR_COMMON_TIMEOUT;
Expand All @@ -3166,8 +3166,8 @@ int32_t uCellNetDeepScan(uDeviceHandle_t cellHandle,
// want to be able to stop the command part way through,
// hence the AT handling code below is more complex than usual.
while ((errorCodeOrNumber == (int32_t) U_ERROR_COMMON_TIMEOUT) && keepGoing &&
!uPortTickTimeExpired(startTimeMs,
U_CELL_NET_DEEP_SCAN_TIME_SECONDS * 1000)) {
!uPortTickTimeExpiredMs(startTimeMs,
U_CELL_NET_DEEP_SCAN_TIME_SECONDS * 1000)) {
if (uAtClientResponseStart(atHandle, NULL) == 0) {
// See if we have a line
errorCodeOrNumber = parseDeepScanLine(atHandle, &cell);
Expand Down
Loading

0 comments on commit 3cf5348

Please sign in to comment.