Openthread: Multiple microseconds timer support

Created on 29 Jun 2017  路  7Comments  路  Source: openthread/openthread

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:

  1. Platform alarm APIs update

    • Combine current two alarm APIs (alarm, usec-alarm) to single microseconds alarm API, platforms which could not support microseconds can still provide milliseconds accuracy but with the same microseconds alarm APIs.
  2. class Timer update
    Option 1:

    • Replace current uint32_t mFireTime field with the structure otPlatUsecAlarmTime
    • This option looks much concise, although it will increase 2 bytes RAM space for each Timer.

    Option 2:

    • Create a new class TimerUsec based on class Timer, with an extra microseconds field.
    • This option will complicate the 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.

enhancement question

Most helpful comment

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.

All 7 comments

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:

  1. Single microseconds timer is not enough to support the coming new features, we need multiple microseconds timer now;
  2. Only 2 bytes increased to each 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:

  • Keep both alarm APIs (alarm, usec-alarm), and update usec-alarm to use single 32-bit microsecond representation;
  • Reuse the original class Timer for both milliseconds timer and microseconds timer;
  • Add a new 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.

Was this page helpful?
0 / 5 - 0 ratings