Skip to content

Commit 16665cf

Browse files
committed
Correct/clarify TimerEvent::insert documentation
There was much confusion over the functionality of the original `TimerEvent::insert` call which was described as "Set relative timestamp of the internal event". This then extended to my Chrono conversion, meaning the new `insert` call is not equivalent. Clarify the original documentation, correct the deprecation messages, and add more notes on conversion. No functional change, as the new Chrono API makes more sense - it's just different from the old API. Problem actually spotted when I saw the strange code `convert_timestamp` was producing for the 32-bit->64-bit timestamp conversion. The caller of it was actually making the mistake of issuing "TimerEvent::insert(rel_timeout)`, meaning they'd also misunderstood the documentation, and were not getting the timeout they expected. (Chrono would have prevented that mistake as durations and time points are incompatible types).
1 parent f38aa59 commit 16665cf

File tree

1 file changed

+28
-14
lines changed

1 file changed

+28
-14
lines changed

drivers/include/drivers/TimerEvent.h

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,36 +52,47 @@ class TimerEvent : private NonCopyable<TimerEvent> {
5252
// The handler called to service the timer event of the derived class
5353
virtual void handler() = 0;
5454

55-
/** Set relative timestamp of the internal event.
56-
* @param timestamp event's us timestamp
55+
/** Set absolute timestamp of the internal event.
56+
* @param timestamp event's 32-bit us timestamp
5757
*
5858
* @warning
5959
* Do not insert more than one timestamp.
6060
* The same @a event object is used for every @a insert/insert_absolute call.
6161
*
6262
* @warning
63-
* Ticker's present timestamp is used for reference. For timestamps
64-
* from the past the event is scheduled after ticker's overflow.
65-
* For reference @see convert_timestamp
63+
* Ticker's present time is used for reference. The wrapping of the 32-bit
64+
* timestamp is handled by assuming that the time specified is always in the
65+
* future. For reference @see convert_timestamp. This could cause problems
66+
* under load, as if the requested time is missed, the event will be scheduled
67+
* one whole 32-bit wrap (over an hour) in the future.
68+
*
69+
* @warning
70+
* Despite previous documentation, this sets an absolute time, based on
71+
* a wrapping 32-bit timestamp, not a time relative to now. The replacement
72+
* Chrono-based call is `insert_absolute(TickerDataClock::time_point timestamp)`,
73+
* not `insert(std::chrono::microseconds timestamp)`. However
74+
* `insert(5ms)` can be used instead of `insert_absolute(_ticker_data.now() + 5ms)`.
75+
* Due to unclear documentation, it's possible some users may have been
76+
* doing `insert(5000)` without adding the current time, and they could be
77+
* corrected to `insert(5ms)`.
6678
*
67-
* @deprecated use `insert(std::chrono::microseconds timestamp)`
79+
* @deprecated use `insert_absolute(TickerDataClock::time_point timestamp)`
6880
*/
69-
MBED_DEPRECATED_SINCE("mbed-os-6.0.0", "Pass a chrono duration, not an integer microsecond count. For example use `5ms` rather than `5000`.")
81+
MBED_DEPRECATED_SINCE("mbed-os-6.0.0", "Pass a chrono time_point, not an integer microsecond count. For example use `last_chrono_time + 5ms` rather than `last_us_time + 5000`.")
7082
void insert(timestamp_t timestamp);
7183

7284
/** Set relative timestamp of the internal event.
73-
* @param timestamp event's us timestamp
85+
* @param rel_time relative time for event's us timestamp
7486
*
7587
* @warning
7688
* Do not insert more than one timestamp.
7789
* The same @a event object is used for every @a insert/insert_absolute call.
7890
*
79-
* @warning
80-
* Ticker's present timestamp is used for reference. For timestamps
81-
* from the past the event is scheduled after ticker's overflow.
82-
* For reference @see convert_timestamp
91+
* @note
92+
* Ticker's present timestamp is used for reference. If the relative time is
93+
* non-positive, the event will be run immediately.
8394
*/
84-
void insert(std::chrono::microseconds timestamp);
95+
void insert(std::chrono::microseconds rel_time);
8596

8697
/** Set absolute timestamp of the internal event.
8798
* @param timestamp event's us timestamp
@@ -92,7 +103,7 @@ class TimerEvent : private NonCopyable<TimerEvent> {
92103
*
93104
* @deprecated use `insert_absolute(TickerDataClock::time_point timestamp)`
94105
*/
95-
MBED_DEPRECATED_SINCE("mbed-os-6.0.0", "Pass a chrono time_point, not an integer microsecond count. For example use `_ticker_data.now() + 5ms` rather than `ticker_read_us(_ticker_data) + 5000`.")
106+
MBED_DEPRECATED_SINCE("mbed-os-6.0.0", "Pass a chrono time_point, not an integer microsecond count. For example use `last_chrono_time + 5ms` rather than `last_us_time + 5000`.")
96107
void insert_absolute(us_timestamp_t timestamp);
97108

98109
/** Set absolute timestamp of the internal event.
@@ -101,6 +112,9 @@ class TimerEvent : private NonCopyable<TimerEvent> {
101112
* @warning
102113
* Do not insert more than one timestamp.
103114
* The same @a event object is used for every @a insert/insert_absolute call.
115+
*
116+
* @note
117+
* If the timestamp is in the past, the event will be run immediately.
104118
*/
105119
void insert_absolute(TickerDataClock::time_point timestamp);
106120

0 commit comments

Comments
 (0)