pinMode(IRQ_PIN, INPUT) needed in examples

[SimonMerrett]

https://github.com/nRF24/RF24/blob/a0cd707137607842f0ef782f12f2e40409a12d38/examples/pingpair_irq_simple/pingpair_irq_simple.ino#L54

I fell foul of this on a samd21-based Seeeduino Xiao today. Without setting the pinMode(INPUT_PULLUP) the sketch enters an ISR loop which prevents USB serial communication and, no doubt, many other features if you attempted to add them.

It might also be worth using the Arduino-recommended digitalPinToInterrupt(0) in attachInterrupt() too. I don't have a fork, just the Arduino library manager version which I have mangled, so this is a request for another contributor to wrap this into their next PR.

[2bndy5]

I've been literally debugging the new examples for the irq pin (as of an hour ago). I'm jealous of the Seeduino Xiao; I've been thinking of getting the adafruit clone that has 2MB extra storage... I digress.

You shouldn't need to use a pull-up resistor on the IRQ pin because it is always HIGH until triggered. From my latest debugging experience, you should use the FALLING mode parameter to attachInterrupt() as using LOW seems to cause more than 1 ISR call per event.

Notice the new example uses digitalPinToInterrupt(2) as arduino docs state that pin 2 is supported on most boards for IRQ events. I think samd supports any digital input pin for IRQ events.

Also samd boards' usb serial communication are best used when you

while (!Serial) {}

after calling Serial.begin() because they start up so fast and boast native USB support.

[SimonMerrett]

Well yes, that should be the case. Note that for Xiao they have tristated all pins on startup and they are not configured as inputs by default (and there isn't a default pullup version on Arduino or other boards that I'm aware of, so we are definitely reliant on the RF24 pullup). They do this to reduce power consumption in sleep, although with a power led that is hard to remove cleanly I'm not sure it's that relevant to the Xiao.

Adafruit + clone made me laugh. For once open source works in the other direction!

Happy to use 'FALLING' personally but wondering if that works on all boards. And you're correct, I should have said digitalPinToInterrupt(2) instead of 0.

I will try with while (!Serial) {} and no pinMode on the Xiao and let you know if that changes anything but my bet is that we're seeing an issue (not the ISR loop I originally assumed) with an ISR enabled on an otherwise uninitialised pin. I do think it's good practice to 'pinMode()' if we are going to use a pin in example code.

[2bndy5]

You definitely need to call pinMode(IRQ_PIN, INPUT). My comment was about weather or not the mode parameter to pinMode() should use INPUT_PULLUP because internal pull-up resistors aren't always available on certain pins or at all on certain boards.

They do this to reduce power consumption in sleep, although with a power led that is hard to remove cleanly

The nRF24L01 doesn't have a power indicating LED, so I'm not sure who "they" are.

Adafruit + clone made me laugh. For once open source works in the other direction!

Not the first time adafruit has cloned an open source board. The Hazzah 8266 is a clone of the nodeMCU... The are many others, but this is the one that actually made the most modifications to the original design (that I've seen). The Seeduino Xiao clone is called QTpy.

I can tell you that atsamd21 benefits from the while (!Serial) {} because I had to use it on my adafruit Feather M0 Express so that the example introductory prompt could print or if (!radio.begin()) could print a hardware failure.

[SimonMerrett]

Good point, I have changed the description.

[SimonMerrett]

The nRF24L01 doesn't have a power indicating LED, so I'm not sure who "they" are.

They are whoever maintains the Arduino core for the Xiao. If you click the link you will see that they commented out the default Arduino practice of initialising every pin as INPUT, to save power on the Xiao, not the nrf.

[2bndy5]

Happy to use 'FALLING' personally but wondering if that works on all boards

Good query. According to the arduino.cc docs on attachInterrupt(), I think it is implemented as the ones that aren't globally supported are explicitly mentioned.

FYI, this repo officially supports all Arduino boards in which support is shipped with the IDE. Notice the line architecture=* in library.properties file. Unfortunately, the Xiao is an edge case where the repo assumes this library will work for it, but with no guarantee.

I've been adding to a huge list of boards in a new "Arduino build" github action that test compiles the examples for as many boards as possible. I'd be happy to add the Xiao to this list, provided the Xiao has the pin numbers 2, 7, & 8 available (& they aren't occupied by the SPI bus's MOSI, MISO, SCK pins). Although, I need to look into whatever is the most popular/recommended core to augment the IDE with Xiao support. For more on that issue please refer to #665

[SimonMerrett]

Sadly not. I personally don't mind if the Xiao isn't supported - it was a quick board for a cheap price available on free next day delivery. But there are probably many boards where the lack of pinMode may catch people out. Thanks for all your contributions.

[2bndy5]

Damn. 1 pin short of seamless compatibility with the examples. I really like the form factor for this board 🤤.

For more updates on the new examples, you can subscribe to #658

FYI, I have removed the pingpair_irq_simple.ino (on my fork) as it was basically a duplicate of the new InterruptConfigure.ino example. The pingpair_irq.ino will remain for reference (throwback to maniacBug's original examples). However this issue still also applies to pingpair_irq.ino.

[2bndy5]

They are whoever maintains the Arduino core for the Xiao. If you click the link you will see that they commented out the default Arduino practice of initialising every pin as INPUT, to save power on the Xiao, not the nrf.

@SimonMerrett I'm sorry, I was using my phone, and the link didn't forward directly to that line (using the github app). It turns out that the commit that commented that block out was directly from adafruit people (as of Apr 17, 2019). Probably a development from cloning the Xiao and forking the Seeed-Studio fork of LynnL4's fork of the Arduino samd core (that's a mouthful "of fork"s).

[SimonMerrett]

No need to apologise! I only thought that might be the culprit as I have recently been chasing the low power genie round a custom samd21 board and that adafruit thread comes up near the top of search terms like "why can't I hit the sleep current in the samd21 datasheet". I now have my custom board definitions with that section commented out as it seems a gratuitous and unnecessary attempt to make behaviour "common"across different Arduinos. That's the only reason I bothered to check the Xiao wiring.c

[2bndy5]

Just an update about a comment I made here

From my latest debugging experience, you should use the FALLING mode parameter to attachInterrupt() as using LOW seems to cause more than 1 ISR call per event.

Turns out that using FALLING will also trigger multiple ISR calls because whatHappened() (which should be called in every ISR callback) clears the status flags. Additionally, the CE pin is still HIGH if using the startFastWrite() in conjunction with whatHappened() (most ideal algorithm). However, when whatHappened() clears the MAX_RT status flag while the CE pin is HIGH, the transmission seems to be re-started because the failed payload is still in the TX FIFO, and the radio is just doing what it was designed to do. Most of this scenario is because C++ is so blistering fast.

In conclusion, the ISR callback should either immediately bring the CE pin LOW (preferred) or flush the TX FIFO on transmission failure (most accessible given this library's API and ce() is a private function). I've opted to do the later in the new interruptConfigure example. I still prefer using FALLING (especially on Linux) for faster MCUs because using LOW can queue multiple ISR callbacks (for the same event) before the initial ISR callback is done executing while FALLING will only queue one ISR callback per event.