hideakitai / MsgPacketizer

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

Re: Odd bug with string encoding; MsgPacketizer randomly outputting messages without length indicator.. #14

Closed KennethThompson closed 9 months ago

KennethThompson commented 10 months ago

I've been using your library in an Arduino project and its awesome- thank you!

However, I encountered a bug that is recreated below. When sending the 'Works' structure, the bytes output via the serial port are as follows (Note, I am omitting the 0x00 byte at the end): [0xb][0x4][0x92][0xa5][0x49][0x33][0x3a][0x30][0x30][0x1][0xe5]

This is as expected.

However, second structure ('Fails') is similar, but changes the string to "I3:0", and the bytes over the serial are as follows: [0x4][0x92][0xa4][0x49][0x33][0x3a][0x30][0x1][0x2b]

As you can see, the 'Fails' structure gets output to the Serial port without the length byte prefixing the data.

What is very strange is if I change "I3:0" to just "I3:", the output is then correct and includes the length byte.

The appears to be some bug with the string encoding portion as the expected behavior is an array of bytes including a length, index, payload, and finally CRC.

Steps to recreate:

  1. Flash sketch below to Arduino
  2. Monitor received bytes, or use your library calls to dump the bytes which would be written to the wire
  3. Observe that the 'Fails' structure is causing unexpected behavior.

Thanks again!

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

struct Works {
    String s = "I3:00";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}w;

struct Fails {
    String s = "I3:0";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}f;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
}

void loop() {
    // must be called to trigger callback and publish data
    MsgPacketizer::send(Serial, some_index, w);
    delay(1000);
    MsgPacketizer::send(Serial, some_index, f);
    delay(1000);
    MsgPacketizer::update();
}
hideakitai commented 10 months ago

Thanks! I'll see it later

hideakitai commented 10 months ago

Which board do you use? (Uno, ESP32, Teensy, STM32, etc.) If you use Uno (or another AVR/SAMD board), could you retry after updating ArxContainer to the latest v0.5.1?

KennethThompson commented 10 months ago

Which board do you use? (Uno, ESP32, Teensy, STM32, etc.) If you use Uno (or another AVR/SAMD board), could you retry after updating ArxContainer to the latest v0.5.1?

I used a Uno Minima R4, and a Uno Wifi R4 to reproduce the bug I detailed. These new-er boards use a different processor than AVR/SAMD, so I am not sure if you want me to try something different?

I also have a Nano ESP32, and Grand Central M4- but I did not test on those.

hideakitai commented 10 months ago

If your board does not use C++ standard libraries (vector, map, etc.), MsgPacketizer uses my container implementation instead (ArxContainer). There were some bugs in ArxContainer v0.5.0, and fixed in v0.5.1.

I've tried your sketch on ESP32 (that uses C++ standard libraries); it worked fine without missing length bytes. I bought Uno R4 WiFi today; I will test your code by this weekend.

hideakitai commented 10 months ago

But I think Uno R4 minima has C++ standard libraries. Hmmm

hideakitai commented 10 months ago

Which version of MsgPacketizer do you use? The latest release of MsgPacketizer cannot be compiled with Uno R4 because the latest FastCRC does not support R4 minima yet.

https://github.com/FrankBoesing/FastCRC/pull/34

hideakitai commented 10 months ago

Oh, that's you :-)

KennethThompson commented 10 months ago

I submitted a pull to add in minima, uno wifi and esp32 nano support to fastcrc. I did not test heavily with my esp32 as I just got async udp working. I will run the sketch on my esp32 nano.

Library wise, i am on the latest version that is installed through arduino ide

On Tue, Nov 14, 2023 at 19:12 Hideaki Tai @.***> wrote:

Oh, that's you :-)

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

KennethThompson commented 10 months ago

Will the r4 minima automatically use stl, or is it toggled by an include?

On Tue, Nov 14, 2023 at 19:48 Kenneth Thompson @.***> wrote:

I submitted a pull to add in minima, uno wifi and esp32 nano support to fastcrc. I did not test heavily with my esp32 as I just got async udp working. I will run the sketch on my esp32 nano.

Library wise, i am on the latest version that is installed through arduino ide

On Tue, Nov 14, 2023 at 19:12 Hideaki Tai @.***> wrote:

Oh, that's you :-)

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

hideakitai commented 10 months ago

R4 automatically enables C++ standard libraries in the compile process.

KennethThompson commented 10 months ago

R4 automatically enables C++ standard libraries in the compile process.

I was re-testing tonight to confirm this one and I changed the code as shown below. The bug will not reproduce itself now (using minima/ESP32). I am curious if its related to the use of the Serial port. I will retest with the Serial and follow up.

#include <MsgPacketizer.h>

struct Works {
    String s = "I3:00";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}w;

struct Fails {
    String s = "I3:0";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}f;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
}

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);
    }
    const auto& packet2 = MsgPacketizer::encode( 1, w);
    Serial.println("");
    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);
    //delay(1000);
    MsgPacketizer::update();
}
hideakitai commented 10 months ago

If you are using Serial together with "Logging" and "MsgPacketizer", it may affect the results. Please try to use another serial (Log: Serial, MsgPacketizer: Serial1)

KennethThompson commented 10 months ago

R4 automatically enables C++ standard libraries in the compile process.

I was re-testing tonight to confirm this one and I changed the code as shown below. The bug will not reproduce itself now (using minima/ESP32). I am curious if its related to the use of the Serial port. I will retest with the Serial and follow up.

#include <MsgPacketizer.h>

struct Works {
    String s = "I3:00";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}w;

struct Fails {
    String s = "I3:0";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}f;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
}

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);
    }
    const auto& packet2 = MsgPacketizer::encode( 1, w);
    Serial.println("");
    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);
    //delay(1000);
    MsgPacketizer::update();
}

So I think I've identified what is happening. See the sketch below. When the 0xa is written to the serial port (which is the first byte of the array), the other side receives 0x1. 0xa is a carriage return, so something is getting hung up on that. This is very odd, and I am not sure why. My receiver is a pyserial test loop, and my google searches have yielded zero results on how to fix this.

Man I am feeling dumb here. Not sure what is going on with the serial port.


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

struct Works {
    String s = "I3:00";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}w;

struct Fails {
    String s = "I3:0";
    int i = 1;
    MSGPACK_DEFINE(s, i);
}f;

const uint8_t some_index = 0x4;

void setup() {
    Serial.begin(115200);
    delay(2000);
}

void loop() {
    // must be called to trigger callback and publish data
    //MsgPacketizer::send(Serial, some_index, w);
   // delay(1000);
    //MsgPacketizer::send(Serial, some_index, f);
    delay(1000);
    const auto& packet = MsgPacketizer::encode( 1, f);
     /// Serial.print("FAILS: ");

    Serial.print("BYTES: ");
    for( int x = 0; x < packet.data.size(); x++ )
    {
      Serial.print(" 0x");
      Serial.print(packet.data.data()[x], HEX);
    }
    Serial.println("");

    Serial.write( (uint8_t*)packet.data.data(), packet.data.size());
    Serial.flush();
    delay(1000);
    uint8_t tmp[512];
    memcpy( tmp, packet.data.data(), packet.data.size());
    Serial.write( tmp, packet.data.size());
    Serial.flush();
    //Serial.println("");
    //Serial.write(packet.data.data(), packet.data.size());
    //Serial.flush();
    //MsgPacketizer::update();
}
hideakitai commented 10 months ago

https://github.com/hideakitai/MsgPacketizer/issues/15#issuecomment-1811800361

hideakitai commented 9 months ago

If you still need help, please reopen this issue.