This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

Best practice using #define for ISR function names

Dear Forum,

I hope someone could comment on "best practice" advice for a problem I recently encountered. I experienced interrupt handler callbacks not running when the corresponding interrupt was triggered due to using #define to "rename" interrupt handler functions, and then forgetting to #include the header file containing the relevant #define's.

It was a silly error but remained a mystery for sometime because neither compiler nor linker generated any errors or warnings (which might be because the IRQ Handler functions are declared as '__weak' so the definitions can be overwritten in user files, but I'm not sure).

To be clear, here is an example in which DMA is used for USART transfers and I "rename" the DMA Channel IRQ handler (to make it easier to change USART or DMA channel later).

My interrupt handler function definition would be in stm32l4_it.c like this:

// This function handles DMA Tx interrupt request
void USART_DMA_TX_IRQHandler(void) {
  HAL_DMA_IRQHandler(USART_DMA_Handle.hdmatx);
}

And then in main.h I have the #define for this function name:

#define USART_DMA_TX_IRQHandler     DMA1_Channel2_IRQHandler

In my case I forgot to #include "main.h" in the stm32l4_it.c file. The compiler/linker did not generate any warning or error about a function without a declaration or there being a definition for something that is not used. The error would be that the handler USART_DMA_TX_IRQHandler() would never be called (and consequently DMA transfer stays in a busy state because the transfer interrupt is not cleared).

What is a better method for "renaming" IRQ handlers and making sure the compiler/linker will catch the type of mistake made here?

Will appreciate some suggestions!

Thanks!

Parents
  • The compiler/linker did not generate any warning or error about a function without a declaration or there being a definition for something that is not used.

    Generating an error for those would be strictly unacceptable. To get a warning, you may have to crank up the warning level a bit.

    What is a better method for "renaming" IRQ handlers and making sure the compiler/linker will catch the type of mistake made here?

    How about the obvious one: don't rename them in the first place? The risk/benefit ratio of that plan just doesn't pay off.

    In the face of __weak symbols being supported by the toolchain at all, there's really no replacement for strict coding discipline in naming your global symbols. And in case of doubt, you really have review the map file, too.

Reply
  • The compiler/linker did not generate any warning or error about a function without a declaration or there being a definition for something that is not used.

    Generating an error for those would be strictly unacceptable. To get a warning, you may have to crank up the warning level a bit.

    What is a better method for "renaming" IRQ handlers and making sure the compiler/linker will catch the type of mistake made here?

    How about the obvious one: don't rename them in the first place? The risk/benefit ratio of that plan just doesn't pay off.

    In the face of __weak symbols being supported by the toolchain at all, there's really no replacement for strict coding discipline in naming your global symbols. And in case of doubt, you really have review the map file, too.

Children