-
Notifications
You must be signed in to change notification settings - Fork 17.4k
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
improved implementation of tailsitter bodyframe-roll control #20855
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK. This is your input function that is not widely used, I think this has been tested by losawing? Basically if your happy with it then I'm happy for it to be merged.
No news about that? |
I can't review this PR. |
@lthall Thanks for responding; I assume you don't have a problem with it being merged. |
Sorry, that statement was too short. I can't review this PR because I don't understand the original code design, so I can't evaluate the effectiveness of the changes. While I often get asked to review code I don't fully understand. I do so if there are parts of the code that are in areas I am responsible for and I feel the other reviewers can cover the areas I do not feel I can review effectively. I also don't want to stifle the development of other aircraft and in this case there are elements in the control structure that I do not understand and the comments don't explain. So I can't provide any value with my review, for or against. |
I had hoped that the VPython implementation and visualizations (linked in the first comment) would clearly show the intent of the code and the differences between the two implementations. But it's also clear that the quaternion manipulations in function bodyFrameRoll_eulerPitchYaw() are very difficult to understand hence probably more complicated than necessary :) |
Tested today in reality and in QStabilize I had an unpleasant behavior. |
@robustini Thanks for testing. |
@kd0aij Damn Mark, you're right, thanks! |
graphical visualizations of the current implementation showed unwanted coupling of pitch and yaw to roll:
https://www.glowscript.org/#/user/kd0aij/folder/QuadPlane/program/bodyframe-master
updated version makes bodyframe-roll independent of pitch and yaw:
https://www.glowscript.org/#/user/kd0aij/folder/QuadPlane/program/bodyframe-roll-update
Also reduced the yaw error threshold from 90 to 30 degrees to reduce dynamic yaw (heading) errors at the expense of yaw rate tracking.
Flight test by @losawing: https://discuss.ardupilot.org/t/dual-motor-tailsitters/15302/2496