Skip to content

Remove newline requirement for serial json parsing#45

Merged
jonnew merged 2 commits intomainfrom
fix-serial-parsing
Apr 2, 2026
Merged

Remove newline requirement for serial json parsing#45
jonnew merged 2 commits intomainfrom
fix-serial-parsing

Conversation

@cjsha
Copy link
Copy Markdown
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
Copy Markdown
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
Copy link
Copy Markdown
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.

I tested with a bunch of weird strings.

  • Nested jsons
  • Missing brackets followed by valid
  • Invalid JSON
  • Conflicting commands
  • Every form of newline

Seemed to work well!

@jonnew jonnew merged commit 3e5472a into main Apr 2, 2026
@jonnew jonnew deleted the fix-serial-parsing branch April 2, 2026 18:59
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