PowerBroker2 / pySerialTransfer

Python package to transfer data in a fast, reliable, and packetized form
MIT License
144 stars 35 forks source link

Arduino tx_data + pySerialTransfer rx_data example fails with esp32 Arduino #51

Open PaulBouchier opened 2 years ago

PaulBouchier commented 2 years ago

On multi-byte wide processors the compiler aligns data items to their native alignment, unless directed otherwise. Thus a float is aligned to a 4-byte boundary. Therefore the testStruct in tx_data is laid out in memory differently between an Arduino Uno & a board like an esp32. Since txObj uses sizeof(T) as the number of bytes to transmit, tx_data sends a different number of bytes from esp32 compared to Uno.

testStruct is shown below with the fix needed for processors with > 8-bit wide data bus struct STRUCT { char z; float y; //} testStruct; } attribute((packed)) testStruct;

The original code (commented out line 4 above) causes 3 empty bytes to be placed after z, and before y on esp32, so txObj sends 8 bytes on an esp32, but only 5 on an Arduino Uno. pySerialTransfer is unaware of the packing difference, so tries to pull the float out of bytes 1-4 (counting byte 0 as z), regardless of the sender-architecture.

The example as provided prints the following correct message on pySerialTransfer + Uno: b'$'4.5 | hello but the following error message on pySerialTransfer with esp32, caused by pySerialTransfer reading the float out of bytes 1-4 then trying to read the char array out of byte 5, which is actually the 2nd byte of the float.: Traceback (most recent call last): File "./rx_data.py", line 30, in arr = link.rx_obj(obj_type=str, File "/home/bouchier/.local/lib/python3.8/site-packages/pySerialTransfer/pySerialTransfer.py", line 359, in rx_obj unpacked_response = unpacked_response.decode('utf-8') UnicodeDecodeError: 'utf-8' codec can't decode byte 0x90 in position 1: invalid start byte

Removing the string transfer part of the example prints the following incorrect message on pySerialTransfer + esp32, : b'$'0.0 | Note the float, which should be 4.5, is 0.0 because pySerialTransfer is reading the float out of the 3 empty bytes plus first byte of the float.

The correct way to eliminate machine/compiler dependent packing is with the packed attribute as shown on line 5 of testStruct above. Making this change produces the correct output on both Uno & esp32: b'$'4.5 | hello

Are you open to pull requests to fix the examples so they work on both Uno & esp32? I would test any such changes on both.

ipsod commented 2 years ago

I was considering using nanopb or the MsgPack format with ArduinoJson on the esp32. This would give a more versatile and universal format, though I'm not exactly sure what the overhead is.

I'm also trying to use this with ESP32 SerialBluetooth.

PaulBouchier commented 2 years ago

@ipsod if all you want is COBS packetizing, there's a package, "PacketSerial" that is just a COBS packetizer and has a c++ and python version. The reason I like this package is it does all the rest of the stuff you want, like CRC, callbacks, packet header with length and packet-type, which I'd otherwise have had to build on top of PacketSerial. I suspect you'll want that, irregardless of the physical media or data encoding. This package is firmly at the transport layer.

ipsod commented 2 years ago

@PaulBouchier I need everything you mentioned, but need it to work with ESP32 and BluetoothSerial.

PowerBroker2 commented 2 years ago

Are you open to pull requests to fix the examples so they work on both Uno & esp32? I would test any such changes on both.

Absolutely! I'm always interested in PRs from the community.

Yeah, I've had to use __attribute__((packed)) for structs on Teensys running SerialTransfer.h, I just never thought to update the examples.

PaulBouchier commented 2 years ago

@ipsod have you tried replacing Serial.begin(115200) myTransfer.begin(Serial);

with SerialBT.begin("ESP32 test"); myTransfer.begin(SerialBT);

I don't know - I've never used BT serial, but it looks to me from a brief web search that SerialBT works like Serial.

ipsod commented 2 years ago

@PaulBouchier No, not yet. Since this library failed with standard serial transfer on the ESP32, I never took the next step of trying to use it with BluetoothSerial. Also, I'm not sure what things will look like on the Python side (using pyBluez for bluetooth serial).

Thanks for the head-start, though.