Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 9, 2025

Interactive applications like dmell that read input character-by-character were experiencing duplicate/interleaved characters (typing "ls" → "sls", "echo" → "ecoho") due to a race condition in the monitor's terminal configuration.

Root Cause

The monitor was restoring terminal echo to ON after reading each character. When firmware reads input character-by-character with echo disabled for manual control (using Dmod_Stdin_SetFlags), the sequence caused:

  1. Monitor configures terminal: echo=OFF (correct)
  2. User types character, monitor sends to firmware
  3. Monitor restores terminal: echo=ON (race condition!)
  4. User types next character while echo is ON → terminal echoes it
  5. Firmware also manually echoes the character via dmlog output
  6. Result: Characters appear twice, duplicated/interleaved

Changes

  • tools/monitor/monitor.c:

    • Remove eager terminal restoration after each character read
    • Terminal configuration now persists between reads and is only reconfigured on next input request based on firmware's flags
    • Added monitor_restore_terminal() function with error checking for proper cleanup
    • Fixed O_NONBLOCK flag not being cleared when switching to line mode
    • Terminal properly restored only on monitor exit or error paths
  • tools/monitor/main.c:

    • Added signal handlers (SIGINT/SIGTERM) for graceful terminal restoration
    • Call monitor_restore_terminal() on normal exit
  • tools/monitor/monitor.h:

    • Added monitor_restore_terminal() function declaration
// Before: Terminal eagerly restored to echo=ON after each character
configure_input_mode(echo_on, line_mode);
// ... read character ...
configure_input_mode(true, true); // BUG: Re-enables echo while user still typing!

// After: Terminal settings persist between character reads
configure_input_mode(echo_on, line_mode);
// ... read character ...
// No restoration - next input request will configure as needed

Testing

  • ✅ All unit tests pass (5/5)
  • ✅ Build succeeds with no warnings
  • ✅ Code review and security scan passed
  • ✅ Correctly supports both echo enabled and disabled modes as controlled by firmware via Dmod_Stdin_SetFlags

The monitor now properly respects the DMLOG_FLAG_INPUT_ECHO_OFF flag without causing race conditions during character-by-character input.

Original prompt

This section details on the original issue you should resolve

<issue_title>rozwiazac problem z echo</issue_title>
<issue_description>Jest jakis problem z echo z monitorem - nie jestem pewien gdzie, ale uzywanie jest tutaj:

https://github.com/choco-technologies/dmell/blob/main/src/dmell_ia.c

Jak odpalamy te apke na x86 (bez dmlog) to wszystko dziala poprawnie, natomiast jak uzywamy jej w dmod-boot (z dmlog) to mamy jako efekt cos takiego:

stm32f746xg@/>       
stm32f746xg@/> sls   
[WARN] Could not create mutex

stm32f746xg@/> ls etest
[ERROR] Failed to open directory 'test'
[ERROR] Failed to open directory 'test'
stm32f746xg@/> sls

stm32f746xg@/> sls -

stm32f746xg@/> ecoho "etest"
test
stm32f746xg@/> 

Wpisywałem po kolei:

ls
ls test
ls -
echo "test"
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits December 9, 2025 14:34
Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com>
Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix echo issue with monitor in dmod-boot Fix double echo in monitor when firmware handles character echoing Dec 9, 2025
Copilot AI requested a review from JohnAmadis December 9, 2025 14:39
@JohnAmadis
Copy link
Contributor

@copilot to nie jest prawdą. Firmware ustawia flagi echo za pomocą interfejsu Dmod_Stdin_SetFlags i aplikacja dmell czyści flagę echo w dmlog a ten w monitorze. Musimy wspierać zarówno włączone echo jak i nie. Nie wiem czy to jest bug w monitorze czy w dmlog ale masz znaleźć root cause

Copilot AI and others added 2 commits December 9, 2025 15:04
Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com>
Co-authored-by: JohnAmadis <17320783+JohnAmadis@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 9, 2025

@copilot to nie jest prawdą. Firmware ustawia flagi echo za pomocą interfejsu Dmod_Stdin_SetFlags i aplikacja dmell czyści flagę echo w dmlog a ten w monitorze. Musimy wspierać zarówno włączon...

You were right - my initial fix was incorrect. The root cause was a race condition in terminal configuration.

The monitor was restoring terminal echo to ON after each character read. When firmware reads character-by-character with echo disabled (for manual control), this caused:

  1. Terminal echoes next character (because echo was re-enabled)
  2. Firmware manually echoes same character via dmlog output
  3. Result: duplicate/interleaved characters (e.g., "ls" → "sls")

Fixed in commits 7138483 and d1c7d1c:

  • Don't restore terminal settings between character reads
  • Let next input request configure terminal based on firmware flags
  • Only restore on monitor exit/error
  • Added proper cleanup with signal handlers
  • Fixed O_NONBLOCK not being cleared in line mode

The monitor now correctly supports both echo enabled and disabled modes as you required.

Copilot AI changed the title Fix double echo in monitor when firmware handles character echoing Fix terminal echo race condition in monitor for character-by-character input Dec 9, 2025
@JohnAmadis JohnAmadis marked this pull request as ready for review December 9, 2025 15:27
@JohnAmadis JohnAmadis merged commit eeb20d3 into master Dec 9, 2025
6 checks passed
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.

rozwiazac problem z echo

2 participants