From ec0cf7a04e9d625f20b8d9ec9789f18109bd8273 Mon Sep 17 00:00:00 2001 From: SeaTechRC <153106523+SeaTechRC@users.noreply.github.com> Date: Wed, 26 Jun 2024 12:00:27 +0200 Subject: [PATCH 1/4] Fix Serial::write_checked_line Create make_checked_line, reuse for echo function and fix Serial::write_checked_line. --- main/modules/serial.cpp | 4 +++- main/utils/uart.cpp | 31 +++++++++++++++++++------------ main/utils/uart.h | 1 + 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/main/modules/serial.cpp b/main/modules/serial.cpp index f0f710dc..aa504910 100644 --- a/main/modules/serial.cpp +++ b/main/modules/serial.cpp @@ -50,7 +50,9 @@ size_t Serial::write(const uint8_t byte) const { void Serial::write_checked_line(const char *message, const int length) const { char buffer[1024]; strncpy(buffer, message, length); - int len = check(buffer, length); + + // TODO: Handle buffer overflows + int len = make_checked_line(buffer, length); buffer[len++] = '\n'; uart_write_bytes(this->uart_num, buffer, len); } diff --git a/main/utils/uart.cpp b/main/utils/uart.cpp index 74dd0511..60886d73 100644 --- a/main/utils/uart.cpp +++ b/main/utils/uart.cpp @@ -15,18 +15,9 @@ void echo(const char *format, ...) { pos += std::sprintf(&buffer[pos], "\n"); - uint8_t checksum = 0; - int start = 0; - for (unsigned int i = 0; i < pos; ++i) { - if (buffer[i] == '\n') { - buffer[i] = '\0'; - printf("%s@%02x\n", &buffer[start], checksum); - start = i + 1; - checksum = 0; - } else { - checksum ^= buffer[i]; - } - } + // TODO: Properly handle buffer overflows + make_check_line(buffer, pos); + print(buffer); } int strip(char *buffer, int len) { @@ -40,6 +31,22 @@ int strip(char *buffer, int len) { return len; } +int make_checked_line(char *buffer, int len) { + uint8_t checksum = 0; + for (unsigned int i = 0; i < len; ++i) { + + if (buffer[i] == '\0') { + sprintf(&buffer[i], "@%02x", checksum); + + } else if (buffer[i] == '\n') { + sprintf(&buffer[i], "@%02x\n", checksum); + + } else { + checksum ^= buffer[i]; + } + } +} + int check(char *buffer, int len) { len = strip(buffer, len); if (len >= 3 && buffer[len - 3] == '@') { diff --git a/main/utils/uart.h b/main/utils/uart.h index fd965be1..46339778 100644 --- a/main/utils/uart.h +++ b/main/utils/uart.h @@ -2,4 +2,5 @@ void echo(const char *fmt, ...); int strip(char *buffer, int len); +int make_checked_line(char *buffer, int len); int check(char *buffer, int len); From eb6dcde38af351a57fe6ea710dd3c0bd0c209b38 Mon Sep 17 00:00:00 2001 From: Falko Schindler Date: Thu, 11 Jul 2024 10:39:42 +0200 Subject: [PATCH 2/4] revert make_checked_line(), fix write_checked_line() instead --- main/modules/serial.cpp | 20 +++++++++++++++----- main/utils/uart.cpp | 31 ++++++++++++------------------- main/utils/uart.h | 1 - 3 files changed, 27 insertions(+), 25 deletions(-) diff --git a/main/modules/serial.cpp b/main/modules/serial.cpp index aa504910..2b48530f 100644 --- a/main/modules/serial.cpp +++ b/main/modules/serial.cpp @@ -50,11 +50,21 @@ size_t Serial::write(const uint8_t byte) const { void Serial::write_checked_line(const char *message, const int length) const { char buffer[1024]; strncpy(buffer, message, length); - - // TODO: Handle buffer overflows - int len = make_checked_line(buffer, length); - buffer[len++] = '\n'; - uart_write_bytes(this->uart_num, buffer, len); + + char line_buffer[1028]; + uint8_t checksum = 0; + int start = 0; + for (unsigned int i = 0; i < length; ++i) { + if (buffer[i] == '\n') { + buffer[i] = '\0'; + const int line_length = sprintf(line_buffer, "%s@%02x\n", &buffer[start], checksum); + uart_write_bytes(this->uart_num, line_buffer, line_length); + start = i + 1; + checksum = 0; + } else { + checksum ^= buffer[i]; + } + } } int Serial::available() const { diff --git a/main/utils/uart.cpp b/main/utils/uart.cpp index 60886d73..74dd0511 100644 --- a/main/utils/uart.cpp +++ b/main/utils/uart.cpp @@ -15,9 +15,18 @@ void echo(const char *format, ...) { pos += std::sprintf(&buffer[pos], "\n"); - // TODO: Properly handle buffer overflows - make_check_line(buffer, pos); - print(buffer); + uint8_t checksum = 0; + int start = 0; + for (unsigned int i = 0; i < pos; ++i) { + if (buffer[i] == '\n') { + buffer[i] = '\0'; + printf("%s@%02x\n", &buffer[start], checksum); + start = i + 1; + checksum = 0; + } else { + checksum ^= buffer[i]; + } + } } int strip(char *buffer, int len) { @@ -31,22 +40,6 @@ int strip(char *buffer, int len) { return len; } -int make_checked_line(char *buffer, int len) { - uint8_t checksum = 0; - for (unsigned int i = 0; i < len; ++i) { - - if (buffer[i] == '\0') { - sprintf(&buffer[i], "@%02x", checksum); - - } else if (buffer[i] == '\n') { - sprintf(&buffer[i], "@%02x\n", checksum); - - } else { - checksum ^= buffer[i]; - } - } -} - int check(char *buffer, int len) { len = strip(buffer, len); if (len >= 3 && buffer[len - 3] == '@') { diff --git a/main/utils/uart.h b/main/utils/uart.h index 46339778..fd965be1 100644 --- a/main/utils/uart.h +++ b/main/utils/uart.h @@ -2,5 +2,4 @@ void echo(const char *fmt, ...); int strip(char *buffer, int len); -int make_checked_line(char *buffer, int len); int check(char *buffer, int len); From 99d0bd76992be67b5bccaff82b0136373878b174 Mon Sep 17 00:00:00 2001 From: Falko Schindler Date: Thu, 11 Jul 2024 12:13:30 +0200 Subject: [PATCH 3/4] fix write_checked_line for messages without trailing line break --- main/modules/serial.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/main/modules/serial.cpp b/main/modules/serial.cpp index 2b48530f..7f8d92ce 100644 --- a/main/modules/serial.cpp +++ b/main/modules/serial.cpp @@ -48,17 +48,19 @@ size_t Serial::write(const uint8_t byte) const { } void Serial::write_checked_line(const char *message, const int length) const { + int len = length; char buffer[1024]; - strncpy(buffer, message, length); + strncpy(buffer, message, len); + len += std::sprintf(&buffer[len], "\n"); char line_buffer[1028]; uint8_t checksum = 0; int start = 0; - for (unsigned int i = 0; i < length; ++i) { + for (unsigned int i = 0; i < len; ++i) { if (buffer[i] == '\n') { buffer[i] = '\0'; - const int line_length = sprintf(line_buffer, "%s@%02x\n", &buffer[start], checksum); - uart_write_bytes(this->uart_num, line_buffer, line_length); + const int line_len = sprintf(line_buffer, "%s@%02x\n", &buffer[start], checksum); + uart_write_bytes(this->uart_num, line_buffer, line_len); start = i + 1; checksum = 0; } else { From f356e0b431fc87854ed16a6f911d5c5d7a0b3abc Mon Sep 17 00:00:00 2001 From: Falko Schindler Date: Thu, 11 Jul 2024 15:28:50 +0200 Subject: [PATCH 4/4] simplification and performance improvement --- main/modules/serial.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/main/modules/serial.cpp b/main/modules/serial.cpp index 7f8d92ce..b58def0f 100644 --- a/main/modules/serial.cpp +++ b/main/modules/serial.cpp @@ -48,23 +48,18 @@ size_t Serial::write(const uint8_t byte) const { } void Serial::write_checked_line(const char *message, const int length) const { - int len = length; - char buffer[1024]; - strncpy(buffer, message, len); - len += std::sprintf(&buffer[len], "\n"); - - char line_buffer[1028]; + static char checksum_buffer[16]; uint8_t checksum = 0; int start = 0; - for (unsigned int i = 0; i < len; ++i) { - if (buffer[i] == '\n') { - buffer[i] = '\0'; - const int line_len = sprintf(line_buffer, "%s@%02x\n", &buffer[start], checksum); - uart_write_bytes(this->uart_num, line_buffer, line_len); + for (unsigned int i = 0; i < length + 1; ++i) { + if (i >= length || message[i] == '\n') { + sprintf(checksum_buffer, "@%02x\n", checksum); + uart_write_bytes(this->uart_num, &message[start], i - start); + uart_write_bytes(this->uart_num, checksum_buffer, 4); start = i + 1; checksum = 0; } else { - checksum ^= buffer[i]; + checksum ^= message[i]; } } }