arduino / ArduinoCore-API

Hardware independent layer of the Arduino cores defining the official API
https://www.arduino.cc/reference/en/
GNU Lesser General Public License v2.1
202 stars 117 forks source link

Should SPI `attachInterrupt()` and `detachInterrupt()` be removed? #183

Open carlosperate opened 1 year ago

carlosperate commented 1 year ago

It looks like these methos are only used in the AVR ports, and even then they indicate that they should not be used.

ArduinoCore-avr/libraries/SPI/src/SPI.h#L306-L310 ArduinoCore-megaavr/libraries/SPI/src/SPI.h#L183-L187

  // These undocumented functions should not be used.  SPI.transfer()
  // polls the hardware flag which is automatically cleared as the
  // AVR responds to SPI's interrupt
  inline static void attachInterrupt() { SPCR |= _BV(SPIE); }
  inline static void detachInterrupt() { SPCR &= ~_BV(SPIE); }

Other cores do nothing or even do not implement it:

When are these methods meant to be used by Arduino users or ArduinoCore developers?

matthijskooijman commented 1 year ago

From a quick glance at the AVR SPI library, the actual ISR is not implement anywhere, which I think means that calling attachInterrupt() will cause the system to lock up the next time an SPI interrupt is triggered (since the default ISR just has an infinite loop IIRC). Sounds like removing these would indeed make sense - the only case these methods could be "useful" is when a sketch also implements the ISR itself, but those are very specific and advanced sketches that can also just modify SPCR directly.