Skip to content

Fix serial connection reliability issues#2494

Merged
sensei-hacker merged 2 commits intoiNavFlight:maintenance-9.xfrom
sensei-hacker:fix-serial-connection-reliability
Dec 23, 2025
Merged

Fix serial connection reliability issues#2494
sensei-hacker merged 2 commits intoiNavFlight:maintenance-9.xfrom
sensei-hacker:fix-serial-connection-reliability

Conversation

@sensei-hacker
Copy link
Member

@sensei-hacker sensei-hacker commented Dec 23, 2025

User description

Summary

Fixes intermittent connection failures where the configurator would fail to connect to the flight controller with MSP timeouts or confusing "connection canceled" messages. It wouldn't reconnect until Configurator is closed and re-opened. This was due to leaking the file descriptor.

Changes

1. Fix error handler in serial.js

The error handler was resolving with {error: false} when there was an actual error. This caused:

  • Confusing "connection opened but request was canceled" messages
  • Improper error propagation preventing retry logic from working correctly

Before: resolve({error: false, id: this._id++});
After: resolve({error: true, msg: error.message || 'Serial port error'});

2. Reset MSP decoder state before adding receive listeners

Moved FC.resetState() earlier in the connection sequence and added MSP.disconnect_cleanup() to reset the MSP decoder state. This ensures:

  • Garbage bytes received during USB enumeration don't corrupt the decoder
  • Boot messages from the FC don't leave the decoder in an invalid state
  • Clean decoder state (IDLE) before any data is processed

Testing

  • Verified connection works after fresh restart
  • Tested 3 disconnect/reconnect cycles - all successful
  • Confirmed MSP decoder starts in state 0 (IDLE) for all received data

Related Issues

Fixes regression introduced in PR #2476 (error handler bug)


PR Type

Bug fix


Description

  • Fix error handler incorrectly reporting success on serial port errors

  • Reset MSP decoder state before adding receive listeners to prevent corruption

  • Ensure clean decoder state (IDLE) before processing any received data

  • Add logging for serial port errors to aid debugging


Diagram Walkthrough

flowchart LR
  A["Serial Port Error"] -->|Before: resolve error:false| B["Confusing Messages"]
  A -->|After: resolve error:true| C["Proper Error Handling"]
  D["USB Enumeration"] -->|Garbage Bytes| E["Corrupted MSP Decoder"]
  D -->|Reset State First| F["Clean Decoder State"]
  C --> G["Successful Reconnection"]
  F --> G
Loading

File Walkthrough

Relevant files
Bug fix
serial.js
Fix serial error handler to report errors correctly           

js/main/serial.js

  • Fixed error handler to correctly report error: true instead of error:
    false when serial port errors occur
  • Added console logging for serial port errors to improve debugging
  • Corrected error message propagation to include error details
+4/-2     
serial_backend.js
Reset MSP decoder state before listeners added                     

js/serial_backend.js

  • Moved FC.resetState() and added MSP.disconnect_cleanup() earlier in
    connection sequence
  • Reset MSP decoder state before adding receive listeners to prevent
    garbage bytes from corrupting decoder
  • Ensures decoder starts in IDLE state before processing any received
    data
+5/-2     

Two fixes for intermittent connection failures:

1. Fix error handler in serial.js to correctly report errors
   - The error handler was resolving with {error: false} when there
     was an actual error, causing confusing "connection canceled"
     messages and preventing proper error handling

2. Reset MSP decoder state before adding receive listeners
   - Move FC.resetState() earlier in the connection sequence
   - Add MSP.disconnect_cleanup() to reset decoder state
   - This ensures garbage bytes or boot messages received during
     USB enumeration don't corrupt the MSP decoder state
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Dec 23, 2025

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Compliance Guide 🔍

All compliance sections have been disabled in the configurations.

When serial port errors occurred, the port handle wasn't being properly
cleaned up, leaving zombie handles that blocked reconnection attempts.

Changes:
- Add cleanup in error handler: removeAllListeners() + destroy()
- Make connect() async to properly await cleanup of existing ports
- Add 100ms delay after destroy() to let OS release file handle
- Fix close() race condition: null reference before close callback
- Handle case where port exists but isn't open

This fixes the "Resource temporarily unavailable Cannot lock port"
error that occurred when reconnecting after a connection failure.
@sensei-hacker sensei-hacker added this to the 9.0 milestone Dec 23, 2025
@sensei-hacker sensei-hacker merged commit 6cce81e into iNavFlight:maintenance-9.x Dec 23, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant