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

Refresh radio parameters delay #235

Closed eeFLis closed 1 month ago

eeFLis commented 1 month ago

Hi there

Wouldn't it make sense to just add a delay with the delta to the last function call instead of 500ms each time? https://github.com/u-blox/ubxlib/blob/7f70e9073ffdc7b23374a892682d7166af233f4b/cell/src/u_cell_info.c#L686C1-L686C41

RobMeades commented 1 month ago

Hi eeFLis: having discussed this with @philwareublox, suggestion is that we make the delay a conditionally-compiled value, implemented as a #ifndef/#define/#endif kinda thing and add a note to:

https://github.com/u-blox/ubxlib/blob/7f70e9073ffdc7b23374a892682d7166af233f4b/cell/api/u_cell_info.h#L69-L79

...pointing it out and suggesting that an application where power/timing is important (your case) might override it to 0 and manage any timing in the application instead.

Would that work?

eeFLis commented 1 month ago

Yes, that works for us. Out of curiosity, do you see a problem if the function is called in a loop without the delay? As the response from the module is awaited, the module should not be overtasked.

RobMeades commented 1 month ago

Good, change should be here in an hour or two.

Out of curiosity, do you see a problem if the function is called in a loop without the delay?

I added that delay because I found in the past that a module didn't like being hit with the radio refresh-related commands frequently. However, I can't recall now which module that was and it may, of course, be something that has since been fixed, hence my [unhelpful] answer is "no, unless you see one" :-).

RobMeades commented 1 month ago

Fix can be found in 87a0a2c07b0ca85e08a209215a38bc70bf219cd4, will close this now: please feel free to re-open if there is more to discuss,