Cell Network Up Keep Going Callback Fails at Tick Wrap-Around

[warasilapm]

Problem Description

The root of this issue seems to be that the stop time is stored as a 64-bit integer in the cell context.

ubxlib/common/device/src/u_device_shared_cell.h

Lines 46 to 51 in 05f4af4

	typedef struct {
	int32_t uart;
	uAtClientHandle_t at;
	int64_t stopTimeMs;
	int32_t pinPwrOn;
	} uDeviceCellContext_t;

When the stop time is calculated, if the tick is within the timeout window of INT32_MAX (by default, 60 seconds), the stop time will be set beyond the range of int32_t.

ubxlib/common/network/src/u_network_private_cell.c

Lines 176 to 177 in 05f4af4

	pContext->stopTimeMs = uPortGetTickTimeMs() +
	(((int64_t) timeoutSeconds) * 1000);

Additionally, even if it were wrapped around correctly, this buggy check in the default keep going callback means that it would immediately time out.

ubxlib/common/network/src/u_network_private_cell.c

Lines 77 to 92 in 05f4af4

	static bool keepGoingCallback(uDeviceHandle_t devHandle)
	{
	uDeviceCellContext_t *pContext;
	uDeviceInstance_t *pDevInstance = NULL;
	bool keepGoing = false;
	if (uDeviceGetInstance(devHandle, &pDevInstance) == 0) {
	pContext = (uDeviceCellContext_t *) pDevInstance->pContext;
	if ((pContext == NULL) ||
	(uPortGetTickTimeMs() < pContext->stopTimeMs)) {
	keepGoing = true;
	}
	}
	return keepGoing;
	}

Proposed Solution

1. Change the stop time value to use int32_t to match the portable API.
2. Fix the check used in the keep going callback to calculate a difference.

See attached example patch

[RobMeades]

Hi, and thanks for spotting this; your proposed fix looks good. We will integrate it but, unfortunately, at the moment the cellular side of our test system is down, as the equipment we use as our cellular test network lost its hard disk at the weekend.

Once we have put the pieces back together again, so that we are able to regression test cellular stuff, we will get the change tested/reviewed and pushed to master here. I will update this issue when that occurs.

[Also talk later I understand].

[RobMeades]

The fix for his issue is included in the rather larger fix of commit 79e808e.