Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pinMode(IRQ_PIN, INPUT) needed in examples #680

Closed
SimonMerrett opened this issue Nov 10, 2020 · 11 comments · Fixed by #691
Closed

pinMode(IRQ_PIN, INPUT) needed in examples #680

SimonMerrett opened this issue Nov 10, 2020 · 11 comments · Fixed by #691
Assignees

Comments

@SimonMerrett
Copy link

SimonMerrett commented Nov 10, 2020

attachInterrupt(0, check_radio, LOW); // Attach interrupt handler to interrupt #0 (using pin 2) on BOTH the sender and receiver

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
Copy link
Member

2bndy5 commented Nov 10, 2020

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
Copy link
Author

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
Copy link
Member

2bndy5 commented Nov 10, 2020

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 SimonMerrett changed the title Pullup needed on irq pin in examples pinMode(IRQ_PIN, INPUT) needed in examples Nov 10, 2020
@SimonMerrett
Copy link
Author

Good point, I have changed the description.

@SimonMerrett
Copy link
Author

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
Copy link
Member

2bndy5 commented Nov 10, 2020

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
Copy link
Author


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
Copy link
Member

2bndy5 commented Nov 10, 2020

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 2bndy5 self-assigned this Nov 10, 2020
@2bndy5
Copy link
Member

2bndy5 commented Nov 10, 2020

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
Copy link
Author

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 added a commit to 2bndy5/RF24Revamped that referenced this issue Nov 11, 2020
@2bndy5
Copy link
Member

2bndy5 commented Nov 15, 2020

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.

This was referenced Nov 27, 2020
@2bndy5 2bndy5 linked a pull request Nov 29, 2020 that will close this issue
2bndy5 added a commit that referenced this issue Dec 14, 2020
* don't hide EN_ACK_PAY reqs; adjust examples & docs

* removed duplicated calls in examples/tests

* too many examples

* update docs about reUseTx()

* deprecate enableDynamicAck()

* clarify reUseTx()

* typo in func ref

* append setAutoAck() about pipe 0

* consistent casing in "auto-ack"

* fix typo

* revert this "see also" changes

* use ack_payloads_enabled to full potential

* undeprecate flush_rx()

* isPVariant return internal var; default dyn_pl on

addresses the following issues:
- #401
- #641
- #660

* oops, readdress #401 #641 #660

* address #640

* deprecate isAckPayloadAvailable() #664

* try to ignore return value; trigger forked actions

attempting to address #636

* this is why I can't have nice things

* retrying to ignore return value; fix init order

* add CI for avr compiling examples

* build doxygen with latest release ver number

* revert doxyfile, fix doxygen.yml, make2 = install

* build examples changes, doxygen.yml bad indent

* remove unnecessary pipe chars in doxygen.yml

* linux CI changed, pde->ino files

* platform specific examples; fix WiringPi build

* fix using void return value

* remove gemma & lilypad; fix certain example vars

* wiringPi may be broken #669

* 8 paragraphs that say keep auto-ack enabled

#667

* reverted examples folders; exclude maple & 3pin

* exclude didn't work

* trigger arduino build on workflow changes

* explicit examples' paths

* no wildcards in explicit paths

* no quotes in yaml sequence

* try using a pipe char

* arduino build "needs" A-style check

* applied arduino IDE's auto formatting

* remove format checker

* fine let the example terminate

* all official arduino cores to build_arduino.yml

* backup examples; new GettingStarted

* no explicit examples; compile all

* new InterruptConfigure example

* // about when IRQ flags get cleared; & a fix typo

* new StreamingData & MulticeiverDemo examples

* reduce streamData compilation size

* tested GetStarted, AckPayloads, Streaming

* IRQ & Multicceiver examples tested

* undo #646; add broken FakeBLE example

* fix arduino build CI

* forgot to use pipe char

* typo

* 2nd job should run on ubuntu-latest

* fqbn arch-type is wrong?!

* uncomment #define IRQ_PIN from testing artifacts

* use SpenceKonde core released zip file

* use SpenceKonde board definitions

* use correct vendor name?!

* use lower case?!

* platform name should match the fqbn

* what am I doing wrong? use SpenceKonde attiny core

* under "platforms" not "with"

* yml formatted "platforms" options

* "with: platforms:"

* source-path uses relative path

* use trailing / in source-path

* clear advice from per1234 about yml syntax

* fixing source-path and version number

* use direct download link to SpenceKonde release

* adjust source-path again

* no source-path

* redo-ing #646

* timingSearch3pin includes stdio

* don't ignore rf24_ATTiny examples

* separate job for separate ATTiny examples

* timingSearch3pin uses redefined spi funcs

* drop testing on attiny43, install core from json

* keywords updated removed duplicate examples

* CallResponse was duplicate; (+) ManualAck example

* doc updates #671 tested new manAck example

* FakeBLE crc calc not checking out

* add per1234's action to check formatting

* did not find expected alphanumeric char?!

* add manualAcknowledgements example to docs

* don't remove comment block prefix

* restored comment prefix in old examples also

* less confusing comments about addresses

* clarify docs on return value for all write-related

* remove broken fakeBLE example

* forgot to remove fakeBLE from arduino workflow

* test deploy to gh-pages

* bad yml format

* something's wrong with the doxygen build CI

* lack of new line in end of  doxyfile

* remove debug prompt; deploy on publish released

* deploy on any release event

* fix main page useful links

* only clear RX_DR flag in read()

* typo

* doc updates; new python examples

* new gettingstarted example for linux

* add micros() to applicable utilities compatiblity

* define micros() in arch_config.h

* printDetails() in linux

* pipe number not printing correctly

* oops it was a non-printable char

* avoid keyboard interrupt

* build wiringPi examples also

* linux build CI for SPIDEV

* ignore return value in utilty/SPIDEV/interrupt.c

* trying rf24_micros()

* ammend example to use rf24_micros()

* let micros() be defined in examples for linux

* need cstdio

* ported arduino examples to linux (& python)

* oops

* should probably declare i huh

* copy n paste much?

* don't use stoi for a char

* typo, expose digitalRead(), ifdef RF24_WIRINGPI

* ifdef WIRINGPI didn't help, fix inline condition

* testing round 1; fix pyRf24/setup.py

* bug fixes in py examples prompt fixes in linux

* fix irqConf for arduino, rename stream_data.py

* copy n paste artifact

* debugging streamingData

* testing round 2

* oops

* timing output changes; printDetails() also

* debugging IRQ configure example

* debugging IRQ_config.py also

* declare timer stuff for micros in manACK example

* typo

* doc read() for python; add flush_rx() to py wrap

* remove cap on how many bytes to read in python!!!

* don't cap bytes read() from RX FIFO in C++

* supposed to be decode() not encode()

* fix typos; better python doc about read()

* py wrapper omits len args for buf-related funcs

* fix streaming_data.py master()

* duplicate prompt in ackPay.py

* prompt format changes

* big commit

- reverted defaults about Dynamic Payloads and Dynamic ACKs per @TMRh20 suggestions about Si24R1 in #658. This reverts #660 & #661
- ammended docs & all examples about above suggestions
- re-wrote InterruptConfigure examples to use attachInterrupt(); py example uses RPi.GPIO equivalant.
- removed MKR-based boards from Arduino build CI as pin 2 on those boards don't support interrupt requests, and InterruptConfigure.ino uses pin 2 for exactly that.
- added building of python wrapper to Linux workflow; it now also runs pylint on python examples
- removed my attempt to port digitalRead() to various Linux drivers as it is no longer needed in the interruptConfigure.cpp example.
- addressed #674 about RX_PW_Px registers manipulation.
- remove and ignore my VSCode folder as it is machine-specific
- removed contradicting "examples_linux" ignores and added "examples_linux/__pycache__"  folder
- RF24::begin() now brute forces a soft reset. It does not reset addresses set to the pipes, but it does close all pipes. This helps address #401
- fixed some typos/copy-n-paste issues in the docs.
- added instructions on how to run the python example scripts

* Linux build CI use pip3 to install RPi.GPIO

* workflow uses python 3.7 x86

* workflow uses python 3.x x86

* python needs x32 called in workflow

* fix address assignment in linux examples

* don't check python examples in workflow

* define INPUT in RPi-BCM driver

* remove GPIO.cleanup() from py examples

* payloadSize is an attribute in python

* fix python examples about getPLsize()

* use RuntimeError in python examples when begin() is false

* testing round 3

* streamData uses write(); python examples need 3.7+

* fix ellapsedTime in streamingData

* status byte saved as private member

addresses #678 #679 #677

removed comments in build_linux.yml about building/testing the python wrapper

added a fail fast prompt to the streaming examples when failures >= 100

* simplify logic about _is_p_variant

* testing round 4 & address #680

* fix irq examples

* fix compilation errors for IRQ examples

* fix irq example for linux

* tweak IRQ py example; writeAckPayload returns bool

* final tweaks to IRQ examples; some doc updates

* leave closeReadingPipe(0) in startListening()

* arduino examples work

address 2 new issues
1. #687 added printPrettyDetails(); updated python wrapper and keywords.txt
2. #686 centralizing all SPI access to write_*() and read_*() functions

Also reverted changes to gitignore about examples_linux folder while adding/ignoring some things about building the python wrapper (venv, *.pyc, __pycache__)

* remove volatile from radio declaration

* datasheets; fix man_ack.py example

updated comment proceeding radio object in python examples

documented enums and linked references to them. Also linked references in COMMON_ISSUES.md & main page's changelog.

* ATTiny examples should work on SpenceKonde core

* exclude ATTinyx313 from timingSearch3pin example

* oops, bad indent

* Astyle indent for #ifdef is 0

* (+) CLI args to examples. pwr down radio on ctrl+c

* fail if no valid arg is specified

* fix parsing args in python examples

* make py examples behave like c++ examples

* found a bug in py examples' -r arg

* testing round 5

* adjust manpage descriptions

* don't use signal.h in cpp examples

* del cli args from all linux examples except 2

* edit doxyfile OUTPUT_DIR

* only run doxygen on master branch

* redirect doc links to nRF24 org

* missed a link

* remove use of timeout bool in manAck.ino

* self-reviewed PR changes

* ATTinyCore adjustments

- use macros from SpenceKonde ATTinyCore
- add macros for CSN settling times for when using the ATTinyCore

* some more self-reviewed PR changes

* exclude root README from doxygen output

* exclude GPIO & SPI classes from doc'd classes

* add Arduino & Linux build badges to readme

* Change link to docs in readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants