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

Commit

Permalink
In-line functions replace the macros.
Browse files Browse the repository at this point in the history
  • Loading branch information
RobMeades committed Mar 15, 2024
1 parent 2e464d8 commit 239c5b9
Show file tree
Hide file tree
Showing 70 changed files with 371 additions and 262 deletions.
18 changes: 14 additions & 4 deletions DEVELOPMENT_PRINCIPLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,20 @@ Since the commit messages are our only change documentation, and since Github is

- when merging a PR, try to stick to squash-merges or rebase-merges, rather than plain-old merges of a branch,
- if a customer makes a PR to the public `ubxlib` repo, bring it in as follows:
- make sure that the customer PR is a single commit (ask them to squash it and re-push if it is not),
- if the customer PR is NEITHER (a) a single commit, NOR (b) made up of nice discrete/sensible changes that would make sense to any other customer, then ask them to squash it into a nice clean single commit and re-push,
- pull the PR into a branch of `ubxlib_priv` so that you can throw it at the test system to prove that it is all good,
- make sure that `ubxlib` `master` is up to date with `ubxlib_priv` `master` (i.e. push `ubxlib_priv` `master` to `ubxlib` `master`, which should always be possible, see above),
- do a rebase-merge of the customer PR into `ubxlib` `master` (i.e. directly, not going via `ubxlib_priv`),
- pull `ubxlib` `master` back into `ubxlib_priv` `master` (i.e. with the latest `ubxlib_priv` `master` on your machine, pull `ubxlib` `master` and then push that to `ubxlib_priv` `master`).

The only exception to the above is when there has been active work on the `ubxlib_priv` `development` branch and that is ready to be brought into `master`: this should be brought into `ubxlib_priv` `master` through a normal (i.e. non-rebase, non-squash) merge since it will likely be a _very_ large commit of disparate things that will not be describable when in one big blob.
- pull `ubxlib` `master` back into `ubxlib_priv` `master` (i.e. with the latest `ubxlib_priv` `master` checked-out on your machine, pull `ubxlib` `master` and then push that to `ubxlib_priv` `master`).

The only exception to the above is when there has been active work on the `ubxlib_priv` `development` branch and that is ready to be brought into `master`: this should be brought into `ubxlib_priv` `master` through a normal (i.e. non-rebase, non-squash) merge since it will likely be a _very_ large commit of disparate things that will not be describable when in one big blob.

As an aside, if `master` moves on underneath a branch **THAT YOU ALONE** are working on, please do a `rebase` of that development branch onto `master`, rather then merging the changes from `master` onto your branch, (i.e. checkout `master` locally, pull the latest `master` and then `rebase` your branch onto `master`); the reason for this is that, otherwise, the merge process can be confused and end up thinking that you intend to remove things that have just been added in the `master` branch. If you share the branch with someone else, i.e. you are not working on it alone, then take care because rebasing obviously changes history; it may still be the right thing to do, 'cos the ground has indeed moved underneath you, history _has_ changed, but make sure that anyone else who is working on the branch with you is aware of what you have done when you push the branch back to the repo.

# 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.

# 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.
5 changes: 4 additions & 1 deletion ble/src/u_ble_sps_intmod.c
Original file line number Diff line number Diff line change
Expand Up @@ -1239,7 +1239,10 @@ int32_t uBleSpsSend(uDeviceHandle_t devHandle, int32_t channel, const char *pDat
uint32_t timeout = pSpsConn->dataSendTimeoutMs;
int32_t time = startTime;

while ((bytesLeftToSend > 0) && (time - startTime < timeout)) {
// Note: this loop is constructed slightly differently to usual
// and so can't use uPortTickTimeExpired() but it
// _does_ perform tick time comparisons in a wrap-safe manner
while ((bytesLeftToSend > 0) && (time - startTimeMs < timeout)) {
int32_t bytesToSendNow = bytesLeftToSend;
int32_t maxDataLength = pSpsConn->mtu - U_BLE_PDU_HEADER_SIZE;

Expand Down
3 changes: 3 additions & 0 deletions cell/src/u_cell.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

#include "u_error_common.h"

#include "u_port.h"
#include "u_port_debug.h"
#include "u_port_os.h"
#include "u_port_heap.h"
Expand Down Expand Up @@ -297,6 +298,8 @@ int32_t uCellAdd(uCellModuleType_t moduleType,
pInstance->pinPwrOn = pinPwrOn;
pInstance->pinVInt = pinVInt;
pInstance->pinDtrPowerSaving = -1;
pInstance->lastCfunFlipTimeMs = uPortGetTickTimeMs();
pInstance->lastDtrPinToggleTimeMs = uPortGetTickTimeMs();
for (size_t x = 0;
x < sizeof(pInstance->networkStatus) / sizeof(pInstance->networkStatus[0]);
x++) {
Expand Down
2 changes: 1 addition & 1 deletion cell/src/u_cell_loc.c
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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs, U_CELL_LOC_TIMEOUT_SECONDS * 1000)) ||
!uPortTickTimeExpired(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
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)) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_CELL_MQTT_LOCAL_URC_TIMEOUT_MS)) {
!uPortTickTimeExpired(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 (!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(pInstance->connectedAtMs,
U_CELL_MQTT_CONNECT_DELAY_MILLISECONDS)) {
while (!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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 &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
!uPortTickTimeExpired(startTimeMs,
U_MQTT_CLIENT_RESPONSE_WAIT_SECONDS * 1000) &&
((pContext->pKeepGoingCallback == NULL) ||
pContext->pKeepGoingCallback())) {
uPortTaskBlock(1000);
Expand Down
8 changes: 4 additions & 4 deletions cell/src/u_cell_mux.c
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 && !U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs, delayMs));
} while (irqSupported && !uPortTickTimeExpired(startTimeMs, delayMs));

if (!irqSupported) {
// If IRQ is not supported, just gotta do the normal send
Expand Down Expand Up @@ -380,7 +380,7 @@ static int32_t serialWriteInnards(struct uDeviceSerial_t *pDeviceSerial,
}
startTimeMs = uPortGetTickTimeMs();
while ((sizeWritten < sizeBytes) && (sizeOrErrorCode >= 0) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
!uPortTickTimeExpired(startTimeMs,
U_CELL_MUX_WRITE_TIMEOUT_MS)) {
// Encode a chunk as UIH
thisChunkSize = sizeBytes - sizeWritten;
Expand All @@ -394,7 +394,7 @@ static int32_t serialWriteInnards(struct uDeviceSerial_t *pDeviceSerial,
if (sizeOrErrorCode >= 0) {
lengthWritten = 0;
while ((sizeOrErrorCode >= 0) && (lengthWritten < (size_t) sizeOrErrorCode) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs,
!uPortTickTimeExpired(startTimeMs,
U_CELL_MUX_WRITE_TIMEOUT_MS)) {
if (!pChannelContext->traffic.txIsFlowControlledOff) {
// Send the data
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) &&
!U_PORT_TICK_TIME_EXPIRED_OR_WRAP_MS(startTimeMs, timeoutMs)) {
!uPortTickTimeExpired(startTimeMs, timeoutMs)) {
uPortTaskBlock(10);
}
if (pTraffic->wantedResponseFrameType == U_CELL_MUX_PRIVATE_FRAME_TYPE_NONE) {
Expand Down
Loading

0 comments on commit 239c5b9

Please sign in to comment.