Closed Hypecta closed 1 year ago
Please bear in mind that this was all ported from the C++ RF24Mesh, so I'll be looking back to that as a source of truth (expected behavior).
The line in question has its C++ roots here:
uint16_t mask = 0xFFFF;
newAddy &= ~(mask << (3 * getLevel(contactNode[i]))); // Get the level of contact node. Multiply by 3 to get the number of bits to shift (3 per digit)
where the C++ RF24Mesh::getLevel()
is defined as
uint8_t ESBMesh<network_t, radio_t>::getLevel(uint16_t address)
{
uint8_t count = 0;
while (address) {
address >>= 3;
count++;
}
return count;
}
I forget why I chose to reduce the original bit-shifting calc for getLevel()
, but I suspect it had something to do with python's int capacity and the fact that it is only used once in this address validation algorithm.
Unfortunately, the decision to use an octal string and count the digits to get the level didn't prove useful for nodes on level 0 (the master node). That is why there is a specific exception for level 0 nodes in this source.
I need to review this a bit more, but I feel like your solution only accounts for level 2 nodes.
My instinct is to do exactly what the C++ source does. Here is the patch for that idea:
--- a/circuitpython_nrf24l01/rf24_mesh.py
+++ b/circuitpython_nrf24l01/rf24_mesh.py
@@ -183,6 +183,13 @@ class RF24MeshNoMaster(NetworkMixin):
if not contacts:
return False
+ def _get_level(address: int) -> int:
+ count = 0
+ while address:
+ address >>= 3
+ count += 1
+ return count
+
new_addr = None
for contact in contacts:
# print("Requesting address from", oct(contact))
@@ -199,8 +206,7 @@ class RF24MeshNoMaster(NetworkMixin):
and self.frame_buf.header.reserved == self.node_id
):
new_addr = struct.unpack("<H", self.frame_buf.message[:2])[0]
- level = 0 if contact < 7 else len(oct(contact)[2:])
- test_addr = new_addr & ~(0xFFFF << (level * 3))
+ test_addr = new_addr & ~(0xFFFF << (_get_level(contact) * 3))
if test_addr != contact:
new_addr = None
else:
I have uploaded my solution to the resolve-48
branch on this repo.
@Hypecta It would be great if you could git checkout
that branch and see if it works.
On first glance, the inline conditional was really just a quick hack I used during development. It should be faster to use just bit shifting math without extra conditional logic wrapping around that. This is why I'm leaning toward the solution in resolve-48
branch.
Hi @2bndy5 thanks for the fast fix on this!
I personally installed this project with pip install circuitpython-nrf24l01
, hence I had to manually replace the fix you did, and it works!
My current mesh setup is able to provide addresses all the way to the last level (level 4) of nodes.
This is my DHCP output:
Along side with my level 4 child node 0o1112
:
Would this suffice as it working? I'd think so!
FYI: you can install a git source using pip:
pip install git+https://github.com/nRF24/CircuitPython_nRF24L01.git@resolve-48
should install the resolve-48 branch, although I probably got a typo in there somewhere.
Would this suffice as it working? I'd think so!
Great to hear! Sorry I don't have a network setup to test this on... If it's all the same to you, I'll merge resolve-48 solution and release the bug fix (as v2.1.3).
🎉 v2.1.3 is live on pypi. If you're using the adafruit community bundle on MCU devices, then the v2.1.3 should be bundled within 24 hours or so.
BTW, I deleted the resolve-48 branch as it was merged to master.
Hi I've been playing around with the RF24Mesh package and found a possible issue relating to the level 2 child nodes.
Issue: Level 2 child nodes are not able to verify that the provided address from the master node is legitimate as the
test_addr
variable will always be0
.File Name:
rf24_mesh.py
Code Line: 202 Code:level = 0 if contact < 7 else len(oct(contact)[2:])
If level 2 child nodes (e.g. 6th/7th mesh network) were to request for an address from a level 1 child node, the bit-shifting algorithm done to derive
test_addr
will always be0
aslevel
will be0
whencontact
is less than 7 (master node 0o0 & child nodes 0o1 to 0o5)Output of
self.frame_buf.header
,self.frame_buf.message
, and code variables.If Line 202 of
rf24_mesh.py
is modified so thatlevel
is1
forcontact
within the range of1 - 7
, it will allowtest_addr
to reflect the correct address accordingly for bit shifting to verify the address provided:Output of
self.frame_buf.header
,self.frame_buf.message
, and code variables.Do let me know if I need to provide more evidence or outputs to support this.