nRF24 / RF24Network

OSI Layer 3 Networking for nRF24L01(+) and nRF52x on Arduino and Raspberry Pi
https://nrf24.github.io/RF24Network/
GNU General Public License v2.0
355 stars 164 forks source link

Multicasting to level 4 fails #226

Closed TMRh20 closed 3 months ago

TMRh20 commented 4 months ago

Per a user in the Arduino forums, there is an issue with multicasting to level 4, sending fails on the transmitter.

Proposed fix is as follows: Change: https://github.com/nRF24/RF24Network/blob/ba5a2a571826452950c06a2281b722a0aa9c41b2/RF24Network.cpp#L1122-L1127

to

if (node == NETWORK_MULTICAST_ADDRESS || node == 010 || node == 01000) {
2bndy5 commented 4 months ago

link to forum post?

TMRh20 commented 4 months ago

https://forum.arduino.cc/t/rf24network-library-issue/1266723/5 but it was really identified/solved via PM

2bndy5 commented 4 months ago

I never fully understood that conditional statement. Technically, any logical address that has a 0 following a non-zero digit is invalid because child nodes start counting at 01. So, I was always confused why 010 was singled out. Now you want to single out 010 and 01000?! What's the real problem here?

TMRh20 commented 4 months ago

Those ( 01, 010, 0100 & 01000 are the logical addresses used for multicast, level 4 is address 01000 so it was being denied as an invalid address. Maybe we should define them?

2bndy5 commented 4 months ago

Oh, that's right! 🤦🏼‍♂️ Yeah, my confusion is why magic numbers are not the best approach. we could define them in the implementation file (RF24Network.cpp) so the definitions are not #included with RF24Network.h.

TMRh20 commented 4 months ago

Right now NETWORK_MULTICAST_ADDRESS is defined in RF24Network_config.h, but yes I think moving them to RF24Network.cpp would work just fine.

2bndy5 commented 4 months ago

NETWORK_MULTICAST_ADDRESS should stay in the header. It is exposed in python bindings and used as a return value from RF24Mesh::requestAddress(). Just the special addresses used for multicasting to network levels should be defined in the cpp file.