hideakitai / MsgPacketizer

msgpack based serializer / deserializer + packetize for Arduino, ROS, and more
MIT License
79 stars 10 forks source link

Re: Additional bug related to encoding of uint64_t value #15

Closed KennethThompson closed 11 months ago

KennethThompson commented 11 months ago

This bug was detected when implementing UDP communication, however, it likely impacts Serial as well.

The code below includes sending two structures with the same layout. However, the fails structure includes the featureMap set to 0x10003.

When receiving the 'Fails' structure via udp, the receiver gets the following bytes: [0x5] [0x4] [0x94] [0x1] [0xce] [0x2] [0x1] [0x7] [0x3] [0xcd] [0x13] [0x88] [0x1] [0x9b] [0x0]

The "Works" structure is received via UDP as: [0xc] [0x4] [0x94] [0x1] [0xcd] [0x10] [0x1] [0xcd] [0x13] [0x88] [0x1] [0x60] [0x0]

So as you can see, the 'Fails' bytes appear corrupt as the length of 0x5 is incorrect. 'Works' shows a proper length value.

I am not sure why changing featureMap from 0x1001 to 0x10003 causes the output bytes to be corrupted/incorrect.

Perhaps I am making a mistake or misunderstanding what can be encoded via MsgPack?

Looking forward to your advice/confirmation.

Thanks very much!

// #define MSGPACKETIZER_DEBUGLOG_ENABLE
#include <MsgPacketizer.h>
#include <Ethernet.h>
#include <EthernetUdp.h>

// Enter a MAC address and IP address for your controller below.
// The IP address will be dependent on your local network:
byte mac[] = {
  0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED
};
IPAddress ip(192, 168, 1, 88);

unsigned int localPort = 54321;      // local port to listen on

// buffers for receiving and sending data
char packetBuffer[UDP_TX_PACKET_MAX_SIZE];  // buffer to hold incoming packet,
char ReplyBuffer[] = "acknowledged";        // a string to send back

// An EthernetUDP instance to let us send and receive packets over UDP
EthernetUDP Udp;

  struct Fails {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x10003;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }f;

  struct Works {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x1001;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }w;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
      // You can use Ethernet.init(pin) to configure the CS pin
  //Ethernet.init(10);  // Most Arduino shields
  //Ethernet.init(5);   // MKR ETH shield
  //Ethernet.init(0);   // Teensy 2.0
  //Ethernet.init(20);  // Teensy++ 2.0
  //Ethernet.init(15);  // ESP8266 with Adafruit Featherwing Ethernet
  //Ethernet.init(33);  // ESP32 with Adafruit Featherwing Ethernet

  // start the Ethernet
  Ethernet.begin(mac, ip);

  // Check for Ethernet hardware present
  if (Ethernet.hardwareStatus() == EthernetNoHardware) {
    Serial.println("Ethernet shield was not found.  Sorry, can't run without hardware. :(");
    while (true) {
      delay(1); // do nothing, no point running without Ethernet hardware
    }
  }
  if (Ethernet.linkStatus() == LinkOFF) {
    Serial.println("Ethernet cable is not connected.");
  }

  // start UDP
  Udp.begin(localPort);
}

void loop() {
    // must be called to trigger callback and publish data
    MsgPacketizer::send(Udp, "192.168.1.2", 54321, some_index, w);
    //delay(1000);
    MsgPacketizer::send(Udp, "192.168.1.2", 54321, some_index, f);
    //delay(1000);
    MsgPacketizer::update();
}
KennethThompson commented 11 months ago

I can further confirm that the MsgPack/Packer is correctly packing the structure. The problem appears in the Packetizer code. I also tried tweaking the constants such as #define MSGPACK_MAX_OBJECT_SIZE 64 , with no success. For now, I have a manual packetizing routine until this issue is smoothed out. Receive is working just fine via MsgPacketizer when I echo back a structure that the current MsgPacketizer cannot encode without error.

hideakitai commented 11 months ago

Thanks! I'll see it later

KennethThompson commented 11 months ago

Thanks! I'll see it later

I've done further testing since, and it looks like the issue may be in the MsgPack code. When I perform manual packing (without the packetizer) on a structure with the 64 bit value (uint64_t), the packer object appears to return an improper .length value, or the memory in the packer is corrupted. I know this to be the case, because a memcpy against the packer.data with the returned packer.length results in a fault and reboot of the Arduino.

When I change the uin64_t portion of the structure to a uint32_t, the problem seems to go away. Perhaps the current version of MsgPack cannot encode 64 bit values?

I am using the following defines:


#define MSGPACKETIZER_MAX_PUBLISH_ELEMENT_SIZE 5
// max destinations to publish
#define MSGPACKETIZER_MAX_PUBLISH_DESTINATION_SIZE 1

// msgpack serialized binary size
#define MSGPACK_MAX_PACKET_BYTE_SIZE 96
// max size of MsgPack::arr_t
#define MSGPACK_MAX_ARRAY_SIZE 7
// max size of MsgPack::map_t
#define MSGPACK_MAX_MAP_SIZE 7
// msgpack objects size in one packet
#define MSGPACK_MAX_OBJECT_SIZE 64

// max number of decoded packet queues
#define PACKETIZER_MAX_PACKET_QUEUE_SIZE 1
// max data bytes in packet
#define PACKETIZER_MAX_PACKET_BINARY_SIZE 96
// max number of callback for one stream
#define PACKETIZER_MAX_CALLBACK_QUEUE_SIZE 3
// max number of streams
#define PACKETIZER_MAX_STREAM_MAP_SIZE 1

#define MSGPACKETIZER_DEBUGLOG_ENABLE
//#define MSGPACKETIZER_ENABLE_STREAM
//#define MSGPACKETIZER_ENABLE_ETHER
#define MSGPACK_MAX_OBJECT_SIZE 64
#include <MsgPacketizer.h>```
hideakitai commented 11 months ago

Are you using Uno R4 minima to reproduce this issue?

KennethThompson commented 11 months ago

Are you using Uno R4 minima to reproduce this issue?

Yes- and esp32 nano, and a uno r4 WiFi. All units have this issue

hideakitai commented 11 months ago

This behavior is correct. The bytes from Fails is

[0x5] [0x4] [0x94] [0x1] [0xce] [0x2] [0x1] [0x7] [0x3] [0xcd] [0x13] [0x88] [0x1] [0x9b] [0x0]

If you decode COBS encoding, it becomes

0x04 0x94 0x01 0xCE 0x00 0x01 0x00 0x03 0xCD 0x13 0x88 0x01 0x9B

and removing index and CRC bytes,

0x94 0x01 0xCE 0x00 0x01 0x00 0x03 0xCD 0x13 0x88 0x01

This is correctly encoded to msgpack. MsgPack encodes integers NOT by TYPE, but by VALUE for efficiency. So 0x1001 is encoded as uint16, and 0x10003 is encoded as uint32.

You can test encode/decode JSON <-> msgpack here. Your Fails struct is expressed in JSON as follows.

[1, 65539, 5000, 1]

https://kawanet.github.io/msgpack-lite/

hideakitai commented 11 months ago

The bytes look correct, but... what was the problem? You couldn't receive the packet correctly?

KennethThompson commented 11 months ago

The bytes look correct, but... what was the problem? You couldn't receive the packet correctly?

The length byte is 0x5, which is incorrect. When I receive the length of 0x5, then the packet does not get read correctly as it is much longer than 5 bytes.

KennethThompson commented 11 months ago

The bytes look correct, but... what was the problem? You couldn't receive the packet correctly?

The length byte is 0x5, which is incorrect. When I receive the length of 0x5, then the packet does not get read correctly as it is much longer than 5 bytes.

[0x5] <--- This should be greater than 5, there are 15 bytes in this array [0x4] [0x94] [0x1] [0xce] [0x2] [0x1] [0x7] [0x3] [0xcd] [0x13] [0x88] [0x1] [0x9b] [0x0]

KennethThompson commented 11 months ago

Here is another sketch that shows the bug without requiring a serial interface:

// #define MSGPACKETIZER_DEBUGLOG_ENABLE
#include <MsgPacketizer.h>

char packetBuffer[512];  // buffer to hold incoming packet,

  struct Fails {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x10003;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }f;

  struct Works {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x1001;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }w;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
      // You can use Ethernet.init(pin) to configure the CS pin

}

void loop() {
    const auto& packet = MsgPacketizer::encode( 1, f);
    Serial.print("FAILS: ");
    for( int x = 0; x < packet.data.size(); x++ )
    {
      Serial.print(" 0x");
      Serial.print(packet.data.data()[x], HEX);
    }
    Serial.println("");

    const auto& packet2 = MsgPacketizer::encode( 1, w);
    Serial.print("WORKS: ");
    for( int x = 0; x < packet2.data.size(); x++ )
    {
      Serial.print(" 0x");
      Serial.print(packet2.data.data()[x], HEX);
    }
    Serial.println("");
    delay(2000);

}

21:14:31.686 -> FAILS: 0x5 0x1 0x148 0x1 0x206 0x2 0x1 0x7 0x3 0x205 0x19 0x136 0x1 0x155 0x0 21:14:31.686 -> WORKS: 0x12 0x1 0x148 0x1 0x205 0x16 0x1 0x205 0x19 0x136 0x1 0x96 0x0

KennethThompson commented 11 months ago

Here is another sketch that shows the bug without requiring a serial interface:

// #define MSGPACKETIZER_DEBUGLOG_ENABLE
#include <MsgPacketizer.h>

char packetBuffer[512];  // buffer to hold incoming packet,

  struct Fails {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x10003;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }f;

  struct Works {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x1001;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }w;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
      // You can use Ethernet.init(pin) to configure the CS pin

}

void loop() {
    const auto& packet = MsgPacketizer::encode( 1, f);
    Serial.print("FAILS: ");
    for( int x = 0; x < packet.data.size(); x++ )
    {
      Serial.print(" 0x");
      Serial.print(packet.data.data()[x], HEX);
    }
    Serial.println("");

    const auto& packet2 = MsgPacketizer::encode( 1, w);
    Serial.print("WORKS: ");
    for( int x = 0; x < packet2.data.size(); x++ )
    {
      Serial.print(" 0x");
      Serial.print(packet2.data.data()[x], HEX);
    }
    Serial.println("");
    delay(2000);

}

21:14:31.686 -> FAILS: 0x5 0x1 0x148 0x1 0x206 0x2 0x1 0x7 0x3 0x205 0x19 0x136 0x1 0x155 0x0 21:14:31.686 -> WORKS: 0x12 0x1 0x148 0x1 0x205 0x16 0x1 0x205 0x19 0x136 0x1 0x96 0x0

I can further confirm that my uno wifi r4, minima, and ESP32 nano are each showing the bug when using the sketch I provided above.

hideakitai commented 11 months ago

The length byte is 0x5, which is incorrect. When I receive the length of 0x5, then the packet does not get read correctly as it is much longer than 5 bytes.

The first byte (0x05) is not the length byte. It shows the position of the first 0x00 byte that appears in your msgpack packet. It is a part of COBS encoding. Please refer https://en.wikipedia.org/wiki/Consistent_Overhead_Byte_Stuffing

So your received packet (COBS encoded [index + msgpack + CRC8])

[0x5] [0x4] [0x94] [0x1] [0xce] [0x2] [0x1] [0x7] [0x3] [0xcd] [0x13] [0x88] [0x1] [0x9b] [0x0]

becomes [index + msgpack + CRC8] as follows bytes by decoding COBS encoding

0x04 0x94 0x01 0xCE 0x00 0x01 0x00 0x03 0xCD 0x13 0x88 0x01 0x9B

and by removing index and CRC bytes, you can get msgpack

0x94 0x01 0xCE 0x00 0x01 0x00 0x03 0xCD 0x13 0x88 0x01
KennethThompson commented 11 months ago

The length byte is 0x5, which is incorrect. When I receive the length of 0x5, then the packet does not get read correctly as it is much longer than 5 bytes.

The first byte (0x05) is not the length byte. It shows the position of the first 0x00 byte that appears in your msgpack packet. It is a part of COBS encoding. Please refer https://en.wikipedia.org/wiki/Consistent_Overhead_Byte_Stuffing

So your received packet (COBS encoded [index + msgpack + CRC8])

[0x5] [0x4] [0x94] [0x1] [0xce] [0x2] [0x1] [0x7] [0x3] [0xcd] [0x13] [0x88] [0x1] [0x9b] [0x0]

becomes [index + msgpack + CRC8] as follows bytes by decoding COBS encoding

0x04 0x94 0x01 0xCE 0x00 0x01 0x00 0x03 0xCD 0x13 0x88 0x01 0x9B

and by removing index and CRC bytes, you can get msgpack

0x94 0x01 0xCE 0x00 0x01 0x00 0x03 0xCD 0x13 0x88 0x01

Ah.. I see. Perhaps I should read the COBS formatting before I assume what the leading byte means. So on my python side, I just need to decode the zeros and then I am good to go. I assume the CRC value is being calculated off the COBS bytes before the decode is occuring?

I appreciate your assistance, and pointing out where I am failing to understand the basic operating principle of COBS!

KennethThompson commented 11 months ago

I truly look forward to this issue being closed and not advertising how dumb I can be. Perhaps someone else will stumble into the issues area and benefit from a read of this dialog.

hideakitai commented 11 months ago

I was just about to add examples of MsgPacketizer in various languages, but you would like to communicate with Python. If so, I'll try to make a Python example first.

KennethThompson commented 11 months ago

I was just about to add examples of MsgPacketizer in various languages, but you would like to communicate with Python. If so, I'll try to make a Python example first.

My python code is about to be operational with the cobs decode. Ill give you my code to speed it up- its using a MsgPack library developed for python

KennethThompson commented 11 months ago

g index and CRC bytes, you can get msgpack

When you are computing the CRC for MsgPacketizer output- do you perform the CRC calculation on the bytes pre-COBS or after performing COBS? Please advise

hideakitai commented 11 months ago

Pre-COBS. COBS is the final encoding.

KennethThompson commented 11 months ago

Excellent. I'll have some example Python code for you to utilize as you wish

On Wed, Nov 15, 2023 at 5:58 PM Hideaki Tai @.***> wrote:

Pre-COBS. COBS is the final encoding.

— Reply to this email directly, view it on GitHub https://github.com/hideakitai/MsgPacketizer/issues/15#issuecomment-1813399783, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4HTDI7BB6GRCBU5IK24GLYEVCKZAVCNFSM6AAAAAA7FQXAZWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTGM4TSNZYGM . You are receiving this because you authored the thread.Message ID: @.***>

KennethThompson commented 11 months ago

Pre-COBS. COBS is the final encoding.

As promised, here is a python script that demonstrates how to receive a MsgPacketizer frame output from an Arduino's serial port and verify the CRC before outputting the COBS-decoded bytes. You have my permission to do whatever you like with this code.

import traceback
import serial
from cobs import cobs
import msgpack
import crc8

#connection = '/dev/ttyACM0'
connection = '/dev/cu.usbmodem101'
arduino = serial.Serial(connection, 115200, timeout=1, xonxoff=False, rtscts=False, dsrdtr=True)

recv_buffer = bytes()

while True:
    try:
        data = arduino.read()
        if data == b'\x00': # MsgPacketizer framed received
            recv_buffer += bytearray(data)
            strb = ''
            for b in bytes(recv_buffer):
                strb += f'[{hex(b)}]'
            print(strb) # Show the received bytes

            messageType = recv_buffer[1]

            data_bytes = recv_buffer[2:]

            crc = recv_buffer[-2]

            data_bytes = recv_buffer[2:]
            data_bytes = data_bytes[:-2]
            combined = (len(data_bytes)+1).to_bytes(1, 'big') + data_bytes 
            decoded = cobs.decode(combined) 

            hash = crc8.crc8()
            hash.update(decoded)
            if hash.digest() == crc.to_bytes(1,'big'):
                payload = msgpack.unpackb(data_bytes, use_list=True, raw=False)
                print(f'Decode success! Unpacked data: {payload}')
            else:
                print(f"Error. CRC validation failed for received message!")
            recv_buffer = bytes() # reset the receive buffer for the next frame
        elif data == b'\n': # Debug Serial.println received
            recv_buffer += bytearray(data)
            print(bytes(recv_buffer).decode('utf8', errors='ignore'))
            recv_buffer = bytes()
        else:
            recv_buffer += bytearray(data) #Continue to add bytes to array
    except Exception as error:
        just_the_string = traceback.format_exc()
        print(just_the_string)
KennethThompson commented 11 months ago

I need to reopen this one, and perhaps this is my error.

When I send 0x60093 via an encoded uin64_t, the CRC in the encoded packet appears to be incorrect. Sender and receiver code is below. The expected CRC is 0xd4, but the packet indicates 0x2d. Perhaps I am not computing the CRC correctly on the python side? I note that I can decode the packet via COBS and MsgPacketizer, despite the CRC problem..

Thank you!

SENDER:

// #define MSGPACKETIZER_DEBUGLOG_ENABLE
#include <MsgPacketizer.h>

char packetBuffer[512];  // buffer to hold incoming packet,

  struct Fails {
      uint8_t protocolVersion = 1;
      uint64_t featureMap = 0x60093;
      uint32_t timeout = 5000;
      uint8_t boardIndex = 1;
      MSGPACK_DEFINE(protocolVersion, featureMap, timeout, boardIndex); 
  }f;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
      // You can use Ethernet.init(pin) to configure the CS pin

}

void loop() {
    const auto& packet = MsgPacketizer::encode( 1, f);
    Serial.print("FAILS: ");
    for( int x = 0; x < packet.data.size(); x++ )
    {
      Serial.print(" 0x");
      Serial.print(packet.data.data()[x], HEX);
    }
    Serial.println("");
    Serial.write(packet.data.data(), packet.data.size());
    delay(2000);

}

RECEIVER:

import serial
from cobs import cobs
import msgpack
import crc8

#connection = '/dev/ttyACM0'
connection = '/dev/cu.usbmodem3485187ABE842'
arduino = serial.Serial(connection, 115200, timeout=1, xonxoff=False, rtscts=False, dsrdtr=True)

recv_buffer = bytes()

while True:
    try:
        data = arduino.read()
        if data == b'\x00': # MsgPacketizer framed received
            recv_buffer += bytearray(data)
            strb = ''
            for b in bytes(recv_buffer):
                strb += f'[{hex(b)}]'
            print(strb) # Show the received bytes

            messageType = recv_buffer[1]

            data_bytes = recv_buffer[2:]

            crc = recv_buffer[-2]

            data_bytes = recv_buffer[2:]
            data_bytes = data_bytes[:-2]
            combined = (len(data_bytes)+1).to_bytes(1, 'big') + data_bytes 
            decoded = cobs.decode(combined) 

            hash = crc8.crc8()
            hash.update(decoded)
            if hash.digest() == crc.to_bytes(1,'big'):
                payload = msgpack.unpackb(data_bytes, use_list=True, raw=False)
                print(f'Decode success! Unpacked data: {payload}')
            else:
                print(f"Error. CRC validation failed for received message! Computed CRC = {hash.digest()}")
            recv_buffer = bytes() # reset the receive buffer for the next frame
        elif data == b'\n': # Debug Serial.println received
            recv_buffer += bytearray(data)
            print(bytes(recv_buffer).decode('utf8', errors='ignore'))
            recv_buffer = bytes()
        else:
            recv_buffer += bytearray(data) #Continue to add bytes to array
    except Exception as error:
        just_the_string = traceback.format_exc()
        print(just_the_string)
hideakitai commented 11 months ago

It would be best not to use Serial together for MsgPacketizer and log output. (the bytes will be mixed and cause a decoding error)

void loop() {
    const auto& packet = MsgPacketizer::encode( 1, f);
    // Serial.print("FAILS: ");
    // for( int x = 0; x < packet.data.size(); x++ )
    // {
    //   Serial.print(" 0x");
    //   Serial.print(packet.data.data()[x], HEX);
    // }
    // Serial.println("");
    Serial.write(packet.data.data(), packet.data.size());
    delay(2000);

}

This works with my python (WIP) code

DEBUG:root:bytes in_waiting: 15
DEBUG:root:feed: b'\x05\x01\x94\x01\xce\x02\x06\x07\x93\xcd\x13\x88\x01-\x00'
DEBUG:root:cached bytes: bytearray(b'\x05\x01\x94\x01\xce\x02\x06\x07\x93\xcd\x13\x88\x01-\x00')
DEBUG:root:cobs encoded: bytearray(b'\x05\x01\x94\x01\xce\x02\x06\x07\x93\xcd\x13\x88\x01-'), rest: bytearray(b'')
DEBUG:root:cobs decoded: b'\x01\x94\x01\xce\x00\x06\x00\x93\xcd\x13\x88\x01-'
DEBUG:root:index: 1, data: b'\x94\x01\xce\x00\x06\x00\x93\xcd\x13\x88\x01', crc: b'-'
DEBUG:root:calculated crc: b'-'
DEBUG:root:msgpack decoded to: [1, 393363, 5000, 1]
DEBUG:root:received msg = index: 1, msg: [1, 393363, 5000, 1]
callback 0x01: msg = [1, 393363, 5000, 1]

my python code (WIP):

import argparse
import logging
import traceback

import crc8
import msgpack
import serial
from cobs import cobs

class MsgPacketizer:
    def __init__(self):
        self._recv_bytes = bytearray()
        self._recv_msgs = {}
        self._callbacks = {}

    def feed(self, buffer: bytearray):
        logging.debug(f"feed: {buffer}")
        self._recv_bytes += buffer
        logging.debug(f"cached bytes: {self._recv_bytes}")
        # find delimiter (0x00)
        while self._recv_bytes.find(b"\x00") != -1:
            # if delimiter found, split and process buffer one by one
            [chunk, self._recv_bytes] = self._recv_bytes.split(b"\x00", maxsplit=1)
            logging.debug(f"cobs encoded: {chunk}, rest: {self._recv_bytes}")
            # decode cobs (NOTE: 0x00 on last byte is not required)
            decoded = cobs.decode(chunk)
            logging.debug(f"cobs decoded: {decoded}")
            # devide into idnex, data, crc
            index = decoded[0]
            data = decoded[1:-1]
            crc = decoded[-1].to_bytes(1, byteorder="big")
            logging.debug(f"index: {index}, data: {data}, crc: {crc}")
            # check crc8
            hash = crc8.crc8()
            hash.update(data)
            digest = hash.digest()
            logging.debug(f"calculated crc: {digest}")
            if digest == crc:
                # unpack msgpack
                payload = msgpack.unpackb(data, use_list=True, raw=False)
                logging.debug(f"msgpack decoded to: {payload}")
                self._recv_msgs[index] = payload
            else:
                logging.warn(f"crc not mathcd (recv: {crc}, calc: {digest})")
                continue

        # iterate over received messages and print them and remove them from buffer
        for index, msg in self._recv_msgs.items():
            # do some stuff in user-defined callback function
            logging.debug(f"received msg = index: {index}, msg: {msg}")
            if (index in self._callbacks) and (self._callbacks[index] is not None):
                self._callbacks[index](msg)

        self._recv_msgs.clear()

    def subscribe(self, index: int, callback: callable):
        self._callbacks[index] = callback

# logging.basicConfig(level=logging.INFO)
logging.basicConfig(level=logging.DEBUG)

# -p, --port /dev/tty.usbserial-0185D96C
# -b, --baudrate 115200
argparser = argparse.ArgumentParser()
argparser.add_argument("port", help="serial port")
argparser.add_argument("-b", "--baudrate", default=115200, help="baudrate")
args = argparser.parse_args()

arduino = serial.Serial(args.port, args.baudrate)

msgpacketizer = MsgPacketizer()
msgpacketizer.subscribe(0x01, lambda msg: print(f"callback 0x01: msg = {msg}"))

while True:
    try:
        num_bytes = arduino.in_waiting
        if num_bytes > 0:
            logging.debug(f"bytes in_waiting: {num_bytes}")
            msgpacketizer.feed(arduino.read(num_bytes))

    except KeyboardInterrupt:
        print("KeyboardInterrupt")
        break

    except Exception:
        just_the_string = traceback.format_exc()
        logging.error(just_the_string)

arduino.close()
KennethThompson commented 11 months ago

It would be best not to use Serial together for MsgPacketizer and log output. (the bytes will be mixed and cause a decoding error)

void loop() {
    const auto& packet = MsgPacketizer::encode( 1, f);
    // Serial.print("FAILS: ");
    // for( int x = 0; x < packet.data.size(); x++ )
    // {
    //   Serial.print(" 0x");
    //   Serial.print(packet.data.data()[x], HEX);
    // }
    // Serial.println("");
    Serial.write(packet.data.data(), packet.data.size());
    delay(2000);

}

This works with my python (WIP) code

DEBUG:root:bytes in_waiting: 15
DEBUG:root:feed: b'\x05\x01\x94\x01\xce\x02\x06\x07\x93\xcd\x13\x88\x01-\x00'
DEBUG:root:cached bytes: bytearray(b'\x05\x01\x94\x01\xce\x02\x06\x07\x93\xcd\x13\x88\x01-\x00')
DEBUG:root:cobs encoded: bytearray(b'\x05\x01\x94\x01\xce\x02\x06\x07\x93\xcd\x13\x88\x01-'), rest: bytearray(b'')
DEBUG:root:cobs decoded: b'\x01\x94\x01\xce\x00\x06\x00\x93\xcd\x13\x88\x01-'
DEBUG:root:index: 1, data: b'\x94\x01\xce\x00\x06\x00\x93\xcd\x13\x88\x01', crc: b'-'
DEBUG:root:calculated crc: b'-'
DEBUG:root:msgpack decoded to: [1, 393363, 5000, 1]
DEBUG:root:received msg = index: 1, msg: [1, 393363, 5000, 1]
callback 0x01: msg = [1, 393363, 5000, 1]

my python code (WIP):

import argparse
import logging
import traceback

import crc8
import msgpack
import serial
from cobs import cobs

class MsgPacketizer:
    def __init__(self):
        self._recv_bytes = bytearray()
        self._recv_msgs = {}
        self._callbacks = {}

    def feed(self, buffer: bytearray):
        logging.debug(f"feed: {buffer}")
        self._recv_bytes += buffer
        logging.debug(f"cached bytes: {self._recv_bytes}")
        # find delimiter (0x00)
        while self._recv_bytes.find(b"\x00") != -1:
            # if delimiter found, split and process buffer one by one
            [chunk, self._recv_bytes] = self._recv_bytes.split(b"\x00", maxsplit=1)
            logging.debug(f"cobs encoded: {chunk}, rest: {self._recv_bytes}")
            # decode cobs (NOTE: 0x00 on last byte is not required)
            decoded = cobs.decode(chunk)
            logging.debug(f"cobs decoded: {decoded}")
            # devide into idnex, data, crc
            index = decoded[0]
            data = decoded[1:-1]
            crc = decoded[-1].to_bytes(1, byteorder="big")
            logging.debug(f"index: {index}, data: {data}, crc: {crc}")
            # check crc8
            hash = crc8.crc8()
            hash.update(data)
            digest = hash.digest()
            logging.debug(f"calculated crc: {digest}")
            if digest == crc:
                # unpack msgpack
                payload = msgpack.unpackb(data, use_list=True, raw=False)
                logging.debug(f"msgpack decoded to: {payload}")
                self._recv_msgs[index] = payload
            else:
                logging.warn(f"crc not mathcd (recv: {crc}, calc: {digest})")
                continue

        # iterate over received messages and print them and remove them from buffer
        for index, msg in self._recv_msgs.items():
            # do some stuff in user-defined callback function
            logging.debug(f"received msg = index: {index}, msg: {msg}")
            if (index in self._callbacks) and (self._callbacks[index] is not None):
                self._callbacks[index](msg)

        self._recv_msgs.clear()

    def subscribe(self, index: int, callback: callable):
        self._callbacks[index] = callback

# logging.basicConfig(level=logging.INFO)
logging.basicConfig(level=logging.DEBUG)

# -p, --port /dev/tty.usbserial-0185D96C
# -b, --baudrate 115200
argparser = argparse.ArgumentParser()
argparser.add_argument("port", help="serial port")
argparser.add_argument("-b", "--baudrate", default=115200, help="baudrate")
args = argparser.parse_args()

arduino = serial.Serial(args.port, args.baudrate)

msgpacketizer = MsgPacketizer()
msgpacketizer.subscribe(0x01, lambda msg: print(f"callback 0x01: msg = {msg}"))

while True:
    try:
        num_bytes = arduino.in_waiting
        if num_bytes > 0:
            logging.debug(f"bytes in_waiting: {num_bytes}")
            msgpacketizer.feed(arduino.read(num_bytes))

    except KeyboardInterrupt:
        print("KeyboardInterrupt")
        break

    except Exception:
        just_the_string = traceback.format_exc()
        logging.error(just_the_string)

arduino.close()

Thank you for the example code! I will parse through and identify why my CRC validation was failing. Although I gave you the example using Serial, I was having the same issue when using UDP. However, I believe I can already see how you are decoding the message in a manner that is different from my implementation. Thank you for all your assistance

hideakitai commented 11 months ago

If you still need help, please reopen this issue.

KennethThompson commented 9 months ago

Nevermind- I reacted too quickly, the problem was mine