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

telemetry: fix target compid of two commands #1570

Merged
merged 1 commit into from
Oct 8, 2021
Merged

Conversation

julianoes
Copy link
Collaborator

If we send them to any component, then we might get an answer from someone else rather then the autopilot and get confused.

Found by @TSC21.

@TSC21
Copy link
Member

TSC21 commented Oct 7, 2021

@julianoes should be target_component_id

src/plugins/telemetry/telemetry_impl.cpp Outdated Show resolved Hide resolved
src/plugins/telemetry/telemetry_impl.cpp Outdated Show resolved Hide resolved
JonasVautherin
JonasVautherin previously approved these changes Oct 7, 2021
@JonasVautherin
Copy link
Collaborator

I'm not clear why it wants it called target_component_id instead of target_component, because I thought it was target_component from https://mavlink.io/en/messages/common.html#COMMAND_LONG, right? 😅

@TSC21
Copy link
Member

TSC21 commented Oct 7, 2021

I'm not clear why it wants it called target_component_id instead of target_component, because I thought it was target_component from https://mavlink.io/en/messages/common.html#COMMAND_LONG, right? sweat_smile

It's what is defined on the MAVSDK API.

@julianoes
Copy link
Collaborator Author

I think Nuno is right. I clearly did not even try to build it.🙈

@JonasVautherin
Copy link
Collaborator

It's what is defined on the MAVSDK API.

Oooh that's in the MAVSDK API! I was really confused by looking at the mavlink specs 😆. Thanks Nuno!

If we send them to any component, then we might get an answer from
someone else rather then the autopilot and get confused.
@julianoes julianoes merged commit d966b41 into main Oct 8, 2021
@julianoes julianoes deleted the pr-fix-target branch October 8, 2021 11:18
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.

3 participants