Make MAVFTP more robust on F4 processors #854
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the robustness of MAVFTP operations on STM32 F4 processors by implementing increased timeouts, retry mechanisms, and optimized settings for better reliability under workload conditions.
Key changes:
- Added increased timeout for ResetSessions operation from default 5s to 10s
- Implemented robust MAVFTP settings with reduced burst sizes and increased retry times
- Added retry logic with progressive timeout increases for parameter downloads
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
ardupilot_methodic_configurator/backend_mavftp.py |
Increased ResetSessions timeout to 10 seconds for better F4 processor compatibility |
ardupilot_methodic_configurator/backend_flightcontroller.py |
Added robust MAVFTP settings helper method, retry logic for parameter downloads, and applied robust settings to all MAVFTP operations |
| Returns: | ||
| MAVFTPSettings: Configured settings with increased timeouts and reduced burst sizes | ||
There was a problem hiding this comment.
The docstring should specify what makes these settings 'robust' and explain the rationale behind the specific parameter values chosen for F4 processors.
| Returns: | |
| MAVFTPSettings: Configured settings with increased timeouts and reduced burst sizes | |
| These settings are considered 'robust' because they are specifically tuned to address the limitations and reliability issues | |
| observed with STM32 F4 flight controllers when performing MAVFTP operations under heavy load. F4 processors have limited | |
| memory and processing power, and can be sensitive to communication bursts and timeouts. The chosen parameter values aim to: | |
| - Reduce burst sizes (burst_read_size, write_size) to prevent overwhelming the controller and to improve stability. | |
| - Lower max_backlog and write_qsize to minimize resource contention and avoid buffer overruns. | |
| - Increase idle_detection_time, read_retry_time, and retry_time to allow more time for the controller to respond, reducing | |
| the likelihood of failed transfers due to transient delays. | |
| - Set packet loss parameters to zero to avoid unnecessary retransmissions, as F4 controllers typically operate in low-loss environments. | |
| These values were determined empirically to maximize reliability and minimize transfer errors during MAVFTP operations on F4 hardware. | |
| Returns: | |
| MAVFTPSettings: Configured settings with increased timeouts and reduced burst sizes for improved reliability on F4 controllers. |
Copilot uses AI. Check for mistakes.
| ] | ||
| ) | ||
|
|
||
| def _download_params_via_mavftp( # pylint: disable=too-many-locals |
There was a problem hiding this comment.
The pylint disable comment indicates high complexity. Consider refactoring the retry logic into a separate helper method to reduce the number of local variables.
Copilot uses AI. Check for mistakes.
| else: | ||
| logging_error(_("All %d parameter download attempts failed"), max_retries) | ||
|
|
||
| except Exception as e: # pylint: disable=broad-exception-caught |
There was a problem hiding this comment.
Catching broad exceptions can mask specific errors. Consider catching more specific exceptions like MAVFTPException or timeout-related exceptions instead of using a bare except.
Copilot uses AI. Check for mistakes.
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
No description provided.