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

Copter: Auto mode CONDITION_YAW treats neg relative angle with CCW direction as CCW #25646

Closed
wants to merge 2 commits into from

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented Nov 27, 2023

This makes Auto mode's handling of the CONDITION_YAW command slightly more intuitive in the case where the user has specified both a negative relative angle and a CCW direction (e.g. Deg = -90, Dir = -1, Relative=1).

Currently master interprets this double negative as a positive and will rotate the vehicle in the clockwise direction. With this change the double negative is interpreted as a negative and so the vehicle rotates in the CCW direction.

This PR also includes some drive-by formatting fixes.

This has been lightly tested in SITL and works as expected.
condyaw-fix-before-vs-after

Note that CONDITION_YAW can also be sent as an immediate command (e.g. while the vehicle is GUIDED mode) but that handler rejects negative angles so the double-negative case is never met. We could them consistent by removing these few lines and the modify the MAVLink spec (see related PR)

There is a related MAVLink PR here ArduPilot/mavlink#341 (related but no dependency)

This resolves issue #23874

Copy link
Contributor

@lthall lthall left a comment

Choose a reason for hiding this comment

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

I don't think this is correct. The CW/CCW define the positive direction and the angle is unambiguous.

This approach means we are changing the definition of angle based on the direction of "positive turn".

I think the current approach both makes sense and is "correct" from a mathematical/engineering perspective. Further the cost of making this mistake is low.

@IamPete1
Copy link
Member

We could add a pre-arm mission check for the double negative.

@lthall
Copy link
Contributor

lthall commented Nov 27, 2023

We could add a pre-arm mission check for the double negative.

Yes, This option would still be "correct". This would make it consistent with the mavlink yaw message I believe.

@rmackay9
Copy link
Contributor Author

rmackay9 commented Nov 27, 2023

OK, I'll go ahead and close this despite all the work I put into making the pretty PR description..

I've been slightly naughty and directly cherry-picked the formatting fix into master.

@rmackay9 rmackay9 closed this Nov 27, 2023
@rmackay9 rmackay9 deleted the copter-cond-yaw-auto-fix branch November 27, 2023 22:48
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