Malformed serial input

lyka0 · July 28, 2021, 11:56pm

I've been pulling my hair out trying to figure out what's wrong with my setup/code here. I have an Arduino UNO controlling a 5M RGB LED strip connected to a PC with a custom application to send serial data to control it. For the purpose of testing though, I'm using the Arduino serial monitor to send data.

This malformed data only occurs sometimes and usually when I have the 'rainbow' field in the JSON string set to true and I send another string with something else changed (eg. brightness, rgb values etc).

Here's an example of a JSON string I send:

<{"red":255,"green":0,"blue":0,"brightness":100,"rainbow":true}>

and the output I expect and most of the time receive:

<{"red":255,"green":0,"blue":0,"brightness":100,"rainbow":true}>
09:46:57.779 -> {"red":255,"green":0,"blue":0,"brightness":100,"rainbow":true}
09:46:57.779 -> RED: 255
09:46:57.779 -> GREEN: 0
09:46:57.779 -> BLUE: 0
09:46:57.779 -> BRIGHTNESS: 100
09:46:57.779 -> RAINBOW: 1

Unfortunately I get malformed data sometimes. Here's an example:

I sent this JSON string:

<{"red":255,"green":0,"blue":0,"brightness":100,"rainbow":true}>

and got this as output:

09:47:04.780 -> <{ss":100,"rainbow":true}>
09:47:04.780 -> {ss":100,"rainbow":true}
09:47:04.780 -> RED: 0
09:47:04.780 -> GREEN: 0
09:47:04.780 -> BLUE: 0
09:47:04.780 -> BRIGHTNESS: 0
09:47:04.780 -> RAINBOW: 0

Here's my full sketch code:

#include <Adafruit_NeoPixel.h>
#include <ArduinoJson.h>

#define MAX_PACKET_LENGTH 128

const uint8_t led_pin = 8;
const int led_count = 141;
const char begin_char = '<';
const char end_char = '>';

uint8_t red = 0;
uint8_t green = 0;
uint8_t blue = 0;
uint8_t brightness = 255;
bool rainbow = false;

Adafruit_NeoPixel led_strip;

void setup() {
  Serial.begin(115200);

  Serial.println("UNO!");
  
  led_strip = Adafruit_NeoPixel(led_count, led_pin, NEO_GRB + NEO_KHZ800);
  led_strip.begin();
  led_strip.clear();
  led_strip.setBrightness(brightness);
  led_strip.show();
}

void loop() {
  led_strip.fill(led_strip.Color(red, green, blue));
  led_strip.setBrightness(brightness);
  led_strip.show();

  static char data[MAX_PACKET_LENGTH];

  memset(&data, 0, MAX_PACKET_LENGTH);

  int count = 0;
  while (data[count] != end_char) {
    if (Serial.available() > 0) {
      char data_t = Serial.read();

      if (data_t == begin_char) {
        count = 0;
        memset(&data, 0, MAX_PACKET_LENGTH);

      } else {
        ++count;
      }

      data[count] = data_t;
    }
  }

  if (data[0] != begin_char) {
    return;
  }

  data[count + 1] = '\0';

  Serial.println(data);

  memmove(&data[0], &(data[1]), strlen(&(data[1])));
  data[strlen(data) - 2] = '\0';

  Serial.println(data);

  StaticJsonDocument<80> doc;

  DeserializationError error = deserializeJson(doc, data);

  red = doc["red"];
  green = doc["green"];
  blue = doc["blue"];
  brightness = doc["brightness"];
  rainbow = doc["rainbow"];

  Serial.print("RED: "); Serial.println(red);
  Serial.print("GREEN: "); Serial.println(green);
  Serial.print("BLUE: "); Serial.println(blue);
  Serial.print("BRIGHTNESS: "); Serial.println(brightness);
  Serial.print("RAINBOW: "); Serial.println(rainbow);

  if (rainbow) {

    while (true) {
      for (long first_hue = 0; first_hue < 5 * 65536; first_hue += 256) {
        for (int i = 0; i < led_strip.numPixels(); ++i) {
          int hue = first_hue + (i * 65536L / led_strip.numPixels());
          led_strip.setPixelColor(i, led_strip.gamma32(led_strip.ColorHSV(hue)));
        }

        led_strip.show();
        delay(5);

        if (Serial.available() > 0) {
          Serial.println("BREAKING OUT");
          return;
        }

      }
    }
  }
  
}

I'm sure I'm missing something obvious here but I haven't been able to figure it out.

Cheers

ruilviana · July 29, 2021, 1:03am

Hi @lyka0

The problem lies in the complexity of the routine inside if(rainbow).
Just as an experience comment (use // in all lines) all these lines of this if and you will see that it does not fail anymore.

What is the need to test the serial within this routine?

RV mineirin

lyka0 · July 29, 2021, 1:12am

As the rainbow routine relies on a while(true) to continuously run, if this routine was running, any further serial commands would do nothing.

For example, if I wanted to turn off the rainbow effect after the routine had been running, without testing the serial no further commands would be registered as the serial parsing routine at the beginning would never run.

Why exactly does the rainbow routine cause this malformed data?

ruilviana · July 29, 2021, 1:13am

Hi
If this routine is to interrupt rainbow processing, then I suggest you use void serialEvent() :
Ref:
https://arduinogetstarted.com/en/reference/serial-serialevent

RV mineirin

lyka0 · July 29, 2021, 1:22am

I don't believe serialEvent() would be suitable here as it is only called at the end of the loop routine as stated in the documentation: This function is automatically called at the end of loop()

anon73444976 · July 29, 2021, 7:42am

A simple "return" would suffice, it seems to me.

TomGeorge · July 29, 2021, 7:45am

Hi, @lyka0

How are you powering your project?
Can you post a circuit diagram that shows power supply and pin names/labels.

Do you have a DMM?

Thanks.. Tom..

lyka0 · July 29, 2021, 7:55am

Haha I completely missed that. I had that goto point setup not at the very beginning of the loop at first and completely forgot I could just return; once I moved it to the beginning.

johnwasser · July 29, 2021, 3:37pm

You are running on an UNO, right? If you this run on a 32-bit processor like an ESP8266 or ESP32 you need at least 80 bytes and the ArduinoJson folks recommend 96. (On an 8-bit Arduino they say 40 is the minimum and they recommend 48 for this document.)

I wonder if adding this after the deserialize would provide useful information:

if (error) {
  Serial.print(F("deserializeJson() failed: "));
  Serial.println(error.f_str());
  return;
}

lyka0 · July 29, 2021, 11:01pm

I don't really think so as the data is malformed before any JSON processing. Though, you did remind me to handle that potential error

johnwasser · July 30, 2021, 2:04pm

Maybe move the BREAK OUT up into the nested loops so there will be less delay in detect a new command arriving. Perhaps the delay is causing the serial buffer to overflow? It's something to try.

 if (rainbow)
  {
    while (true)
    {
      for (long first_hue = 0; first_hue < 5 * 65536; first_hue += 256)
      {
        for (int i = 0; i < led_strip.numPixels(); ++i)
        {
          int hue = first_hue + (i * 65536L / led_strip.numPixels());
          led_strip.setPixelColor(i, led_strip.gamma32(led_strip.ColorHSV(hue)));
          if (Serial.available() > 0)
          {
            Serial.println("BREAKING OUT");
            return;
          }
        }

        led_strip.show();
        delay(5);
      }
    }
  }

What is sending the serial input? Could there be a problem in the sending? Is it always the same part of the message that is missing?

anon73444976 · July 30, 2021, 2:05pm

I'd play safe "5L * 65536"

johnwasser · July 30, 2021, 2:45pm

I'd use 5ul << 16 or 0x050000ul to make it clearer for those who don't know that 65536 is 2¹⁶.

system · November 27, 2021, 2:46pm

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.