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

Do not handle yaw stick multiple times in Altitude and Position mode #21781

Merged
merged 3 commits into from
Jul 3, 2023

Conversation

MaEtUgR
Copy link
Member

@MaEtUgR MaEtUgR commented Jun 30, 2023

Solved Problem

When helping @marcinolokk with a new slower Position mode input feature I realized that there is quite some duplication in yaw handling over FlightTaskManualAltitude and FlightTaskManualAcceleration.

I had unified the handling in the StickYaw class in #21192 but apparently did not follow through and only use it once for all cases.

Solution

  • Remove forgotten parameter metadata trailing zeroes from mc_pos_control: pass on parameter metadata #21729 (FYI @sfuhrer)
  • Remove duplicate instantiation and call to the StickYaw class in FlightTaskManaulAcceleration. It inherits the instance from FlightTaskManualAltitude and also calls the update by running FlightTaskManualAltitude's update already.
  • Remove all the legacy yaw handling from FlightTaskManualAltitude which was replaced by the unified StickYaw class.

Changelog Entry

The user should not notice a difference although this is a bug.

Cleanup: Do not handle yaw stick multiple times in Altitude and Position mode.

Test coverage

  • Simulation testing: I went through all the Altitude and Position mode variants with the MPC_POS_MODE settings and verified the yaw stick input still works. Now it must always be using the StickYaw class in FlightTaskManualAltitude only.

Copy link
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

Looks like a nice clean up to me! I didn't check that generateYawSetpoint() does 1:1 the same as the previous logic in _updateHeadingSetpoints(), but it now seems to be consistent over all manual modes which seems reasonable to me for heading control

@MaEtUgR MaEtUgR merged commit b7e2a9c into main Jul 3, 2023
82 of 85 checks passed
@MaEtUgR MaEtUgR deleted the maetugr/flight-task-yaw-duplication branch July 3, 2023 07:52
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.

None yet

3 participants