Skip to content

mc_att_control_main: fix check for hover thrust estimate update #24750

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

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Apr 22, 2025

Solved Problem

When working on #24710 together with @dawr68 I added some suggestions and admittedly did this mistake which was apparently not found in review. It doesn't blow things up because the vehicle status is also regularly updated but it's of course completely wrong and hence worth a separate pr to fix.

See https://github.com/PX4/PX4-Autopilot/pull/24710/files#r2054448109

Solution

Updating the hover thrust estimate should be dependent on the correct topic's update.

Test coverage

I just found this while following up on reducing the parameter options like we discussed in the dev call. So it's purely based on reading the code again.

Copy link
Member

@bresch bresch left a comment

Choose a reason for hiding this comment

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

Ouch, good catch

@dakejahl dakejahl merged commit 9eaec53 into main Apr 22, 2025
65 of 66 checks passed
@dakejahl dakejahl deleted the maetugr/fix-hover-thrust-subscription-update-check branch April 22, 2025 23:53
Louis-max-H pushed a commit to Louis-max-H/PX4-Autopilot that referenced this pull request Jun 6, 2025
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