Add button to download the last .bin file from the FC#774
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to download the last flight log (.bin file) from an ArduPilot flight controller using MAVFTP. The feature provides a GUI button for users to download flight logs for analysis purposes.
Key changes:
- Adds a new "Download last flight log" button to the parameter editor interface
- Implements comprehensive flight log download functionality with multiple fallback methods for log discovery
- Includes progress tracking and error handling for the download process
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| frontend_tkinter_parameter_editor.py | Adds GUI button and threading logic for flight log downloads |
| backend_flightcontroller.py | Implements flight log download methods with fallback strategies for log number detection |
| # Test if this log file exists | ||
| temp_test_file = f"temp_test_{mid}.tmp" | ||
| mavftp.cmd_get([remote_filename, temp_test_file]) | ||
| ret = mavftp.process_ftp_reply("OpenFileRO", timeout=5) # Must be > idle_detection_time (3.7s) |
There was a problem hiding this comment.
The comment references idle_detection_time (3.7s) but uses a timeout of 5 seconds. This magic number relationship should be documented more clearly or the idle_detection_time should be referenced as a constant.
|
|
||
| # Binary search to find the highest log number | ||
| low = 1 | ||
| high = 9999 # Reasonable upper bound for log numbers |
There was a problem hiding this comment.
The magic number 9999 should be defined as a named constant (e.g., MAX_LOG_NUMBER = 9999) to improve maintainability and make the upper bound configurable.
|
|
||
| # Download the actual log file | ||
| mavftp.cmd_get([remote_filename, local_filename], progress_callback=get_progress_callback) | ||
| ret = mavftp.process_ftp_reply("OpenFileRO", timeout=0) # No timeout for large log files |
There was a problem hiding this comment.
Using timeout=0 (infinite timeout) for large log files could cause the application to hang indefinitely if there are network issues. Consider using a reasonable large timeout value instead.
|
|
||
| # Start the download in a separate thread to avoid blocking the GUI | ||
| def download_thread() -> None: | ||
| error_msg = "" |
There was a problem hiding this comment.
The variable 'error_msg' is declared but only used in the exception handler. It should be declared closer to where it's used or removed if not needed for the initial empty string assignment.
| error_msg = "" |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
amilcarlucas
force-pushed
the
download_last_bin_log
branch
5 times, most recently
from
31ccb9c to
08b2cd2
Compare
amilcarlucas
force-pushed
the
download_last_bin_log
branch
from
08b2cd2 to
3e2c230
Compare
amilcarlucas
force-pushed
the
download_last_bin_log
branch
from
3e2c230 to
ca75300
Compare
No description provided.