ticker_insert_event() can crash on KLXX (and probably other platforms) if an event is inserted with a timestamp before the current real time.
The problem is easy to trigger: you just need to set up a Ticker object, and then disable interrupts for slightly longer than the Ticker object's interval. It's generally bad practice to disable interrupts for too long, but there are some cases where it's unavoidable, and anyway it would be better for the core library function not to crash. The case where I had an unavoidably long interrupts-off interval was writing flash with the FTFA. The FTFA hardware prohibits flash reads while an FTFA command is in progress, so interrupts must be disabled for the whole duration of each command to ensure that there are no instruction fetches from flash-resident ISRs in the course of the execution. An FTFA "erase sector" command takes a fairly long time (milliseconds), and I have a fairly high frequency Ticker (1ms).
The problem and the fix are pretty straightforward. ticker_insert_event() searches the linked list to figure out where to insert the new event, looking for a spot earlier than any event currently queued. If the event is in the past, it'll usually end up at the head of the list. When the routine sees that the new event belongs at the head of the list, it calls data->interface->set_interrupt() to schedule the interrupt for the event, since it's the new soonest event. The KLXX version of us_ticker_set_interrupt() then looks to see if the event is in the past, which we've stipulated that it is, so rather than actually setting the interrupt, it simply calls the handler directly. The first thing the Ticker interrupt handler does is re-schedule itself, so we re-enter ticker_insert_event() at this point. This is where the problem comes in: we didn't finish updating the linked list before we called set_interrupt() and thus before we recursed back into ticker_insert_event(). We set the head of the list to the new event but we didn't set the new event's 'next' pointer.
The fix is simply to finish updating the list before we call set_interrupt(), which we can do by moving the obj->next initialization ahead of the head pointer update.
If target supports flash algo, it can get common HAL implementation, providing
feature "HAL_FLASH_CMSIS_ALGO". This simplifies target code, and having
one implementation that can be shared by many targets.
Be careful with flash cmsis algo, in some cases they execute code that
can affect the system. For instance, it can disable cache, or affect
some system clocks. Therefore this feature should be well tested.
Add sleep/deepsleep functions to platform layer which are replacing HAL
functions with the same name, rename existing symbols in HAL layer
to hal_sleep/hal_deepsleep. This way sleep functions
are always available, even if target doesn't implement them, which makes
the code using sleep clearer. It also enables us to make decision on in
which builds (debug/release) the sleep will be enabled.
Unfortunately, it is very difficult to infer the parameters of
function-objects generically in C++03 without any additional type
information. The current implementation fails to do so, and the compiler
simply bails with "unable to deduce template parameter".
Rather than leaving the user with a small novel of error messages, this
patch removes the problematic callback overloads, leaving only callback
overloads for the original pointer types.
The callback class can now accept generalized function-objects:
class Thing {
public:
int value;
void operator()() {
printf("hi! %d\n", value);
}
};
Callback<void()> cb(Thing(2));
However, with the intention of avoiding implicit dynamic-memory
allocations, the Callback class is limited to a single word of storage.
Exceeding this size will eliminate the function-object type from the
overload set and fail to compile.
Effort was invested to make this situation very clear to the user. Here
is an example error message with noise removed:
[ERROR] ./main.cpp: In function 'int main()':
./mbed-os/hal/api/Ticker.h:101:10: note:
no known conversion for argument 1 from 'BigFunc' to 'mbed::Callback<void()>'
The real benefit of this change is the ability for users to hook into
the attributes of the Callback class. This mostly allows lifetime
management of the function-objects from third-party libraries (such as
the Event class from mbed-events).
Note: The convenient `callback` function may become ambiguous if
provided with a type that defines multiple incompatible `operator()`
member functions.
This allows additional attributes to be attached to the internally
generated type such as move and destructor operations with no increase
in RAM footprint.
The current overloads can't take advantage of this, but it does open
the possibility for more powerful overloads that can provide these
additional attributes.
Changes to mbed-os memory consumption:
.text .data .bss
before 57887 2292 7692
after 57842 2292 7691
Enable the low power timer for the following targets:
- NUCLEO_F411RE
- NUCLEO_F401RE
- DISCO_F429ZI
- NUCLEO_F446RE
- NUCLEO_F410RB
- DISCO_F469NI
- NUCLEO_F446ZE
- B86B_F446VE
mjrgh
Christopher Haster
0xc0170
Bartek Szatkowski
Martin Kojtal
Sam Grove
Jimmy Brisson
Bogdan Marinescu
andreas.larsson
Vincent Coubard
svastm
Mike Fiore