BossHobby / QUICKSILVER

Flight Controller Firmware
MIT License
171 stars 40 forks source link

Add option to make angle yaw not around gravity #40

Closed Quick-Flash closed 1 year ago

Quick-Flash commented 2 years ago

Add an option to make angle mode yaw act on the quads yaw rather than gravities yaw. This will add an option to allow for more of an Emu/BF feel to angle mode. Won't effect how well QS handles crashes or any other angle mode improvements QS has made. From personal experience Emu users tend to prefer this yaw feeling compared to the yaw around gravity feeling. Would be a nice option to help with adoption. And no this isn't an effort to make this like BF or to copy BF. Its simply an effort to allow me to more happily convert my whoops over to QS, haha.

bkleiner commented 2 years ago

if this is to be a feature to increase adoption by removing migration churn, it should really be a runtime configurable thing.

Quick-Flash commented 2 years ago

if this is to be a feature to increase adoption by removing migration churn, it should really be a runtime configurable thing.

Any standard operating procedure for making something runtime configurable?

bkleiner commented 2 years ago

if this is to be a feature to increase adoption by removing migration churn, it should really be a runtime configurable thing.

Any standard operating procedure for making something runtime configurable?

add a new member to the profile struct, both in the struct itself and in the appropriate *_MEMBERS define.

although not perfectly fitting, the pid section is probably most appropriate. https://github.com/BossHobby/QUICKSILVER/blob/master/src/main/config/profile.h#L124

this will make it available in the variable profile.pid.<whatever for you to switch on in the codebase and automatically generate the cbor encoding/decoding code to expose it via usb.

this will already allow you to change the value in the profile*.json file as downloaded via the config, adding a new toggle input to the configurators vue code would be the next step

Quick-Flash commented 2 years ago

this will already allow you to change the value in the profile*.json file as downloaded via the config, adding a new toggle input to the configurators vue code would be the next step

I'm a complete noob when it comes to dealing with configurators, but i can see what I can do, haha, thanks for pointing me in the right direction.

bkleiner commented 2 years ago

this will already allow you to change the value in the profile*.json file as downloaded via the config, adding a new toggle input to the configurators vue code would be the next step

I'm a complete noob when it comes to dealing with configurators, but i can see what I can do, haha, thanks for pointing me in the right direction.

i can take care of that part in an upcoming relase, would be good tho if the firmware side was already done tho.

NotFastEnuf commented 2 years ago

Silverware originally maintained yaw around the quads Z axis. The change was made to yaw around the gravity vector because when a yaw is performed around the z axis - it just dumps a ton of orientation error into roll and pitch. The gravity vector system was created to alleviate the system of being flooded with error every time a yaw manuver is performed. This change was a massive improvement to angle mode performance and handling. It is also worth noting that angle pid system uses angle error to move a setpoint weight between two sets of angle pids (high and low error). Flooding roll and pitch axis with error is going to also cause a response by the angle pid system as this setpoint weight will shift on roll and pitch with every yaw input until the dumped error is eliminated. This is going to impact stick feel in unexpected and unpredictable ways especially since different users run different high and low error angle pids.

The end result of yaw around gravity vector vs Z axis is going to be the same eventually anyway. The quad will find the same orientation in both cases eventually. Yaw around gravity will use math to get to the right spot faster and with less error in the system - yaw around Z axis will rely on multiple loop cycles gradually moving the quad to eliminate the yaw dumped error to get to the same place.

This approach seems inadvisable to me. It is going to create unpredictable impact to flight feel, crash handling, and the quad's ability to stabilize itself based on user set angle pids.

On Wed, Dec 15, 2021, 12:22 PM Benedikt Kleiner @.***> wrote:

this will already allow you to change the value in the profile*.json file as downloaded via the config, adding a new toggle input to the configurators vue code would be the next step

I'm a complete noob when it comes to dealing with configurators, but i can see what I can do, haha, thanks for pointing me in the right direction.

i can take care of that part in an upcoming relase, would be good tho if the firmware side was already done tho.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/BossHobby/QUICKSILVER/pull/40#issuecomment-995002554, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIRZCHPSUUW2C5LQYFLCSCLURDFF3ANCNFSM5KCO4WKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Quick-Flash commented 2 years ago

This approach seems inadvisable to me. It is going to create unpredictable impact to flight feel, crash handling, and the quad's ability to stabilize itself based on user set angle pids.

I disagree, I've tested both approaches before. It does not create an unpredictable flight feel, in fact for many users they feel that yaw feels more predictable. This is simply an option given to users and should not create any problems for users choosing it over the traditional method. Some will find it preferable, others will not. I'm also looking to change how the code calculates its angle error, removing some issues that euler angles bring to play, and a yaw around gravity type approach isn't the most compatible with this. So I definitely feel this is worth letting users decide what they want.

NotFastEnuf commented 2 years ago

Every yaw input is going to change the active roll and pitch angle pids. This is going to impact stick feel. It will create unpredictable results for different users because different users run different values for these and because different amounts of error will be dumped into roll and pitch based on different amounts of yaw command or different preceding roll and pitch orientations when yaw is commanded. You're going to need to toss out the high error/low error angle pid system (and its resulting improved crash recovery) along with this.

On Wed, Dec 15, 2021, 2:02 PM Kevin Plaizier @.***> wrote:

This approach seems inadvisable to me. It is going to create unpredictable impact to flight feel, crash handling, and the quad's ability to stabilize itself based on user set angle pids.

I disagree, I've tested both approaches before. It does not create an unpredictable flight feel, in fact for many users they feel that yaw feels more predictable. This is simply an option given to users and should not create any problems for users choosing it over the traditional method. Some will find it preferable, others will not. I'm also looking to change how the code calculates its angle error, removing some issues that euler angles bring to play, and a yaw around gravity type approach isn't the most compatible with this. So I definitely feel this is worth letting users decide what they want.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/BossHobby/QUICKSILVER/pull/40#issuecomment-995094349, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIRZCHJ6VYEKWFUTPY4BZ5LURDQ3ZANCNFSM5KCO4WKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Quick-Flash commented 2 years ago

Every yaw input is going to change the active roll and pitch angle pids. This is going to impact stick feel. It will create unpredictable results for different users because different users run different values for these and because different amounts of error will be dumped into roll and pitch based on different amounts of yaw command or different preceding roll and pitch orientations when yaw is commanded. You're going to need to toss out the high error/low error angle pid system (and its resulting improved crash recovery) along with this.

That's totally wrong. I have used this exact system before with the high error/low error pid system and ended up with incredible crash recovery. I rewrote the way that angle mode works in emu, and took a bunch of ideas from silverware. This will work and some users will like it. Some will not. This doesnt make angle mode unflyable all of a sudden. Itll still handle crashes and itll still be predictable on the sticks. I do even think that this would help with the yaw problem mr shutterbug mentioned a while back, given that he is used to this style more. There is nothing wrong about this version or the way quicksilver currently does things. This is just some more options for users that may want things to feel a little different.

bkleiner commented 2 years ago

Let's make one thing very clear:

As this is an optional thing, all changes in this or following PRs can not degrade the performance of the gravity vector based approach, that one is definitely here to stay, and will stay the primary one going forward.

Furthermore i am not keen on building two distinct and competing systems in one firmware. However obviously there different schools of thought on how the rotation around yaw should work so i am fine with having two variants for that, but everything down the line must be made in a way that it works for both.

Quick-Flash commented 2 years ago

Furthermore i am not keen on building two distinct and competing systems in one firmware.

I don't either. This is really just a simple option, if some users like it, then I think it has a place. If not then it should go the way of the dodo.

bkleiner commented 1 year ago

Closing as outdated. If we were to got forward, we can pull changes manually.