u-blox / ubxlib

Portable C libraries which provide APIs to build applications with u-blox products and services. Delivered as add-on to existing microcontroller and RTOS SDKs.
Apache License 2.0
287 stars 82 forks source link

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

Closed warasilapm closed 2 months ago

warasilapm commented 4 months ago

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.

https://github.com/u-blox/ubxlib/blob/05f4af4287e9c80f45e5272ca2e3d4bafb4a66ad/common/device/src/u_device_shared_cell.h#L46-L51

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.

https://github.com/u-blox/ubxlib/blob/05f4af4287e9c80f45e5272ca2e3d4bafb4a66ad/common/network/src/u_network_private_cell.c#L176-L177

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

https://github.com/u-blox/ubxlib/blob/05f4af4287e9c80f45e5272ca2e3d4bafb4a66ad/common/network/src/u_network_private_cell.c#L77-L92

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 commented 4 months ago

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 commented 2 months ago

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