PX4-Autopilot
PX4-Autopilot copied to clipboard
Parameter migration: code cleanup, migration script, and land_detector trial
Describe problem solved by this pull request As discussed on Slack, legacy c parameter files can/should be migrated to the newer YAML module definitions.
Describe your solution This PR:
- Performs some minor code cleanup on the parameter parsing and validation code in favor of new Python3 idioms
- Adds a python script that uses this parsing code to autogenerate yaml module definitions
- Uses the above script to migrate
land_detector
to YAML definitions.
Test data / coverage Not flight tested. Build completes successfully locally, so uploading this to run on CI.
@dagar Could you take a look at this please?
The only issue I can see so far is MAVROS which is running a bionic container with Python 3.6. I will file a PR to upgrade that to 3.7 or 3.8 as long as there no other side effects of doing so.
Also should do a rebase and pass clang tidy, but functionally I think this is complete. If this looks good to you I can continue the work on the other modules.
As per slack, we're going to go aggressive on this and just migrate the entire codebase + scrap the old junk. In the meantime a sample (land_detector) is pushed.
- [x] land_detector
- [x] airspeed_selector
- [x] angular_velocity_controller
- [x] attitude_estimator_q
- [x] battery_status
- [x] commander
- [x] commander/failure_detector
- [x] dataman
- [x] ekf2
- [x] events
- [x] flight_mode_manager
- [x] gimbal
- [x] gyro_calibration
- [x] gyro_fft
- [x] landing_target_estimator
- [x] load_mon
- [x] local_position_estimator
- [x] logger
- [x] mag_bias_estimator
- [x] manual_control
- [x] mavlink
- [x] mc_hover_thrust_estimator
- [x] navigator
- [x] rc_update
- [x] *_control
- [x] sensors
- [x] sih
- [x] simulator
- [x] temperature_compensation
- [x] verify no change to parameters.xml
- [ ] remove parser for old parameter definitions
- [ ] remove script middle steps
- [ ] scripting/tooling upgrades
@dagar All of modules and drivers has been migrated. diffing parameters.xml shows no technical changes, only a couple formatting irregularities and a couple strange parameters i had to update (especially the rc_update module). Nonetheless it would be good to get a second eye on it.
There are a few remaining files in the libraries and other directories. Will the module.yaml work there as well? (I'll try it tomorrow).
Multiple small bugs in the parameter pipeline have been found and fixed. Once we're completely free of C source I'll start simplifying the scripts.
Update: something wrong with simulator/battery_simulator, probably because I testing for the fmu-v5x target. Will explore tomorrow.
Everything is now successful except for a new dependency (dataclasses
to be backwards compatible with older Python versions). Updated in this PR: https://github.com/PX4/PX4-containers/pull/318
@dagar This is ready when you want to take a look.
Hopefully we can get this in soon to minimize merge conflicts. The tooling will be moved to a separate PR.
Thanks @bkueng for the review, I will implement your changes in the morning, and squash the WIP commit. If you have time, could you please check the XML and json/md diff on main
? The diffs I posted are probably out of date, and I've checked but I'd like another eye on it if you are able to.
I looked at the diff you provided and it looks good. Might be worth checking if QGC doesn't have a problem with the scientific float notation (3.0e-3
), and the case change of reboot_required
.
Hi @coderkalyan , would it help get this PR moving again if we started creating a few smaller *_param.c
file to *.yaml
file PRs?
I'd be happy to help with a few of those.
Hi @mcsauder thanks for pitching in. There are two parts to this PR:
- multiple bug fixes and feature improvements of the parameter build system
- the actual migration, facilitated by tools/migrate_c_params.py with some amount of manual editing and merging.
I was talking with the dev team and I am going to try to pull the build system improvements into a separate PR. Merging that will unblock this PR. The main issue with migrating everything here is that we start spending hours on rebasing the generated files all the time.
I'm going to additionally split drivers into a new PR, since they don't get updated nearly as often. I think I can get both of those merged soon. Could you please take a couple modules at a time and rebase/create new PRs? I think if you start off with all the *_control modules that would be a good chunk done and we can touch base after that.
Please keep in mind that some of the bug fixes I made are required for the system to accept all the new yaml files. If you find any errors building or missing parameters in the generated output, double check with this PR or rebase with my new tooling PR once I push that. Thanks!
Hi @coderkalyan , just checking in. I'm chipping away on branches to further this PR, but waiting now on #20108 and #20172. I will continue work to get this across the finish line after those PRs.
Sorry all for the absence. I'm pulling the python tooling updates into a separate PR right now. I'll get this all wrapped up within the next few days.