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

gimbal: mutex fixes and added env variable for v2 #2298

Merged
merged 1 commit into from
May 13, 2024
Merged

Conversation

julianoes
Copy link
Collaborator

  • Make sure that _gimbal_protocol is also protected by the mutex.
  • Add MAVSDK_FORCE_GIMBAL_V2 to force usage of the gimbal v2 protocol and prevent the fallback to gimbal v1. This helps when we don't immediately detect all the components.
  • Keep trying to request the GIMBAL_MANAGER_INFORMATION message.

With MAVSDK v3, I want to remove support for gimbal v1, it's just a pain.

@julianoes julianoes marked this pull request as ready for review May 10, 2024 23:14
- Make sure that _gimbal_protocol is also protected by the mutex.
- Add MAVSDK_FORCE_GIMBAL_V2 to force usage of the gimbal v2 protocol
  and prevent the fallback to gimbal v1. This helps when we don't
  immediately detect all the components.
- Keep trying to request the GIMBAL_MANAGER_INFORMATION message.

Signed-off-by: Julian Oes <julian@oes.ch>
Copy link
Collaborator

@JonasVautherin JonasVautherin left a comment

Choose a reason for hiding this comment

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

Instead of adding an env variable, why not just defaulting to V2 in case of timeout? I guess your problem now is that sometimes the init times out and it defaults to V1, right? But if V1 is presumably not used by anyone anymore, then V2 seems like a better default, and it would move the problem to those who use V1. What do you think?

Copy link

sonarcloud bot commented May 13, 2024

Quality Gate Passed Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@julianoes
Copy link
Collaborator Author

julianoes commented May 13, 2024

@JonasVautherin that's a good idea. It's technically a break though as it changes the behavior somewhat unexpectedly.

The problem is that we have no check to identify v1. Otherwise, we could indeed have a timeout on that.

@julianoes julianoes merged commit 6ff85c2 into main May 13, 2024
28 checks passed
@julianoes julianoes deleted the pr-gimbal-fixes branch May 13, 2024 03:36
@JonasVautherin
Copy link
Collaborator

Oh, good point 😅.

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.

3 participants