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

mc_att_control: landing gear publish correct message #11372

Merged
merged 1 commit into from
Feb 7, 2019

Conversation

dagar
Copy link
Member

@dagar dagar commented Feb 4, 2019

I stumbled across this invalid orb_publish.

How is landing gear even supposed to be working? The attitude controller is constantly setting it from manual control switches, but it's also polling the landing_gear topic. Shouldn't this change based on the attitude/rate controller handling manual input or not?

https://github.com/PX4/Firmware/blob/master/src/modules/mc_att_control/mc_att_control_main.cpp#L891

@dagar dagar added this to the Release v1.9.0 milestone Feb 4, 2019
@dagar dagar requested review from bkueng and MaEtUgR February 4, 2019 16:31
@LorenzMeier
Copy link
Member

This probably needs another round of validation. Various adopters are using this, but might be carrying patches.

@bkueng
Copy link
Member

bkueng commented Feb 7, 2019

@dagar you're right, this is definitely not correct.
Merging this already.

@bkueng bkueng merged commit 7f3b170 into PX4:master Feb 7, 2019
@dagar dagar deleted the pr-mc_landing_gear_fix branch February 7, 2019 13:34
@MaEtUgR
Copy link
Member

MaEtUgR commented Feb 8, 2019

Ouch that one goes on me, sorry. It was because of the conflicts with the attitude setpoint generation I mentioned here: #10842 (comment)
Conflict line: https://github.com/PX4/Firmware/pull/10842/files#diff-81c4f50bdf837c4183b5d3cb81eb40eaR527

I think these setpoints that are generated and processed internally by the attitude controller need a cleanup. The attitude setpoint in manual stabilized is also pubished and subscribed in the same module and so every generated setpoint goes through uORB first. I can revisit that in #11308

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants