nRF24 / RF24Mesh

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

parsing dhcplist.txt #204

Closed 2bndy5 closed 2 years ago

2bndy5 commented 2 years ago

I've been experimenting with an idea to convert an existing dhcplist.txt into a JSON format, but I'm hitting some confusion.

A dhcplist.txt contents are in binary, and the following is from running an example in which

key : pair = node_id (in decimal) : mesh_address (in octal)
1 : 05
2 : 04

but the actual contents of the dhplist.txt after this session is:

b'\x01\x00\x05\x00\x02\x00\x04\x00'
output from this python command ``` python -c "with open('dhcplist.txt', 'rb') as f: print(f.read())" ```

Given the use of sizeof(addrListStruct), I expected there to be 3 bytes per key:pair in the dhcplist.txt (not 4).

Notice the extra 0x00 between the node_id and the mesh_address. This extra 0x00 seems related to an uninitialized value because the output from a similar session using the pyRF24 lib is:

b'\x02\x27\x05\x00\x01\x27\x04\x00'

After running a new session with the pyRF24 lib as mesh master, the dhcplist.txt changes to:

b'\x02\xe7\x05\x00\x01\xe7\x04\x00'

I have a feeling this extra byte has something to do with casting the addrList[i] as a char* in saveDHCP(), but maybe I'm missing something.


I'm hesitant to propose any changes since it could break backward compatibility on Linux.

TMRh20 commented 2 years ago

I think it would be because the minimum variable size on the RPi would be 4 bytes, and the sizeof(addrlist) returns 4, so it 'should' always be writing 4 bytes per entry. no?

2bndy5 commented 2 years ago

Wouldn't uint8_t + uint16_t = 3 bytes? I added some test output in a mesh example on my RPi:

sizeof(addrListStruct): 4
sizeof(uint8_t): 1
sizeof(uint16_t): 2

I've seen ostream have trouble with uint8_t in the past, but I was able to remedy that by casting a uint8_t to unsigned int. I doubt that would be an acceptable solution here. Thankfully, this extra byte doesn't seem to be breaking anything at runtime, but it may need to be addressed if load/saveDHCP() is ever ported to arduino platforms.

I think the struct is being expanded to fit 4 bytes for memory alignment which might explain why this rogue byte appears after the uint8_t nodeID.

Going forward, I think I'll just skip the second byte during parsing. If we do anything that would alter how the dhcplists.txt is written, I'm afraid it will break previous versions of loadDHCP().

TMRh20 commented 2 years ago

Wouldn't uint8_t + uint16_t = 3 bytes?

Technically yes

I think the struct is being expanded to fit 4 bytes for memory alignment which might explain why this rogue byte appears after the uint8_t nodeID.

Yup, I think the struct should have been re-ordered with the uint16_t first, that way the padding would be at the end of the struct, so it would align the same on Arduino. Not sure if we want to change it now...

2bndy5 commented 2 years ago

I have the dhcp list in CirPy saved as a JSON if/when the user explicitly calls save_dhcp(). This is why I thought to convert the binary txt to json (or vice versa). I'm glad I started from the C++ side because this rogue byte wouldn't exist if CirPy lib saved the list as binary txt.

2bndy5 commented 2 years ago

I envision saveDHCP() for arduino with SD cards in mind (probably pass a file reference to it). There's nothing stopping someone from inheriting RF24Mesh and implementing their own save/loadDHCP() for arduino.

I'm very hesitant to suggest people use EEPROM (or CirPy's nvm module) to store the dhcp list.

TMRh20 commented 2 years ago

Yeah, I think the EEPROM has a limited number of writes it can handle. Like 100,000 or something if I remember.

2bndy5 commented 2 years ago

That and the lib would have to assume a certain offset has a sufficiently available number of bytes that isn't used by other parts of a user's app. I guess it could take an offset param...

The life cycle of NVM writes can be different depending on the chip manufacturer also.