AP_Camera/AP_Mount: camera info msg gets device id to improve multi camera support #24354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rmackay9
reviewed
Jul 19, 2023
rmackay9
changed the title
rmackay9
reviewed
Jul 19, 2023
Davidsastresas
force-pushed
the
master-PR/Update_latest_gimbal_manager_v2
branch
from
45a2757
to
ea9fb5d
Compare
|
Thanks for the review @rmackay9. I removed the "WIP" from the 2 latest commits, and I fixed the GCS_common one as per your comment. |
rmackay9
reviewed
Jul 19, 2023
Davidsastresas
force-pushed
the
master-PR/Update_latest_gimbal_manager_v2
branch
from
ea9fb5d
to
a843c6d
Compare
…send, as 0: By default it sends this field as 0 ( no gimbal attached to this camera ), and then the backends can reimplement it, like mavlinkcamv2 or Siyi, sending the correspondent gimbal_device_id. In the future this might be controled by a parameter, to allow standalone cameras to be associated with a gimbal
Davidsastresas
force-pushed
the
master-PR/Update_latest_gimbal_manager_v2
branch
from
a843c6d
to
6e3a5b7
Compare
|
Thanks, this looks good to me. |
10 tasks
|
Hello, do we have any news on this one? Is something needed to be fixed before merging? Thanks! |
|
Closing in favour of #24551 Sorry, for some reason I was unable to directly push to this PR so I created a new one. The credit on the commits was kept though. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR matches the following mavlink PR ArduPilot/mavlink#323. It won't build until that PR is in.
Gimbal protocol v2 messages are not WIP anymore, and this commit adds support for the latest extra fields.
The first commits are straightforward, they basically add the gimbal_device_id field for all the backends involved.
For AP_Mount_Backend: it enables GCS to understand to which gimbal corresponds gimbal_device_attitude_status message. Right now it is impossible to understand where this message is coming from if AP has more than 1 gimbal.
For the rest of aditions it enables GCS to understand if a particular gimbal is linked to a particular camera. This is useful for a feature that is being developed in QGC to move gimbal clicking over video feed. QGC needs to know the fov/zoom of the camera to scale clicks appropiately.
The latest 2 commits are marked as WIP:
For the GCS_Common one I am not 100% sure what should be sent there. My understanding is that z angular speed is what should be sent, as we are sending angular z target for feed_forward_angular_velocity_z field, but I would love to get some feedback to confirm this. Once confirmed I will fix it and squash.
For the AP_Camera_Backend one it is marked as WIP in case we want to include a parameter to select the gimbal_device_id the camera belongs to. As it is now it is fine and it will work for mavlink cameras and gimbal cameras ( Siyi, viewpro, xacti, etc ) so I am happy as well leaving it as it is ( default to 0 ).
Please @rmackay9 if you could give me your opinion on this one as well. Thanks!