Current OT only support multiple milliseconds timer, and a single microseconds timer used by CSMA backoff (only enabled by nRF52840 now).
To support the coming new features, we need multiple microseconds timer, the change proposal:
Platform alarm APIs update
class Timer update
Option 1:
uint32_t mFireTime field with the structure otPlatUsecAlarmTimeTimer. Option 2:
class TimerUsec based on class Timer, with an extra microseconds field.class TimerScheduler to handle the two timers (two queues, time comparison between them), it will increase some code size.I prefer option 1 more, please let me know your preference and other better options.
For background, we've had significant discussion around this topic before in #1267. Note that #1267 implemented Option 2 above. However, the discussion led to #1379 being implemented instead. Please review that discussion and describe why something like #1267 is preferred now.
The implementation in #1267 is more like the Option 2: create a new class TimerUsec and handle the two different timers in TimerScheduler.
The option 1 above is different, it will convert all class Timer to microseconds timer, so there is no new class, just increase 2 bytes (RAM space) to each Timer (totally ~50 bytes), but the code is more concise and less code size than Option 2.
Regarding to the discussion in #1267, I think there are two different factors now:
Timer, instead of 4 bytes that mentioned in #1267, since the class Timer has been refined in #1904.That's the reason I prefer Option 1 now, while I'm also agree to reopen #1267 (Option 2) if we consider RAM size is more critical than flash size.
Due to padding, I believe adding a single uint16_t to the Timer class will actually add 4 bytes to each instance. On the default CLI build for CC2538, that adds 112 bytes RAM (flash overhead difficult to know without some additional prototyping). For comparison, the change in #1267 adds 16 bytes RAM and 1072 bytes flash.
A separate question to ask: Does your use cases for microsecond timer really require a (32-bit millisecond, 16-bit microsecond) representation? For example, simplifying to a single 32-bit microsecond representation should give you over an hour before roll-over.
A slight variation/view on Option 2 is to just have logically different Timer schedulers (one for millisecond another for microsecond) both using a single 32-bit integer representation. If a single 32-bit microsecond representation is sufficient for your use cases, then I think this approach may be preferable because you will not have any increase in RAM overhead. It also makes clear what timer type is really required by the user, making it obvious which services are affected if a microsecond timer is not provided by a given platform.
A possible 3rd option (which I personally prefer)
Creating a new UsecTimer on its own (not sub-classing Timer) and have it use its own linked-list and basically its own separate UsecTimerScheduelr to manage only the TimerUsec instances.
This gives us flexibility to use TimerUsec where we need it and also not complicate the logic of comparing timers of different accuracy in a single TimerShceduler.
Thoughts?
@abtink, see my comments above :)
Thanks @jwhui for the suggestion.
Below is the updated solution:
class Timer for both milliseconds timer and microseconds timer;UsecTimerScheduler to handle microsecond timers, and call into usec-alarm API.I have submitted #1962 to support multiple microseconds timer, it adds 320 bytes flash and 32bytes RAM.
I made some changes to the previous solution, and follow @abtink's suggestion to add separate UsecTimer and UsecTimerScheduler, because of that:
Since #1926, to reduce RAM usage in single Instance mode, the TimerSchedulerLocator does not store the TimerScheduler initialized in Timer's constructor, which makes it difficult to reuse class Timer for both Msec and Usec Timer without adding the mScheduler back (will add 112 bytes RAM).
Another possible solution I proposed in #1954 is adding a parameter aTimerScheduler to Timer::Start() and Timer::Stop() function, but the change will add ~1k bytes flash. So finally I think that adding separate UsecTimer and UsecTimerScheduler is the most efficient way.
Most helpful comment
Due to padding, I believe adding a single
uint16_tto theTimerclass will actually add 4 bytes to each instance. On the default CLI build for CC2538, that adds 112 bytes RAM (flash overhead difficult to know without some additional prototyping). For comparison, the change in #1267 adds 16 bytes RAM and 1072 bytes flash.A separate question to ask: Does your use cases for microsecond timer really require a (32-bit millisecond, 16-bit microsecond) representation? For example, simplifying to a single 32-bit microsecond representation should give you over an hour before roll-over.
A slight variation/view on Option 2 is to just have logically different Timer schedulers (one for millisecond another for microsecond) both using a single 32-bit integer representation. If a single 32-bit microsecond representation is sufficient for your use cases, then I think this approach may be preferable because you will not have any increase in RAM overhead. It also makes clear what timer type is really required by the user, making it obvious which services are affected if a microsecond timer is not provided by a given platform.