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

MAVLink app: added static of struct vehicle_attitude_setpoint_s att_sp, preserving previous value #2112

Closed
wants to merge 1 commit into from

Conversation

dvorak0
Copy link

@dvorak0 dvorak0 commented May 1, 2015

1d283bf removed 'static' incorrectly making it unable to preserve the value of struct vehicle_attitude_setpoint_s att_sp.

The detail is well explained by the comment above this code:

The tricky part in parsing this message is that the offboard sender can set attitude and thrust using different messages. Eg.: First send set_attitude_target containing the attitude and ignore bits set for everything else and then send set_attitude_target containing the thrust and ignore bits set for everything else.

@AndreasAntener
Copy link
Member

@thomasgubler could you take a look? :)
It would be nice to make this a member variable instead of the static declaration.

@thomasgubler
Copy link
Contributor

We have #1983 exactly for this

@dvorak0
Copy link
Author

dvorak0 commented May 1, 2015

Thanks for your reply. @thomasgubler @AndreasAntener

I checked #1983. It fixed the bug by adding variable. However, it is still not merged. So what I should do is waiting for it?

@AndreasAntener
Copy link
Member

Thanks Thomas, I completely forgot that this is not merged yet ;)
@dvorak0 so you did a test with #1983 and are able to fly with this? Because #1983 is only waiting for testers to confirm that it's working, so it would be awesome if you could give it a test in real. Then we can go ahead and merge it.

@dvorak0
Copy link
Author

dvorak0 commented May 1, 2015

@AndreasAntener I've pulled #1983 locally. During my test(set attitude and thrust through mavros) it works well. Maybe it's time to merge it.

@LorenzMeier
Copy link
Member

Ok, thanks for testing. Will apply #1983 then.

@LorenzMeier LorenzMeier closed this May 4, 2015
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.

4 participants