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

Remove recently added vehicle_attitude.timestamp check from MulticopterLandDetector::_get_maybe_landed_state(). #13072

Merged
merged 2 commits into from
Oct 2, 2019

Conversation

mcsauder
Copy link
Contributor

@mcsauder mcsauder commented Oct 2, 2019

Describe problem solved by the proposed pull request
PR #12681 added a check to the MulticopterLandDetector::_get_maybe_landed_state() method for a valid vehicle_attitude.timstamp value to finish work in PR #9756. It was discovered that the addition of that check leaves it possible to fly in acro mode without a valid attitude and auto-disarm can engage, allowing the multicopter to fall out of the sky.

Test data / coverage
This simply PR restores the previous logic currently in v1.9.2.

Thanks @bkueng for making this issue known!

Please let me know if you have any questions on this PR. Thanks!

-Mark

@mcsauder mcsauder requested a review from bkueng October 2, 2019 15:21
@dagar dagar added the bug label Oct 2, 2019
@mcsauder mcsauder changed the title Remove recently added vehicle_attitude.timestamp check from Multicopt… Remove recently added vehicle_attitude.timestamp check from MulticopterLandDetector::_get_maybe_landed_state(). Oct 2, 2019
@dagar dagar merged commit 508ffa1 into PX4:master Oct 2, 2019
@mcsauder mcsauder deleted the vehicle_attitude_timestamp branch October 3, 2019 02:11
@@ -178,7 +178,7 @@ bool MulticopterLandDetector::_get_maybe_landed_state()
const hrt_abstime now = hrt_absolute_time();

// When not armed, consider to be maybe-landed
if (!_actuator_armed.armed || (_vehicle_attitude.timestamp == 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

_vehicle_attitude is now unused and can be removed completely

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_vehicle_attitude is now unused and can be removed completely

Thanks @bkueng ! Fixed with #13134

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