Skip to content

Fix/mqtt publish hanging - #81

Merged
jehanshah8 merged 3 commits into
mainfrom
fix/mqtt-publish-hanging
Sep 26, 2025
Merged

Fix/mqtt publish hanging#81
jehanshah8 merged 3 commits into
mainfrom
fix/mqtt-publish-hanging

Conversation

@jehanshah8

@jehanshah8 jehanshah8 commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Fix robot status hang issue with best-effort publishing

Problem
Applications using the EdgeSDK would hang indefinitely when _send_robot_status() called wait_for_publish() without a timeout, particularly during network instability or MQTT reconnection attempts. This caused connectors to appear frozen while still consuming resources, leading to poor user experience and operational issues.

Solution
Replaced the blocking status publishing approach with a best-effort, non-blocking implementation that eliminates hanging while maintaining status accuracy through InOrbit's multi-layer detection system.

Key Changes
Removed blocking wait_for_publish() calls that could hang indefinitely
Direct client.publish() with exception handling for immediate, non-blocking status updates
Eliminated threading complexity from _on_connect while preserving functionality

Resilience Design
InOrbit's detection system provides multiple fallback layers when status publishing fails:
Data message detection - InOrbit detects when "offline" robots send data and requests status updates
Ping-based detection - Active pinging with automatic offline marking after failed attempts
Timeout-based cleanup - Stale connection detection and cleanup
InOrbit Status Request Flow: When InOrbit receives data from a robot marked as offline (if connection goes through but status update does not in _on_connect()), it automatically sends a get_state command requesting the robot to re-send its status. This means best-effort status publishing in EdgeSDK is not problematic - if the initial status fails, InOrbit will detect the discrepancy through data flow and explicitly request an update. Applications using EdgeSDK (like connectors) can implement get_state handlers to respond to these requests with accurate robot state information.
Note: The get_state command handler should be implemented at the connector level (not EdgeSDK) to provide intelligent robot state assessment based on actual robot connectivity, not just MQTT connection status. This way the connector can also decide how to publish status when the connector is online but disconnected from robot.
This makes explicit status messages an optimization rather than a strict requirement for accurate online/offline detection.

@jehanshah8 jehanshah8 self-assigned this Sep 25, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

Comment thread inorbit_edge/robot.py Outdated
@jehanshah8
jehanshah8 force-pushed the fix/mqtt-publish-hanging branch from ef58570 to 133b260 Compare September 25, 2025 15:48
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@jehanshah8
jehanshah8 force-pushed the fix/mqtt-publish-hanging branch from b98f0fd to 55489b6 Compare September 26, 2025 00:18
cursor[bot]

This comment was marked as outdated.

@jehanshah8
jehanshah8 force-pushed the fix/mqtt-publish-hanging branch 2 times, most recently from 87c83e8 to 0df7be1 Compare September 26, 2025 00:35
Replace blocking _send_robot_status with non-blocking best-effort approach:
- Remove wait_for_publish() calls that could hang indefinitely
- Use direct client.publish() with exception handling
- Maintain status accuracy through InOrbit's multi-layer detection system
- Eliminate threading complexity while preserving functionality
@jehanshah8
jehanshah8 force-pushed the fix/mqtt-publish-hanging branch from 0df7be1 to 91fafd0 Compare September 26, 2025 01:30
@jehanshah8

Copy link
Copy Markdown
Contributor Author

@leandropineda can I please get a quick re-review, code changes are trivial but need to make sure the design is good (see pr description)

- Add get_state handling to _handle_in_cmd method for InOrbit protocol compliance
- Add set_online_status_callback() method for connectors to provide status logic
- Add _handle_get_state() method that uses callback or defaults to online
cursor[bot]

This comment was marked as outdated.

@jehanshah8
jehanshah8 merged commit 1050174 into main Sep 26, 2025
31 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.

2 participants