-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Use new name for ArduPlane RTL altitude in all Safety components #13673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
So in looking at this, both this change and #12195 were not done correctly. They should be using the built in FirmwarePlugin::paramNameRemapMajorVersionMap functionality to provide parameter name remapping between versions. This way the qml code only needs to be updated once to use the "r.FOO" variants of parameter names and then the FirmwarePlugin will switch to the right parameter name based on firmware version. |
|
Thanks @DonLakeFlyer! I'll update this and have a look for other spots where this should be changed. |
Awesome, thanks so much |
|
I had a look through 4.5 release notes and searched the codebase for any usage of renamed parameters, only ALT_HOLD_RTL and ARSPD_FBW_{MIN|MAX} were used. Also there I checked that ArduPilot team didn't rename any parameters in 4.6, should I bump this number? I believe they don't do breaking changes to parameters on patch releases. qgroundcontrol/src/FirmwarePlugin/APM/ArduPlaneFirmwarePlugin.cc Lines 95 to 99 in fa3ee73
I also searched for |
04c7b31 to
365cd6c
Compare
|
I have confirmed the RTL altitude works correctly everywhere in GUI with ArduPlane 4.6, but can't test 4.4 to ensure the remapping works, because of Python problems with But I consider this tested enough, because the remapping system was already tested with airspeed. |
|
This looks all good, thanks so much. I'm just gonna do a quick SITL test as well and then merge. |
Description
Use the correct RTL_ALTITUDE parameter for ArduPlane > 4.5.
Reuses logic introduced to other QML files referencing ALT_HOLD_RTL.
It was originally done in 2db7285 PR #12195, but this file wasn't changed then.
Test Steps
Checklist:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Contribution on behalf of @FlyfocusUAV