Skip to content
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

WIP - gimbal_manager_status and mav_cmd_do_gimbal_manager_configure support #23713

Conversation

Davidsastresas
Copy link
Contributor

@Davidsastresas Davidsastresas commented May 7, 2023

WIP - Adding support for gimbal_manager_status mavlink message. This is necessary to continue the work being done in QGC:

mavlink/qgroundcontrol#10667 (comment)

I talked with @julianoes about this one, it seems we could forget for the moment about secondary control, and leave primary control at 0 unless somebody ( GCS or Autopilot ) requests control, so we change sysid and compid accordingly.

My best guess is that we should create a couple new variables for this, is Mount_Backend the proper place for it? @rmackay9 let me know your thoughts on this please.

We should also add mav_cmd_do_gimbal_manager_configure to our mavlink, because that is what the GCS will send to request and release control of the gimbal.

@Davidsastresas Davidsastresas marked this pull request as draft May 7, 2023 23:07
@Davidsastresas Davidsastresas changed the title provisional commit gimbal manager status support WIP - gimbal manager status and MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE support May 7, 2023
@Davidsastresas Davidsastresas changed the title WIP - gimbal manager status and MAV_CMD_DO_GIMBAL_MANAGER_CONFIGURE support WIP - gimbal_manager_status and mav_cmd_do_gimbal_manager_configure support May 7, 2023
@Davidsastresas Davidsastresas force-pushed the master-PR/gimbal_manager_status branch from 527aacb to 002207c Compare May 8, 2023 00:00
@rmackay9
Copy link
Contributor

rmackay9 commented May 8, 2023

Hi, thanks for taking the initiative on this. I looked at this a couple of weeks ago and got this far: https://github.com/rmackay9/rmackay9-ardupilot/commits/mount-gimbal-manager-status

My intention was just to get some slightly half-baked handling of these messages into AP master so that it doesn't hold up the QGC development and then circle back and do the more difficult work of restructing the mode handling.

@rmackay9
Copy link
Contributor

rmackay9 commented May 8, 2023

By the way, @khanasif786 will be our GSoC student this summer helping with the gimbal and camera integration. We have lots of work to do though so I'm still looking forward to PRs from anyone and everyone interested in improving our camera and gimbal support.

@Davidsastresas Davidsastresas force-pushed the master-PR/gimbal_manager_status branch from 002207c to 68dda81 Compare May 8, 2023 01:20
@Davidsastresas
Copy link
Contributor Author

Davidsastresas commented May 8, 2023

Sorry, again I worked over something you were already working. Shall we close this one, and continue yours? This one is very "draft", lots of details to close yet, yours is by far more completed.

I started working on it in order to continue on the QGC side. What we would need, in order to not do a lot of ugly workarounds in QGC would be:

  • Support getting control using gimbal_manager_configure where primary control is GCS sysid and compid ( SUPORTED in your branch )
  • Updating mavlink_control_id in your AP_Mount_Backend whenever the autopilot, or the user ( rc control ) takes command again
  • Sending right away gimbal_manager_status whenever a change happens in the primary control, so all the system is updated as soon as possible of the change.
  • Ignoring mavlink gimbal_manager commands if the sender sysid and compid are not in control, forcing them to send a gimbal_manager_configure first.

Thank you very much, and sorry for not talking to you before working on this, I should have imagined you were already on it!

@Davidsastresas
Copy link
Contributor Author

Closed in favor of #23737. Thanks @rmackay9 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants