noInterrupts() and Interrupts() call potentially unneccesary

[Flole998]

I'm currently looking into code similar to this which can be found all over the library:

acan2517FD/src/ACAN2517FD.cpp

Line 726 in 6ee34ab

noInterrupts () ;

As far as I can see, as we are using attachInterrupt, the mSPI.beginTransaction (mSPISettings) ; a few lines before is taking care of that:

If your program will perform SPI transactions within an interrupt, call this function to register the interrupt number or name with the SPI library. This allows SPI.beginTransaction() to prevent usage conflicts. Note that the interrupt specified in the call to usingInterrupt() will be disabled on a call to beginTransaction() and re-enabled in endTransaction().

Looking at the SPI Library that seems to be the case: https://github.com/arduino/ArduinoCore-avr/blob/24e6edd475c287cdafee0a4db2eb98927ce3cf58/libraries/SPI/src/SPI.h#L181
As we are using attachInterrupt() the interruptMode is greater than 0 so it should not be necessary to do this.

Unless someone has any reason to not do so I would suggest to remove the noInterrupts() and Interrupts() calls. As far as the taskDISABLE_INTERRUPTS () ; for the ESP is concerned I think this should be moved before the beginTransaction() to prevent a race condition there. An interrupt could occur after the beginTransaction() call or in the middle of it messing up the settings. Same thing for the enabling, that should be done after the endTransaction().

These changes should remove quite some instructions from each SPI transaction, thus increasing the performance.

[Flole998]

After reading pierremolinaro/acan2517#5 I am not sure if all of my suggestions would work: Apparently there is a reason why noInterrupts was added there. However, this is no longer needed on the MCP2518FD so we could add a #ifdef there and introduce a DISABLEMCP2517FDSUPPORT define which disables this workaround if it's not needed as the faulty 2517 is not used.