From af3dc61710595f8fc6168bf1107a1f5a6701873f Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Fri, 26 Aug 2022 21:03:33 -0500 Subject: [PATCH] Focus: correctly handle delayed newline Previously, a newline that wasn't read with the rest of the command in a single event would get appended to the command buffer instead of remaining in the "peek" buffer, so the command dispatch would never fire. Stash a space delimiter instead of storing it in the buffer, only to overwrite it later. Fix an off-by-one error that could cause a buffered command to not be null-terminated. This probably didn't cause a problem in practice, because Focus plugins mostly did a `strcmp` against a fixed string that is smaller than the command buffer size. Signed-off-by: Taylor Yu --- .../src/kaleidoscope/plugin/FocusSerial.cpp | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/plugins/Kaleidoscope-FocusSerial/src/kaleidoscope/plugin/FocusSerial.cpp b/plugins/Kaleidoscope-FocusSerial/src/kaleidoscope/plugin/FocusSerial.cpp index 70db42cd..486ad022 100644 --- a/plugins/Kaleidoscope-FocusSerial/src/kaleidoscope/plugin/FocusSerial.cpp +++ b/plugins/Kaleidoscope-FocusSerial/src/kaleidoscope/plugin/FocusSerial.cpp @@ -33,6 +33,7 @@ namespace kaleidoscope { namespace plugin { EventHandlerResult FocusSerial::afterEachCycle() { + int c; // GD32 doesn't currently autoflush the very last packet. So manually flush here Runtime.serialPort().flush(); // If the serial buffer is empty, we don't have any work to do @@ -41,33 +42,28 @@ EventHandlerResult FocusSerial::afterEachCycle() { } do { - command_[buf_cursor_++] = Runtime.serialPort().read(); - } while (command_[buf_cursor_ - 1] != SEPARATOR && buf_cursor_ < sizeof(command_) && Runtime.serialPort().available() && (Runtime.serialPort().peek() != NEWLINE)); - - - // If there was no command, there's nothing to do - if (command_[0] == '\0') { - buf_cursor_ = 0; - memset(command_, 0, sizeof(command_)); - return EventHandlerResult::OK; - } + // If there's a newline pending, don't read it + if (Runtime.serialPort().peek() == NEWLINE) { + break; + } + c = Runtime.serialPort().read(); + // Don't store the separator; just stash it + if (c == SEPARATOR) { + break; + } + command_[buf_cursor_++] = c; + } while (buf_cursor_ < (sizeof(command_) - 1) && Runtime.serialPort().available()); - if ((command_[buf_cursor_ - 1] != SEPARATOR) && (Runtime.serialPort().peek() != NEWLINE) && buf_cursor_ < sizeof(command_)) { + if ((c != SEPARATOR) && (Runtime.serialPort().peek() != NEWLINE) && buf_cursor_ < (sizeof(command_) - 1)) { // We don't have enough command to work with yet. // Let's leave the buffer around for another cycle return EventHandlerResult::OK; } - // If this was a command with a space-delimited payload, - // strip the space delimiter off - if ((command_[buf_cursor_ - 1] == SEPARATOR)) { - command_[buf_cursor_ - 1] = '\0'; - } - // Then process the command Runtime.onFocusEvent(command_); while (Runtime.serialPort().available()) { - char c = Runtime.serialPort().read(); + c = Runtime.serialPort().read(); if (c == NEWLINE) { // newline serves as an end-of-command marker // don't drain the buffer past there