From 142694ceb206b50e3c0e59f94ff56fff49a65056 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 14:29:27 +0000 Subject: [PATCH 1/5] Initial plan From 3070b093477c9be3ddf41721d7e9ee4b1ad9d640 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 14:34:16 +0000 Subject: [PATCH 2/5] Fix double echo issue in monitor - always disable terminal echo Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com> --- tools/monitor/monitor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/monitor/monitor.c b/tools/monitor/monitor.c index c522202..c27e2d1 100644 --- a/tools/monitor/monitor.c +++ b/tools/monitor/monitor.c @@ -788,7 +788,9 @@ bool monitor_handle_input_request(monitor_ctx_t *ctx) // Read input from file or stdin (no prompt, firmware should print its own prompt) char input_buffer[512]; - bool echo_on = (ctx->ring.flags & DMLOG_FLAG_INPUT_ECHO_OFF) == 0; + // Always disable terminal echo - the firmware application handles echoing + // If we enable terminal echo here, characters will be duplicated (terminal echo + app echo) + bool echo_on = false; bool line_mode = (ctx->ring.flags & DMLOG_FLAG_INPUT_LINE_MODE) != 0; configure_input_mode(echo_on, line_mode); From cbbb7d43028f986bbbdeff28c6b1526521898f61 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 14:36:34 +0000 Subject: [PATCH 3/5] Improve comment to clarify ECHO_OFF flag behavior Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com> --- tools/monitor/monitor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/monitor/monitor.c b/tools/monitor/monitor.c index c27e2d1..b6f8834 100644 --- a/tools/monitor/monitor.c +++ b/tools/monitor/monitor.c @@ -788,7 +788,9 @@ bool monitor_handle_input_request(monitor_ctx_t *ctx) // Read input from file or stdin (no prompt, firmware should print its own prompt) char input_buffer[512]; - // Always disable terminal echo - the firmware application handles echoing + // Always disable terminal echo - the firmware application is responsible for all echoing + // via the dmlog output buffer. The DMLOG_FLAG_INPUT_ECHO_OFF flag tells the firmware + // whether to echo characters back, not the monitor terminal. // If we enable terminal echo here, characters will be duplicated (terminal echo + app echo) bool echo_on = false; bool line_mode = (ctx->ring.flags & DMLOG_FLAG_INPUT_LINE_MODE) != 0; From 7138483f58bbff1fe16988f008e9ecfd53ebad94 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 15:04:49 +0000 Subject: [PATCH 4/5] Fix echo race condition - don't restore terminal between char reads Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com> --- tools/monitor/main.c | 21 +++++++++++++++++++++ tools/monitor/monitor.c | 39 ++++++++++++++++++++++++++++++++------- tools/monitor/monitor.h | 1 + 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/tools/monitor/main.c b/tools/monitor/main.c index 107e331..acce789 100644 --- a/tools/monitor/main.c +++ b/tools/monitor/main.c @@ -1,6 +1,7 @@ #include #include #include +#include #include "dmlog.h" #include "monitor.h" #include "trace.h" @@ -10,6 +11,19 @@ # define DMLOG_VERSION "unknown" #endif +static volatile bool exit_requested = false; + +/** + * @brief Signal handler for graceful shutdown + */ +static void signal_handler(int signum) +{ + (void)signum; + exit_requested = true; + // Restore terminal settings immediately + monitor_restore_terminal(); +} + void usage(const char *progname) { printf("Usage: %s [options]\n", progname); @@ -178,10 +192,17 @@ int main(int argc, char *argv[]) } } + // Register signal handlers for graceful shutdown + signal(SIGINT, signal_handler); + signal(SIGTERM, signal_handler); + monitor_run(ctx, show_timestamps, blocking_mode); TRACE_INFO("Exiting monitor\n"); + // Restore terminal settings before exit + monitor_restore_terminal(); + // Main monitoring loop would go here monitor_disconnect(ctx); diff --git a/tools/monitor/monitor.c b/tools/monitor/monitor.c index b6f8834..265d141 100644 --- a/tools/monitor/monitor.c +++ b/tools/monitor/monitor.c @@ -10,6 +10,24 @@ #include #include +/** + * @brief Restore terminal to normal settings (echo on, line mode on, blocking) + * + * This should be called before the monitor exits or on error conditions to ensure + * the terminal is left in a usable state. + */ +void monitor_restore_terminal(void) +{ + struct termios tty; + tcgetattr(STDIN_FILENO, &tty); + tty.c_lflag |= ECHO | ICANON; + tcsetattr(STDIN_FILENO, TCSANOW, &tty); + + // Clear O_NONBLOCK + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags & ~O_NONBLOCK); +} + /** * @brief Configure terminal input mode (echo and line mode) * @@ -32,6 +50,9 @@ static void configure_input_mode(bool echo, bool line_mode) if(line_mode) { tty.c_lflag |= ICANON; + // Clear O_NONBLOCK when switching to line mode + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags & ~O_NONBLOCK); } else { @@ -788,13 +809,10 @@ bool monitor_handle_input_request(monitor_ctx_t *ctx) // Read input from file or stdin (no prompt, firmware should print its own prompt) char input_buffer[512]; - // Always disable terminal echo - the firmware application is responsible for all echoing - // via the dmlog output buffer. The DMLOG_FLAG_INPUT_ECHO_OFF flag tells the firmware - // whether to echo characters back, not the monitor terminal. - // If we enable terminal echo here, characters will be duplicated (terminal echo + app echo) - bool echo_on = false; + bool echo_on = (ctx->ring.flags & DMLOG_FLAG_INPUT_ECHO_OFF) == 0; bool line_mode = (ctx->ring.flags & DMLOG_FLAG_INPUT_LINE_MODE) != 0; + // Configure terminal according to firmware's requested flags configure_input_mode(echo_on, line_mode); // Read input, potentially switching from init script to stdin @@ -890,13 +908,13 @@ bool monitor_handle_input_request(monitor_ctx_t *ctx) // Other error - this should not happen in normal blocking mode // Continue trying to read } - configure_input_mode(true, true); // Restore terminal settings - + // Send input to firmware size_t input_len = strlen(input_buffer); if(!monitor_send_input(ctx, input_buffer, input_len)) { TRACE_ERROR("Failed to send input to firmware\n"); + configure_input_mode(true, true); // Restore terminal settings before error return return false; } @@ -905,11 +923,18 @@ bool monitor_handle_input_request(monitor_ctx_t *ctx) if(backend_write_memory(ctx->backend_type, ctx->socket, ctx->ring_address + offsetof(dmlog_ring_t, flags), &new_flags, sizeof(uint32_t)) < 0) { TRACE_ERROR("Failed to clear INPUT_REQUESTED flag\n"); + configure_input_mode(true, true); // Restore terminal settings before error return return false; } // Update local cache ctx->ring.flags = new_flags; + + // Note: Terminal settings are NOT restored here. They will be reconfigured on the next + // input request based on the firmware's flags. This prevents a race condition where + // terminal echo is enabled between character-by-character reads, causing duplicate echoing. + // Terminal settings will be restored to normal (echo on, line mode on) when monitor exits + // or when firmware requests input with echo enabled. // Return true to continue monitoring, false to exit // Exit conditions: diff --git a/tools/monitor/monitor.h b/tools/monitor/monitor.h index 6df44bc..ee77590 100644 --- a/tools/monitor/monitor.h +++ b/tools/monitor/monitor.h @@ -41,5 +41,6 @@ bool monitor_handle_input_request(monitor_ctx_t *ctx); bool monitor_handle_send_file_request(monitor_ctx_t *ctx); bool monitor_handle_receive_file_request(monitor_ctx_t *ctx); bool monitor_send_file_transfer(monitor_ctx_t* ctx, const dmlog_file_transfer_t* transfer, uint32_t flags); +void monitor_restore_terminal(void); #endif // MONITOR_H \ No newline at end of file From d1c7d1cf7021b2a21a0e7385d447462b009e4578 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Dec 2025 15:06:39 +0000 Subject: [PATCH 5/5] Add error checking to terminal restoration and simplify signal handler Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com> --- tools/monitor/main.c | 4 +--- tools/monitor/monitor.c | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/monitor/main.c b/tools/monitor/main.c index acce789..246cd0d 100644 --- a/tools/monitor/main.c +++ b/tools/monitor/main.c @@ -11,17 +11,15 @@ # define DMLOG_VERSION "unknown" #endif -static volatile bool exit_requested = false; - /** * @brief Signal handler for graceful shutdown */ static void signal_handler(int signum) { (void)signum; - exit_requested = true; // Restore terminal settings immediately monitor_restore_terminal(); + exit(0); } void usage(const char *progname) diff --git a/tools/monitor/monitor.c b/tools/monitor/monitor.c index 265d141..f00f6c5 100644 --- a/tools/monitor/monitor.c +++ b/tools/monitor/monitor.c @@ -19,13 +19,25 @@ void monitor_restore_terminal(void) { struct termios tty; - tcgetattr(STDIN_FILENO, &tty); + if(tcgetattr(STDIN_FILENO, &tty) < 0) + { + TRACE_WARN("Failed to get terminal attributes during restore\n"); + return; + } + tty.c_lflag |= ECHO | ICANON; - tcsetattr(STDIN_FILENO, TCSANOW, &tty); + + if(tcsetattr(STDIN_FILENO, TCSANOW, &tty) < 0) + { + TRACE_WARN("Failed to set terminal attributes during restore\n"); + } // Clear O_NONBLOCK int flags = fcntl(STDIN_FILENO, F_GETFL, 0); - fcntl(STDIN_FILENO, F_SETFL, flags & ~O_NONBLOCK); + if(flags >= 0) + { + fcntl(STDIN_FILENO, F_SETFL, flags & ~O_NONBLOCK); + } } /**