Skip to content

Remove newline requirement for serial json parsing#45

Open
cjsha wants to merge 2 commits intomainfrom
fix-serial-parsing
Open

Remove newline requirement for serial json parsing#45
cjsha wants to merge 2 commits intomainfrom
fix-serial-parsing

Conversation

@cjsha
Copy link
Member

@cjsha cjsha commented Mar 6, 2026

  • Rewrite serial buffer builder to not need newline termination:
    • Let ArduinoJson decide if serial buffer is terminated
    • Ignore characters preceding {
      • This prevents DeserializeJson from parsing strings or arrays instead of JSON objects (i.e. handles the case when a double quotes " or open square bracket [ precedes your JSON which would otherwise brick serial inputs until the string or array is terminated)
      • An alternative is to rollback ArduinoJson to v5 and instead use the ParseObject method
    • Restart buffer if new { is received
      • This prevents a singular open curly bracket '{' from bricking serial inputs. This is a proposed improvement from

I tried using std::cin instead of constructing our own buffer. There were some problems associated with that.

Originally, the process_serial_commands was only called when a new character was received from the serial port. This meant that if I sent a message like "a{turn:1}" or "{enable:true}{turn:1}", only the first item would be parses (in the first case, an invalid input "a"; in the second case, a valid JSON), and the second item wouldn't be parsed until another character was received. If I remove the guard, std::cin would block when it's empty, and there doesn't seem to be any standard way to check if std::cin is empty without blocking.

- Rewrite serial buffer builder to not need newline termination:
  - Let ArduinoJson decide if serial buffer is terminated
  - Remove characters preceding '{'
  - Restart buffer if new '{' is received
@cjsha cjsha requested a review from jonnew March 6, 2026 16:02
Copy link
Member

@jonnew jonnew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be something that flushes standard in an resets the serial_buffer when the remote_avialabel transitions from low to high, e.g.


static int process_serial_commands(context_t *ctx)
{
    static char serial_buffer[MAX_SERIAL_BUFFER_LENGTH] = {0};
    static uint16_t serial_buffer_index = 0;
    static bool last_remote_available = false;

    if (!last_remote_available && remote_available(ctx))
    {
        // TODO: reset buffer and flush stdin
        last_remote_available = true;

    } else {
        last_remote_available = false;
        return ERROR_REMOTE_INTERFACE_LOCKED;
    }


or perhaps something a bit less dumb

@cjsha cjsha requested a review from jonnew March 9, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants