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

meshStarted uninitialized #202

Closed 2bndy5 closed 2 years ago

2bndy5 commented 2 years ago

In RF24Mesh::begin():

    if (meshStarted) {
        radio.stopListening();
    }
    meshStarted = true;
    if(!radio.begin())
        return 0;

So what's with the meshStarted variable? I assume it should be initialized to 0, but that isn't always true (depending on the compiler) because it is never explicitly initialized in the src code. So, when testing my pyRF24 fork, I've been getting an error saying

Traceback (most recent call last):
  File "/home/pi/github/pyRF24/examples/mesh_example_master.py", line 12, in <module>
    if not mesh.begin():
RuntimeError: can't send spi message

The only reason I can think of (right now) is if the compiler is not initializing the meshStarted var to 0 which makes RF24Mesh::begin() call radio.stopListening() before the radio is actually initialized on the SPI bus.

I found that modifying the example to use

if not radio.begin() or not mesh.begin():

avoids this problem, but it means RF24::begin() is called twice which adds delay to mesh example startup time.

2bndy5 commented 2 years ago

git blames https://github.com/nRF24/RF24Mesh/commit/951691059b7f49a0aa741f9a873f8fc5baad26cb citing something about a problem with examples re-init-ing mesh while radio was not in standby mode.

TMRh20 commented 2 years ago

Yeah, it’s used so that mesh.begin can be called if address renewal fails etc. Maybe we just need to initialize the variable?

On Feb 5, 2022, at 6:25 PM, Brendan @.***> wrote:

 git blames 9516910 citing something about a problem with examples re-init-ing mesh while radio was not in standby mode.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you are subscribed to this thread.

2bndy5 commented 2 years ago

Yeah, that would be best practice.

2bndy5 commented 2 years ago

I can understand the convenience of being able call mesh.begin() multiple times because there's nothing stopping the user from mal-configuring the radio (with the RF24 object in user code space). Otherwise renewAddress() seems like a better/faster solution than abusing mesh.begin().

2bndy5 commented 2 years ago

yep, initializing the var to false fixed the problem. I'm closing this since I pushed the solution to master.

2bndy5 commented 2 years ago

I think I found another uninitialized var: addrMemAllocated. By not initializing this one, yet another var addrListTop is not properly initialized. Which means, DHCP() falls victim to segmentation fault since addrListTop isn't guaranteed to match the number of elements in addrList.

Upon first response to mesh child node requesting an address, an out-of-bounds error triggers a seg fault (~I think~ - local tests do confirm this). This took me a while to track down because it is systemic.

2bndy5 commented 2 years ago

closing this again since https://github.com/nRF24/RF24Mesh/commit/aacb9dc00cdc2cec871d1c49bc08a502beb4f94b to master has fixed the other uninitialized vars.