Race condition in interrupt handler causing failures when sending frames

[PinkPandaKatie]

If MCP2515::intHandler() is called from an actual interrupt handler while another SPI transfer is in progress, it will try to initiate another SPI transfer, causing one or both transfers to fail.

There are two ways to work around this in client code:

1. Surround every call to sendFrame(), Mode(), SetRXFilter(), etc. with noInterrupts()/interrupts()
2. Do nothing except set a flag in the actual interrupt handler, then call MCP2515::intHandler later.

The first method works for me because I am not using any other SPI devices on the bus. If I were, then I would have to also disable interrupts for those operations. The second method works best if the flag can be polled frequently, otherwise received frames might get lost.

I'm not sure what the best solution would be for fixing the library, though.

[collin80]

Yeah... it is an interesting problem. Of course I would not want to block an interrupt handler (or silently fail) if something external started an SPI transaction first but on the other hand I really don't want the user sketch to have to poll for the flag either. That's added complication in every single user sketch and also might drastically delay the interrupt from being serviced (depending on what the end user sketch is doing in loop). On the ESP32 I did address this problem by creating a task for the interrupt servicing and waiting on the actual interrupt to set a flag before doing the calls within that task. So, that was option 2 that you wrote. But, it was possible to silently do because the ESP32 is dual core and uses FreeRTOS. On the Macchina or M2 we're not using FreeRTOS and there are no threads.

The ideal scenario is a unified SPI handler that accepts transactions in a queue and services them automatically. I'm not so sure that the SAM3X processor is powerful and spacious enough that such a thing would be very desirable to a wide range of developers. But, maybe... It would solve SPI contention but it would change the SPI API and require anyone using the library to switch to the different SPI API in order to use queued transactions. It sounds like a lot of work. I'd think about doing it if the SAM3X processor seemed to have a long future but well... things are progressing past ATMEGA and SAM3X these days so maybe writing a queuing SPI driver isn't the wisest use of time. Like I said, I've run this library code on the ESP32 and there I modified the library to be safer already.

So, I suppose I ought to go with option 1 and disable interrupts while doing any SPI work outside the interrupt handler. It's simplest but of course has its own issues.