ardupilot icon indicating copy to clipboard operation
ardupilot copied to clipboard

Fix: Negative yaw rotation for LOCAL_FRD should be positive

Open bengtlofgren opened this issue 1 month ago • 1 comments

This fix addresses issue https://github.com/ArduPilot/ardupilot/issues/31651 using a TDD approach.

The sign error was showcased by adding a test in the math directory.

Commit history:

  1. Add test demonstrating the math error
  2. Fix the error

Found other places in the codebase that actually use the correct math:

https://github.com/ArduPilot/ardupilot/blob/master/ArduPlane/quadplane.cpp#L2173

https://github.com/ArduPilot/ardupilot/blob/master/libraries/AC_PrecLand/AC_PrecLand.cpp#L630

bengtlofgren avatar Dec 07 '25 09:12 bengtlofgren

Why did you submit a PR with a failing test? Concerned that this was AI generated and not actually written or checked properly.

The fix should be in #31659

tpwrules avatar Dec 10 '25 16:12 tpwrules

I couldn't get the test to run in my local environment so I tried to port something I'd written over to what was used in CI (asking claude to write it for me in the correct directory - the actual test logic was mostly hand written ...)- some docs on running CI tests locally would be helpful :)) @tpwrules Glad it was fixed

bengtlofgren avatar Dec 13 '25 15:12 bengtlofgren

Also #31659 would benefit from some tests to show why that works

bengtlofgren avatar Dec 13 '25 15:12 bengtlofgren

I couldn't get the test to run in my local environment so I tried to port something I'd written over to what was used in CI (asking claude to write it for me in the correct directory - the actual test logic was mostly hand written ...)- some docs on running CI tests locally would be helpful :)) @tpwrules Glad it was fixed

https://ardupilot.org/dev/docs/the-ardupilot-autotest-framework.html

peterbarker avatar Dec 13 '25 23:12 peterbarker

Also #31659 would benefit from some tests to show why that works

... and to make sure it doesn't break....

peterbarker avatar Dec 13 '25 23:12 peterbarker