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

Proper sign convention with motor mixing, handles inverted pitch #884

Closed
LESLIEWANGer opened this issue Nov 14, 2021 · 8 comments
Closed

Comments

@LESLIEWANGer
Copy link

Hi!
Recently I read the pid controller in CF's firmware(controller.c\pid.c and so on).I found a minus sign in line 114 in controller_pid.c and it confused me.As mentioned in the guidance,this plane follows the right-hand system so the Z-axis points up.Does it mean the "yaw rate data" from the imu points down as the same direction as gravity or any reason else?
Thank you!

71ad5bbcbf1793c92b6be39deb7ffda

[](url)
@knmcguire
Copy link
Member

Hi!

Indeed according to the right hand convention the minus looks off. From historical reasons, the state and the following PID controller evolved that way, as probably it is easier for non-aerospace people to recognize a negative pitch as going down and a positive pitch as going up. At least that's how I interpreted it.

The current coordinate system that depicts the state of the crazyflie can be found here, where you see that indeed the rotation around the y access is not according to the righthand notation and at one point, this probably will need to change.

The problem is, is that changing these coordinates will have quite a lot of implications on a lot of systems within the firmware at this point as it needs to be changed simultaneously in all state estimators, controllers, documentation and perhaps even more.

@LESLIEWANGer
Copy link
Author

Thank You!
I understand the rule about pitch.
Actually I was going to ask some questions about the "yaw" direction. That's to say, what‘s the meaning of the minus before "control->yaw"?
image
Thank you again!

@knmcguire
Copy link
Member

Ahh oke yeah I missed that line. It is indeed a bit odd so we will check it out... but it worked so far! We get back to you once we got a good look at it.

@knmcguire
Copy link
Member

To be completely honest, I can not find the reason why this yaw rate command is negative 😕 not in the cflib, or the gyroscope measurements.

@matejkarasek you looked at the yaw controller quite closely recently. Any thoughts?

@matejkarasek
Copy link
Contributor

matejkarasek commented Nov 22, 2021

Yeah, this has something to do with the sign conventions...
The control->yaw is later used by the power distribution (power_distribution_stock.c), where the signals going to individual motors are generated.
Just by looking at the power distribution formulas, it seems the sign convention for yaw of the power distribution is opposite to the CF2.X frame used elsewhere (ENU with opposite pitch):
https://www.bitcraze.io/images/documentation/overview/coordinate_system.jpg
E.g. motor 1 has a positive sign next to control->yaw.
https://github.com/flapper-drones/crazyflie-firmware/blob/c696d88b5b31debd9d83048c7eed10c39e86c505/src/modules/src/power_distribution_stock.c#L89
The M1 prop spins CCW when viewed from the top, so the reaction moment (which turns the crazyflie) will be CW. But positive yaw direction should be CCW, according to ENU...

Note the same sign change is done in Mellinger

control->yaw = clamp(-M.z, -32000, 32000);

And also in INDI there are multiple places where the yaw sign is changed, e.g. here:
attitudeDesired.yaw = -radians(attitudeDesired.yaw); //convert to radians and add negative sign to convert from ENU to NED

So, the current code is obviously correct but confusing...
We could:

  1. make changes to the power distribution, such that the sign convention is consistent with the rest (ENU with opposite pitch)
  2. Use a proper sign convention all across the code. ENU (with correct pitch) would likely be easier, NED would be what aerospace is using...

@knmcguire
Copy link
Member

Thanks for your elaborate answer @matejkarasek Ah yeah I didn't think to look in the power distribution that closely but yes ofcourse you are right! Ofcourse the code is correct or else it would have not been flying... but it's confusing indeed. I do believe that we have to discuss internally what the right approach here.

Personally I like the ENU with reversed pitch but that is since I come originally from ground based robotics where the up z-axis makes sense. And then the reversed pitch makes more sense to me for control, but that might be quite personal (in videogames with any type of airplane I reverse the pitch always for me). Also we need to make a decision if we want to distinguish the crazyflie as a robotic platform that happens to fly, or a fullblown quadcopter type that is never going to touch the ground in any form.

Anyway, this item has been marked by @jonasdn for a triage, so let's keep it open for that. I believe that we have provided enough info to make a decision about the next step. In the mean time, I hope that @LESLIEWANGer's questions have been resolved now?

@jonasdn
Copy link
Contributor

jonasdn commented Dec 7, 2021

So, what we have said from Bitcraze is that is not something that we are likely to work actively on. We are aware of these issues, and that the current code is a result of making stuff work. There are room for plenty of improvement to make the calculations, formulas and general flow of control more canonical.

We would be happy to look at contributions for people who want to make the code easier to follow!

@knmcguire knmcguire changed the title About pid control rule Proper sign convention with motor mixing, handles inverted pitch Mar 7, 2022
@knmcguire
Copy link
Member

I'll close this as this issue is related to the discussion issue we had before: #396. There is a bunch of stuff that needs to be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants