nRF24 / RF24Mesh

OSI Layer 7 Mesh Networking for RF24Network & nrf24L01+ & nrf52x devices
http://nrf24.github.io/RF24Mesh
GNU General Public License v2.0
421 stars 154 forks source link

RPi Pico - Potential Instability Issue #215

Closed TMRh20 closed 1 year ago

TMRh20 commented 1 year ago

I have one Pico running the stock RF24Ethernet MQTT example using the Arduino IDE, and I found there was some instability issues. The device would typically work for a while, a few days? then I would find it just blinking the LED and not responding to anything. Further testing revealed that adding mesh.setCallback(yield); in my setup function resolved the issue. I would assume this issue would affect the RF24Mesh examples as well. I'm not sure exactly how to address the issue. maybe put something in the documentation etc?

@2bndy5 I know there are examples for the Pico SDK, would this issue potentially affect those examples also? I haven't looked into this too much to see if having to call the yield() function is normal for the Pico, but it may be worth investigating.

I'm pretty sure the issue is with the RF24Mesh layer technically, but I'll see about recreating it without using RF24Ethernet. It seems easiest to recreate the issue if the master node is taken down for a while and the Pico has to continually renew its address.

TMRh20 commented 1 year ago

Info: https://arduino-pico.readthedocs.io/_/downloads/en/latest/pdf/

2bndy5 commented 1 year ago

Further testing revealed that adding mesh.setCallback(yield); in my setup function resolved the issue.

My first reaction to this is: Did the clang-format change to MESH_CALLBACK definition cause this?

I'm not sure exactly how to address the issue. maybe put something in the documentation etc?

If it only affects the RP2040, then we can add a call to RF24Mesh::begin().

#if defined(RP2) || defined(ARDUINO_PICO) // these might be incorrect macro names
setCallback(yield);
#endif

I'm not really sure where to put something like this in documentation. My initial idea is to add a note in the RF24Mesh::begin() doc string.

there are examples for the Pico SDK, would this issue potentially affect those examples also?

Only one way to find out for sure. I rarely ever use yield(), so it isn't something I've tested. And, I rarely run an example for more than a few minutes. Recently, I accidentally left a RF24 scanner example running for a few days...

2bndy5 commented 1 year ago

Ironically, the author of that Arduino-pico core also heavily contributed to the esp8266 Arduino core. And yield() is often needed in the esp8266 core to let the WiFi stack have a chance to do its thing.

TMRh20 commented 1 year ago

Yeah, that is still ugly to add to a bunch of examples unless they are Pico specific imho. I'd rather add a note somewhere since this is an issue specific to that piece of hardware. Wish there was a better solution... this may take a while to test too, I only have one device to test with. I typically run slightly modified examples on a range of different devices, just to ensure everything is working on different hardware in addition to some fully customized ones. Most of my monitoring is done through Node-Red, so everything is automated and I can tell just by looking at some charts whether a node has lost comms.

TMRh20 commented 1 year ago

Your comment about the ESP8266 got me thinking, there is a spot in RF24Ethernet where yield() is called for the 8266, so I added it for the Pico, removed the mesh.callback line and am now testing that out now to see if the problem is actually at teh Ethernet layer.

2bndy5 commented 1 year ago

I just purchased a RPi pico W board, so I'll be better equipped to test the picoSDK support (want to try diagnosing nRF24/RF24#857).

Any problems with this issue since adding a yield() to the Ethernet layer?

TMRh20 commented 1 year ago

Nope, she has been running steady since that change, want to run it a bit longer though to make sure, but I think it’s good.

On Aug 12, 2022, at 2:29 PM, Brendan @.***> wrote:

 I just purchased a RPi pico W board, so I'll be better equipped to test the picoSDK support (want to try diagnosing nRF24/RF24#857).

Any problems with this issue since adding a yield() to the Ethernet layer?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.

2bndy5 commented 1 year ago

I haven't found any evidence in the picoSDK that mandates yield() calls. There is evidence that yield() is important for multi-threading in the arduino-pico core when using the FreeRTOS feature (see arduino-pico FreeRTOS docs). The arduino-pico core does automatically use yield() in the loop() if TinyUSB is used (TinyUSB stack should be the default - see arduino-pico USB docs).

However, none of our examples (for Arduino or PicoSDK) use the FreeRTOS feature because we don't use multi-threading for the RP2040 (for simplicity - might be beneficial for IRQ). It is possible that the Ethernet layer needs to call yield() to let the Serial bus stay current.