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!
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.
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.
This is one of the cases where I'd really like to up-vote the comment.
There are a lot of occasions where #defines are used without any demonstrable benefit, and a lot of downsides through unnecessary obfuscation. The IRQ Handler is called from one place only, there is no replication.
Where you need a dozen defines to map a single peripheral and it's configuration, consider putting that stuff in a structure, or localize it so the scope doesn't span the entire project.
It is also possible to edit the startup file, removing the WEAK linkage, and scrubbing the vector table. There often seems to be a fear of editing project/board specific files because they are supposedly "library" files. They should be considered as template/boiler-plate code that should be audited and edited to meet higher standards imposed on one's own code.
Thanks for your comments! The feedback is appreciated and makes sense -- clearly there's little to gain from a #define used only once...and as in my example it's possible to lead to unnecessary "problems".
(Wish this forum had a "mark as solved" button and basic quote features...it's really lacking...)
"This is one of the cases where I'd really like to up-vote the comment" +1