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

AP_Mount: add topotek gimbal driver #27053

Merged
merged 4 commits into from
Jul 8, 2024
Merged

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented May 13, 2024

This adds support for the Topotek gimbals and replaces #26666

This has been heavily reworked to reduced its flash size (now 6660 on CubeOrangePlus) and to make it more robust to errors.

This has been tested on real hardware including confirming that:

  • angle control works
  • rate control works
  • camera take-picture and record-video works
  • pictures and videos are recorded to the SD card with the correct date
  • pictures include EXIF data including accurate location where the picture was taken

Some of the changes compared to the initial PR are:

  • display firmware version to user
  • switch automatically to RC control
  • simplfy set-gimbal-lock
  • replace use of ctime
  • use unix line endings
  • comment improvements

The only known issues are:

  • some use of "int" and "long" (see send_location_info() and gimbal_dist_info_analyse()
  • the model version is reported as "Unknown"
  • during yaw rate control the gimbal always moves in "earth-frame". Once it stops rotating (e.g. user centers sticks) the gimbal does correctly maintain either earth-frame or body-frame according to the "lock" vs "follow" state. The DJI RS2/RS3 gimbals also act like this so it's not a blocker

@laozhou-fujian this builds on your PR and the single commit is now Co-authored by both of us. Can you confirm you are OK with these changes?

Size comparison
image

Wiki PR ArduPilot/ardupilot_wiki#6092

Copy link
Collaborator

@Hwurzburg Hwurzburg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs MNT_TYPE metadata addition

libraries/AP_Mount/AP_Mount_Topotek.cpp Outdated Show resolved Hide resolved
libraries/AP_Mount/AP_Mount_Topotek.h Outdated Show resolved Hide resolved
libraries/AP_Mount/AP_Mount_Topotek.h Outdated Show resolved Hide resolved
libraries/AP_Mount/AP_Mount_Topotek.h Outdated Show resolved Hide resolved
tridge
tridge previously requested changes May 14, 2024
libraries/AP_Mount/AP_Mount_Topotek.cpp Outdated Show resolved Hide resolved
@rmackay9
Copy link
Contributor Author

We discussed this on the dev call and the major concern was that the driver consumes far too much flash (about 9K). Various devs have given suggestions on how it could be made smaller so I will attempt to rework the driver based on this advice.

@laozhou-fujian
Copy link
Contributor

Okay, I will start working on the modifications, thank you.

@rmackay9
Copy link
Contributor Author

@laozhou-fujian,

This came up on the dev call and Tridge and I agreed that we should help make this driver smaller. I've got a 2 week vacation coming up soon but hopefully we can help.

@laozhou-fujian
Copy link
Contributor

@laozhou-fujian,

This came up on the dev call and Tridge and I agreed that we should help make this driver smaller. I've got a 2 week vacation coming up soon but hopefully we can help.

Thank you for your help. I will try my best to complete it this week. Thank you.

@laozhou-fujian
Copy link
Contributor

@laozhou-fujian,

This came up on the dev call and Tridge and I agreed that we should help make this driver smaller. I've got a 2 week vacation coming up soon but hopefully we can help.

Sorry, I've been very busy recently. I will finish it as soon as possible

@rmackay9
Copy link
Contributor Author

@laozhou-fujian,

Just FYI, I've started shrinking the driver and I expect I will complete it within the next week. No need to stress, I just wanted to make sure we don't waste effort by both doing the same thing.

@laozhou-fujian
Copy link
Contributor

@laozhou-fujian,

Just FYI, I've started shrinking the driver and I expect I will complete it within the next week. No need to stress, I just wanted to make sure we don't waste effort by both doing the same thing.

Thank you for your advice. I will revise it right away. I feel very sorry for having to deal with a lot of urgent things recently

@rmackay9 rmackay9 requested a review from tridge July 4, 2024 06:09
@rmackay9 rmackay9 dismissed stale reviews from tridge and Hwurzburg July 4, 2024 06:10

Tridge's requests completed

@tridge tridge merged commit b6c5ad4 into ArduPilot:master Jul 8, 2024
92 checks passed
@rmackay9 rmackay9 deleted the mount-topotek branch July 8, 2024 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants