New option for Gimbal control, and update gimbal indicator docs - #14000
New option for Gimbal control, and update gimbal indicator docs#14000Davidsastresas wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new setting to control gimbal yaw lock behavior during position commands and updates the documentation for the gimbal indicator. The change allows users to preserve the current yaw lock state instead of forcing yaw follow mode when centering or tilting the gimbal to 90 degrees.
Changes:
- Added
preserveYawLockOnPositionCommandssetting to preserve yaw lock status during gimbal positioning - Refactored gimbal control to use dedicated
tilt90Gimbal()method instead of direct pitch/yaw commands - Updated gripper action enum mappings to use correct MAVLink constants
- Extended gimbal indicator documentation with detailed operation guide
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/UI/toolbar/GimbalIndicator.qml | Added UI checkbox for new yaw lock preservation setting and updated Tilt 90 button to use new method |
| src/Settings/GimbalControllerSettings.h | Added setting declaration for preserving yaw lock on position commands |
| src/Settings/GimbalControllerSettings.cc | Added implementation declaration for new setting |
| src/Settings/GimbalController.SettingsGroup.json | Added JSON configuration for new yaw lock preservation setting |
| src/MAVLink/QGCMAVLink.h | Updated gripper action enum values to match correct MAVLink constants |
| src/Gimbal/GimbalController.h | Added tilt90Gimbal() method declaration |
| src/Gimbal/GimbalController.cc | Implemented logic for preserving yaw lock state in centerGimbal() and new tilt90Gimbal() methods |
| docs/en/qgc-user-guide/fly_view/fly_view_toolbar.md | Added comprehensive gimbal indicator documentation section |
|
|
||
| #### Gimbal Control Acquisition | ||
|
|
||
| If the option "Show Acquire/Release control button is set, a new **Acquire/Release Control**button will appear |
There was a problem hiding this comment.
Missing closing quote after 'button'. Should be 'If the option "Show Acquire/Release control button" is set'.
| If the option "Show Acquire/Release control button is set, a new **Acquire/Release Control**button will appear | |
| If the option "Show Acquire/Release control button" is set, a new **Acquire/Release Control** button will appear |
|
|
||
| #### Gimbal Control Acquisition | ||
|
|
||
| If the option "Show Acquire/Release control button is set, a new **Acquire/Release Control**button will appear |
There was a problem hiding this comment.
Missing space before 'button'. Should be 'Acquire/Release Control button' with a space between the bold text and the word.
1c24c0f to
c9edb07
Compare
Build ResultsPlatform Status
Some builds failed. Pre-commit
Pre-commit hooks: 4 passed, 32 failed, 7 skipped. Artifact Sizes
|
c9edb07 to
6783187
Compare
|
@Davidsastresas Don't worry about screen shots for now. We'll do that when we get closer to stable. Dont want to capture now and have things still move around and have to redo. |
| GripperActionRelease = GRIPPER_ACTION_RELEASE, | ||
| GripperActionGrab = GRIPPER_ACTION_GRAB, | ||
| GripperActionHold = GRIPPER_ACTION_HOLD, | ||
| GripperActionRelease = GRIPPER_ACTION_OPEN, |
There was a problem hiding this comment.
Is this a mistake? Seems unrelated?
There was a problem hiding this comment.
If I recall correctly I got compilation errors and it was because of it. It is in a separate commit 38871ff
Maybe it was already fixed in master.
There was a problem hiding this comment.
Usually means you CPM cache is out of date with respect to latest mavlink headers.
There was a problem hiding this comment.
Oh I am sorry, I wasn't aware about it. Please let me take a look at this again in the next few days. The user that reported the issue added some more comments and maybe that changes the current PR. I will remove this mavlink thing and test, and also work on those possible new requests.
Thanks!
6783187 to
752f53b
Compare
|
@DonLakeFlyer you were totally right. I removed the mavlink commit and rebased to master, force pushed. #13955 was closed recently so I don't have nothing to add to this PR. |
752f53b to
da7b7c4
Compare
|
@Davidsastresas Sorry lost track of this. I'll pick it back up when I get back in town first week in July... |
|
@DonLakeFlyer no problem at all. This was done exclusively because that user complained, but he doesn't seem to be interested anymore, so there is really no hurry. You can ping me anytime about this and I will rebase it to master at the moment, and double check again. |
I'm gonna move this to 5.2 then so it has some bake time after it goes in |
Folowing #13955 (comment)
For Center and tilt 90 gimbal commands, we were sending setting also the gimbal to follow mode, as that is usually the expected behaviour. We added a new setting now to instead preserve the previous gimbal lock status:
When this is set, center and tilt 90 will just preserve current gimbal lock/follow status.
Additionally, the top toolbar indicator docs were extended to include some info from this gimbal indicator.
For awareness @DonLakeFlyer.