fix(interface): make connect_interface a no-op when already connected

[jmthomas]

What changed

The redundant-disconnect check and the state change that records it now share one critical section; splitting them let a second thread disconnect the interface twice. attempting() gained the already-connected guard, so the reconnect path calls attempt_connection() directly to avoid being ignored when interface cleanup raises or leaves connected? true.

Why it changed

Connecting a CONNECTED interface set state to ATTEMPTING without touching the socket, so a working connection was torn down and rebuilt only once the blocking read returned, costing a full read_timeout. Interface connect now closes any existing stream instead of abandoning it to the GC, and disconnect no longer skips cleanup based on connected?.

Fixes #3798

Testing strategy

Mostly unit tests. Also ran the demo and connected and disconnect. Ran script to test multiple connect / disconnect:

for interface in ["INST_INT", "INST2_INT"]:
  disconnect_interface(interface)
  wait(3)
  disconnect_interface(interface)
  wait(3)
  connect_interface(interface)
  wait(3)
  connect_interface(interface)
  wait(3)

while watching the CmdTlmServer. Observed Connect ignored, already connected messages. Finally connected EXAMPLE_INT and TEMPLATE_INT and then restarted their backend interfaces to observe reconnect.

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.30%. Comparing base (b5ee05d) to head (09b7a0c).
⚠️ Report is 44 commits behind head on main.

Files with missing lines	Patch %	Lines
openc3/lib/openc3/interfaces/serial_interface.rb	0.00%	1 Missing ⚠️
...lib/openc3/microservices/interface_microservice.rb	94.73%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3828      +/-   ##
==========================================
+ Coverage   79.27%   79.30%   +0.03%     
==========================================
  Files         895      896       +1     
  Lines       67271    67399     +128     
  Branches     2641     2608      -33     
==========================================
+ Hits        53327    53453     +126     
- Misses      13273    13276       +3     
+ Partials      671      670       -1     

Flag	Coverage Δ
frontend	66.56% <ø> (+0.35%)	⬆️
python	79.37% <ø> (+<0.01%)	⬆️
ruby-api	82.22% <ø> (-0.28%)	⬇️
ruby-backend	84.58% <90.90%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

🟡 Changes recommended

InterfaceThread#connect now calls disconnect without handling exceptions, which can break reconnection if an interface’s disconnect raises.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens the interface connection state machine to make connect_interface a safe no-op when an interface is already CONNECTED, while also addressing a disconnect race that could trigger duplicate disconnects and improving resource cleanup to avoid leaking sockets/streams.

Changes:

• Add an “already connected” guard in attempting() and introduce attempt_connection() to ensure reconnect paths still force a state transition.
• Fix a potential double-disconnect race by combining the redundant-disconnect check and state recording in a single critical section; move reconnect-delay sleep outside the mutex.
• Ensure connection implementations cleanly close any existing stream/socket before overwriting it; add/expand unit tests and update scripting API docs.

File summaries

File	Description
openc3/spec/microservices/interface_microservice_spec.rb	Adds Ruby specs covering redundant connect no-op, reconnect edge cases, and cleanup behavior.
openc3/python/test/microservices/test_interface_microservice.py	Adds Python tests mirroring the “already connected” no-op and reconnect edge cases.
openc3/python/openc3/microservices/interface_microservice.py	Implements CONNECTED no-op for redundant connect, refactors reconnect path, and improves disconnect synchronization.
openc3/python/openc3/interfaces/tcpip_client_interface.py	Closes any existing stream before creating a new TCP/IP client stream.
openc3/python/openc3/interfaces/serial_interface.py	Closes any existing stream before creating a new serial stream.
openc3/python/openc3/interfaces/mqtt_stream_interface.py	Closes any existing stream before creating a new MQTT stream.
openc3/lib/openc3/tools/cmd_tlm_server/interface_thread.rb	Adds pre-connect disconnect to avoid leaking existing connections in the CmdTlmServer interface thread.
openc3/lib/openc3/microservices/interface_microservice.rb	Implements CONNECTED no-op for redundant connect and fixes disconnect synchronization/reconnect path in Ruby microservice.
openc3/lib/openc3/interfaces/tcpip_client_interface.rb	Closes any existing stream before creating a new TCP/IP client stream.
openc3/lib/openc3/interfaces/serial_interface.rb	Closes any existing stream before creating a new serial stream.
openc3/lib/openc3/interfaces/mqtt_stream_interface.rb	Closes any existing stream before creating a new MQTT stream.
docs.openc3.com/docs/guides/scripting-api.md	Documents the no-op behavior for redundant connect_interface and the updated disconnect semantics.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud