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

multicopter hover thrust estimator use vehicle_thrust_setpoint (work in stabilized/manual mode) #19633

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dagar
Copy link
Member

@dagar dagar commented May 11, 2022

Update the hover thrust estimator to use vehicle_thrust_setpoint (body frame) rather than vehicle_local_position_setpoint thrust (NED). This allows it to work in stabilized mode.

https://logs.px4.io/plot_app?log=6df3713e-f511-4e2e-854d-8fc7ab98ec65

TODO:

  • review usability and safety in stabilized mode
  • battery scaling?

@dagar dagar changed the title multicopter hover thrust estimator use vehicle_thrust_setpoint (work in stabilized mode) multicopter hover thrust estimator use vehicle_thrust_setpoint (work in stabilized/manual mode) May 11, 2022
@@ -166,10 +161,24 @@ void MulticopterHoverThrustEstimator::Run()

_hover_thrust_ekf.predict(dt);

vehicle_local_position_setpoint_s local_pos_sp;
vehicle_attitude_s vehicle_attitude{};
_vehicle_attitude_sub.copy(&vehicle_attitude);
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO: use all of these messages (vehicle_local_position, vehicle_attitude, vehicle_thrust_setpoint) on the same timestamp_sample?

Copy link
Member

Choose a reason for hiding this comment

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

Given that we don't take the motor lag into account (the model assumes instant acceleration), I don't think that aligning the data perfectly is important.

@dagar
Copy link
Member Author

dagar commented May 11, 2022

Should we go even lower level and use the allocated_thrust from control_allocator_status?

@bresch
Copy link
Member

bresch commented May 12, 2022

Should we go even lower level and use the allocated_thrust from control_allocator_status?

Il would improve the estimate on setups that are often saturating, yes. For the other ones, it shouldn't change anything.
Should we maybe change to that once we switch to the new control allocation system completely?

1 similar comment
@bresch
Copy link
Member

bresch commented May 12, 2022

Should we go even lower level and use the allocated_thrust from control_allocator_status?

Il would improve the estimate on setups that are often saturating, yes. For the other ones, it shouldn't change anything.
Should we maybe change to that once we switch to the new control allocation system completely?

@MaEtUgR
Copy link
Member

MaEtUgR commented Sep 18, 2024

Let's pick this up again. I've not seen the hover thrust estimator fail 👀

once we switch to the new control allocation system completely?

That's done 😃

@mahimayoga mahimayoga force-pushed the pr-mc_hover_thrust_stabilized branch from 0a90621 to 4d00981 Compare December 13, 2024 10:45
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