Callbacks for received data on CAN or UART?

[WasabiFan]

It doesn't seem like there are callbacks available which are fired on receipt of, e.g., a CAN frame. As far as I can tell I need to check for items in the CAN queue manually.

Is there a way to get this without manually editing the modm interrupt handler? Was there a particular reason it was omitted?

[salkinium]

modm is by design polling, there are only a few exceptions (like AdcInterrupt for some obscure reason).
The issue here is the static memory size on AVRs, providing an interrupt hook for many things quickly fills RAM. So you'd need a split API for AVR and Cortex-M. It's not impossible to do this smoothly (ie. only provide a functional API subset on AVR, which is compatible with an upgraded Cortex-M API), but it's not been done yet, due to prioritizing porting issues.

However, on Cortex-M you can hook the NVIC interrupt vector directly, iff the interrupt table is in RAM.

Something along this pseudocode:

void uart_irq_rx()
{
    MODM_ISR_DECL(USART1_RX);
    MODM_ISR_CALL(USART1_RX);
    // your code after USART Rx call
}

// in main
NVIC_SetVector(USART1_RX_IRQn, (uint32_t)uart_irq_rx);

[nesos]

I'm currently trying to implement a clock synchronization mechanism over CAN and I need the exact time a CAN message arrived (rather than the time I'm reading it from the queue). So interrupts would be very helpful on that.
I have to use a STM32F0, so I can't transfer the vector table to RAM.

This may be naive but wouldn't it be possible to have an abstract class as a callback class. The driver would then hold a pointer to an object to that class and if it is not null (meaning a handler is registered) the handle method will then be called.

Speaking in pseudo code the interface handler would look somehow like this:

class InterruptHandler
{
  void handleInterrupt(void) = 0;  //add parameters as needed for the callback
}

and the actual call could look like this:

MODM_ISR(CAN_RX0)
{
  // process the mailbox

  if (callbackHandler != nullptr)
  {
    callbackHandler->handleInterrupt();
  }
}

This way only the pointers have to be stored in ram. Also the callbacks could be thrown out by a generator options to keep the poll-only version of the driver.

[salkinium]

Yes, we did add a callback like that to the SysTick interrupt here.

Also the callbacks could be thrown out by a generator options to keep the poll-only version of the driver.

Hm, perhaps a better way would be to not automatically generate the interrupt handlers inside modm, it would be cool to be able to only have them as default, and be able to overwrite them with your own handler inside which you call the library IRQ handler.

So, your idea, but inside out:

MODM_ISR(CAN_RX0)
{
    // your custom code
    Can0::irq_handler(); // static function
    // more of your custom code
}

Currently this is not possible because modm defines in its library the MODM_ISR(CAN_RX0) itself, and thus there'd be a symbol conflict, BUT: You can define MODM_ISR(CAN_RX0) as a weak symbol and have it be overwritten.

Which is exactly what we already do for the generated vector table, BUT we alias ALL weak symbols to Undefined_Handler(), which would require us to have like a two-stage weak linking (weak weak linking?).

So… here's another idea: each lbuild module knows what interrupts it needs to define, so it can communicate them to the :platform:cortex-m module using an lbuild Collector, and instead of weakly linking to the Undefined_Handler(), it would weakly link directly to the Can0::irq_handler().
That way you can overwrite any IRQ defined in modm without any cost at all.

Does that make sense to you? The changes would be completely backwards compatible and wouldn't require any active configuration via lbuild options.

[asmfreak]

I like the idea of an optional callback. Would there be a way to call the original function in this model? If yes - how?

[salkinium]

You mean Can0::irq_handler()? That would be defined in modm library and communicated to the :platform:cortex-m module to place into the Vector table.

I mean, this isn't really a callback mechanism, since a callback is defined more semantically clearly, like call me back on CAN message received. This model just allows better custom IRQ hooking, which is significantly more coarse.

[asmfreak]

I understand. I thought that there is maybe a way to call a weak symbol inside strong symbol?
Maybe we could specify both functions something like this:

void main_handler() __attribute__(weak, alias "can_module_handler");
void can_module_handler();

then in the user code we could do something like:

void main_handler(){
  //some code before
  can_module_handler();
  // some code after
}

Or this is not useable?
Handlers should be define-functions like MODM_ISR():

• main_handler is just MODM_ISR(can)
• can_module_handler() call can be something like MODM_ISR_FALLBACK(can)()

[nesos]

I really like the idea of salkinium of implementing the actual ISRs as static function and generating the necessary calls as weak functions in the :platform:cortex-m module.

So just to make sure we're on the same page: All ISRs are declared weak, and depending on whether the module is enabled or not, the generator will call the static function of the module (e.g. the MODM_ISR(CAN0_RX) will call the Can0::isr_rx() ) or if the module is not enabled the generator will just create a call to the DefaultHandler.

[salkinium]

Yes for both. I think we're all talking about the same thing, so let me sketch out the implementation a little more to explain some limitations.

Current situation in vectors.c:

void CAN1_TX_IRQHandler(void)  __attribute__((weak, alias("Undefined_Handler")));
void CAN1_RX0_IRQHandler(void) __attribute__((weak, alias("Undefined_Handler")));
void CAN1_RX1_IRQHandler(void) __attribute__((weak, alias("Undefined_Handler")));

The :platform:can:1 module needs to hook into CAN1_TX, CAN1_RX0 and CAN1_RX1.

extern "C" void
Can1_Rx0() // use C linkage for now
{
    // driver code
}

It would add this to the :platform:cortex-m:vector_defaults collector:

def build(env):
    # StringCollector with format "VECTOR:function"
    env.collect(":platform:cortex-m:vector_defaults",
                "CAN1_TX:Can1_Tx",
                "CAN1_RX0:Can1_Rx0",
                "CAN1_RX1:Can1_Rx1")

Now the vector table is generated differently:

void CAN1_TX_IRQHandler(void)  __attribute__((weak, alias("Can1_Tx")));
void CAN1_RX0_IRQHandler(void) __attribute__((weak, alias("Can1_Rx0")));
void CAN1_RX1_IRQHandler(void) __attribute__((weak, alias("Can1_Rx1")));

Now you can overwrite the CAN1_TX_IRQHandler etc functions by declaring them in your code:

MODM_ISR(CAN1_TX)
{
    // your code
    Can1_Tx();
    // your code
}

BUT here's the catch: alias only works for functions within the same translation unit!!! Furthermore, it must be name-mangled!

alias ("target")

The alias attribute causes the declaration to be emitted as an alias for another symbol, which must be specified. For instance,

void __f () { /* Do something. */; }
void f () __attribute__ ((weak, alias ("__f")));

defines ‘f’ to be a weak alias for ‘__f’. In C++, the mangled name for the target must be used. It is an error if ‘__f’ is not defined in the same translation unit.

This attribute requires assembler and object file support, and may not be available on all targets.

So the above code won't work. I think (but very gladly shown wrong) a "forwarding function" is required, something like this:

void CAN1_TX_forwarder(void) {
    Can1_Tx();
}
void CAN1_RX0_forwarder(void) {
    Can1_Rx0();
}
void CAN1_RX1_forwarder(void) {
    Can1_Rx1();
}
void CAN1_TX_IRQHandler(void)  __attribute__((weak, alias("CAN1_TX_forwarder")));
void CAN1_RX0_IRQHandler(void) __attribute__((weak, alias("CAN1_RX0_forwarder")));
void CAN1_RX1_IRQHandler(void) __attribute__((weak, alias("CAN1_RX1_forwarder")));

But of course this introduces a little overhead for every single interrupt, so it's not free anymore.
But name mangling isn't an issue anymore.

However, an interesting observation: modm modules can now add multiple interrupt handlers for one interrupt. So you could create a lbuild module in modm which adds more functions:

env.collect(":platform:cortex-m:vector_defaults",
            "CAN1_RX0:Can1_Rx0_ClockMeasure",
            "CAN1_RX1:Can1_Rx1_ClockMeasure")

(You don't need to declare an lbuild module for this, you could also add collectors to the project config now):

<library>
  <collectors>
    <collect name=":platform:cortex-m:vector_defaults">CAN1_RX0:Can1_Rx0_ClockMeasure</collect>
  </collectors>
</library>

And then vectors.c would need the forwarding function anyways:

void CAN1_RX0_forwarder(void) {
    Can1_Rx0();
    Can1_Rx0_ClockMeasure();
}
void CAN1_RX1_forwarder(void) {
    Can1_Rx1();
    Can1_Rx1_ClockMeasure();
}
void CAN1_TX_IRQHandler(void)  __attribute__((weak, alias("CAN1_TX_forwarder")));
void CAN1_RX0_IRQHandler(void) __attribute__((weak, alias("CAN1_RX0_forwarder")));
void CAN1_RX1_IRQHandler(void) __attribute__((weak, alias("CAN1_RX1_forwarder")));

So the big question is: can you somehow do away with the overhead of the forwarding function if only one IRQ is declared by the modm modules?

[nesos]

Okay I'll setup a branch tomorrow and try to get it working

[salkinium]

Ok, a few tips to prevent frustration:

0. Collector docs.
1. You have to move the generation of vectors.c into the post-build step, since that's the only place you can access the collectors.
2. Because the :build:scons etc files extract the source files list from the build log, they are also generated in the post-build step.
3. All (post-)build steps are run from lowest submodules group up to root in random order per level. Since :build:scons and :platform:cortex-mare on the same submodule level, the buildlog might contain thevectors.c` file already OR IT MIGHT NOT! I know that sucks, but intra-module build order dependency was an explicit non-goal of lbuild. You need to hack it.
4. You probably want to validate the StringCollector values for correct syntax {vector}:{function} and for correct vector name spelling. See validate_function, example in modm.

[asmfreak]

@salkinium

However, an interesting observation: modm modules can now add multiple interrupt handlers for one interrupt. So you could create a lbuild module in modm which adds more functions:

This is a nice way to handle the single-translation-unit problem. However, I think we need a mechanism to specify ordering of these handlers i.e. which of them are pre- and which of them are post-. Maybe we could sort the collected handlers by name lexicographically (like SysV init scripts)?
Something line lines of:

// separate Translation Unit, driver's impl
void H1_CAN_RX1(){
  // main driver's function
}
// separate TU, user code
void H0_CAN_RX1_clock_before(){
}
// separate TU, user code
void H2_CAN_RX1_clock_after(){
}

// separate TU, vectors.c
MODM_ISR(CAN_RX1){
  H0_CAN_RX1_clock_before();
  H1_CAN_RX1();
  H2_CAN_RX1_clock_after();
}

Also, maybe we could declare the handlers to be always inline? Maybe this would remove some call overhead?

[salkinium]

You could also add an explicit (optional) priority to the StringCollector CAN1_RX1: Can1_Rx1_ClockMeasure:100?

[asmfreak]

I like @salkinium's idea with explicit priority in collector far more then mine, as it is more flexible. Should the priority be ascending or descending?
Whatever it'll be it should be documented, please!