Implementation of interrupt hooks

[nesos]

So second try on the interrupt hooks. 😄

This approach will do the interrupt handling in three steps. First it calls the entry hook, then the main method and at last the exit hook. The two hooks are defined as weak, empty functions, that can be redefined in the user code.

To enable the hook, the ISR macro in the driver just has to be changed from MODM_ISR to MODM_ISR_HOOKS . This macro will be compatible to the old interrupt handling but additionally creates the two hook methods.

To implement the hooks there are two macros: MODM_ISR_ENTRY_HOOK to implement the entry hook and MODM_ISR_EXIT_HOOK to implement the exit hook.

I added an example for the Nucleo-F042 that toggles the LED whenever a byte has been received via the UART.

[salkinium]

Do a scons listing, open modm/build/nucleo_f042k6/can/release/hooks.lss and search for USART2_IRQHandler. Hmmmmm:

08000e0c <USART2_IRQHandler>:
USART2_IRQHandler():
modm/examples/nucleo_f042k6/hooks/modm/src/modm/platform/uart/uart_2.cpp:145
MODM_ISR_HOOKS(USART2)
 8000e0c:	b510      	push	{r4, lr}
 8000e0e:	f7ff f9af 	bl	8000170 <USART2_ISR_Entry>
 8000e12:	f7ff ffc7 	bl	8000da4 <_Z15USART2_ISR_Mainv>
 8000e16:	f7ff ffc3 	bl	8000da0 <USART2_ISR_Exit>
 8000e1a:	bd10      	pop	{r4, pc}

Making the *_ISR_Main function static inline copies the function into the *_IRQHandler, however there seems to be no way to inline a weak function.

[nesos]

I tried to apply some optimizations on that but so far had no success.
It appears that inlining the hook mechanism breaks the whole thing along with a warning.

The compiler also doesn't drop the call to the empty functions, which is weird. I read that it would only drop it, if the function is static, but as it is declared weak, we can't declare the hooks static. I really don't get why the compiler does not completely optimize out empty functions as they don't do anything.

So here is an idea on how to optimize this:

We could use lbuild to enable the hooks by adding an option something like
<option name="modm:platform:uart:2:hooks.enabled">true</option>
If this option is enabled, the generator will use the MODM_ISR_HOOK macro otherwise it will use the normal MODM_ISR macro. This way we don't have overhead when not using the hooks and only add the overhead of calling enter and exit hook functions when we actually need it

What do you think?

[salkinium]

What I don't like about MODM_ISR_HOOK is that you need to know which IRQs are implemented in modm, you cannot simply declare MODM_ISR_ENTRY_HOOK(UART3_RX) and expect it to work, if modm wasn't built with the ::uart:3 module.

<option name="modm:platform:uart:2:hooks.enabled">true</option>

Isn't this just another way to implement #307 though?

You could implement UART callbacks as an optional lbuild feature, but they are then UART driver specific with UART driver semantics (ie. callback for receiving a byte, not callback for getting an IRQ). I'd be ok with that (analogue for SPI, I2C, ADC etc).

Or a way to hook NVIC interrupts, but then it would need to be a generic mechanism that's transparent to any driver using these IRQs too.

In general, allowing to define drivers callbacks that then get executed in an (nested) interrupt context is a very, very good recipe for a lot of concurrency bugs. I've never ever seen it work well.
The solutions to this are the typical RTOS solutions (ie. wake a specific thread after IRQ exit, via semaphore signalling). Effective, but complicated and still very easy to get wrong. This is why modm doesn't have callbacks, it drags in so much necessary overhead and API complexity.

[nesos]

What I don't like about MODM_ISR_HOOK is that you need to know which IRQs are implemented in modm

Hmm isn't that also the case for the EXTI ISRs?

However, I see your point. So should we drop the custom ISR hooks?

[salkinium]

I need to think about this some more, I find this problem very interesting since it probably hasn't been solved many times before. There's also a tangent relation to the RTFM scheduler concept.