-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[v4.4] Gimbal fixes #11706
[v4.4] Gimbal fixes #11706
Conversation
Otherwise, we potentially work with garbage.
This changes how the found gimbals are referenced. Instead of only using the gimbal device ID for the gimbal map, we now also use the gimbal manager compid. This assumes that it is valid to have more than one gimbal manager with non-MAVLink gimbals attached, which means the gimbal_device_id would clash in that case, e.g. both would be 1. Therefore, we use the gimbal manager compid as well as the associated gimbal_device_id as the map key.
We should use the new yaw value, not the previous one when calculating the missing frame.
We shouldn't just send the commands to the vehicle because the gimbal manager might be implemented on any component, not just the autopilot.
@Davidsastresas would be good to get your testing, so I can get release v4.4.1 soon. |
I just tested with Ardupilot 4.5.4 SITL, and it seems everything works alright. I tested all the functionality on following setup:
I have not tested other combinations than that. But I think the other interesting scenario to test after such changes is precisely what was reported to be fixed in #11686, so I think we should be good. I saw you caught some sneaky bugs like ddeb2b5 how we were updating gimbal yaw from gimbal_device_attitude_status message, we were using the status of the last message instead of the one we were processing, very nice catches! And overall, very nice fixes too, it is great that we keep covering all the scenarios we didn't thought about at the beginning, very nice work. Whenever this goes in please let me know and I will leverage it in #11696, so hopefully we can keep updating master from now on with the updates we keep doing on 4.x. Thanks! |
Great, thanks so much for testing @Davidsastresas. And I'm impressed that you have a test setup with 2 gimbals, something I don't have yet. |
This includes several gimbal fixes. For more information, please check the individual commit descriptions.
Fixes #11686.