48 improve flush robustness#109
Conversation
pierre-rouanet
commented
Jan 5, 2026
pierre-rouanet
commented
- Remove assert and uses Err instead.
- Add retry and post delay to improve robustness
- Add retries and delay - Raise proper error instead of assert.
There was a problem hiding this comment.
Pull request overview
This PR improves the robustness of the serial port flush operation by replacing an assert with proper error handling and implementing a retry mechanism with configurable delays.
Key Changes:
- Replaced
assert!with error return to handle flush failures gracefully - Added retry logic with configurable attempts and delays between retries
- Improved logging for flush operations with warnings and errors
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ok(()) | ||
| } else { | ||
| log::error!("Could not flush input buffer before sending instruction"); | ||
| Err(Box::new(CommunicationErrorKind::ParsingError)) |
There was a problem hiding this comment.
The error kind ParsingError is misleading for a flush failure. Consider using a more appropriate error variant that indicates a communication or buffer flush failure, or create a new error variant if one doesn't exist.
| Err(Box::new(CommunicationErrorKind::ParsingError)) | |
| Err(Box::new(CommunicationErrorKind::TimeoutError)) |
| max_retries | ||
| ); | ||
| self.flush(port)?; | ||
| std::thread::sleep(flush_after_delay); |
There was a problem hiding this comment.
Blocking the thread with std::thread::sleep inside a retry loop may not be ideal for async contexts or performance-sensitive code. Consider whether this blocking behavior is acceptable for all use cases of this trait, or if a configurable sleep strategy would be better.
