Pid update

[stephanbro]

• Changes Z command to a float to allow negative values
• Change errorMax to outputLimit in the PID module and does as the name implies
• Smarter cap on the integral
• Add roll, pitch, and yaw rate limits
• Tweak the ordering of assignment in the commander module to make things more explicit
• Properly set PID outputLimit so a parameter can change the value

Also see PSVL/lps-ros#1 and whoenig/crazyflie_ros#49

[whoenig]

Hi,
I was wondering what the benefit of the float-thrust value is. I see that this allows negative values now, but this doesn't have any physical meaning as the rotors can not spin in opposite directions?
Thanks,
Wolfgang

[stephanbro]

It's true that the thrust cannot go negative, but within the crazyflie-firmware the name thrust is somewhat of a misnomer, as it is used for direct/scaled thrust, z velocity, or z position. The value should be properly limited based on what the flight mode is within the crazyflie-firmware.

[ataffanel]

Hi,
Thanks for the PR, the PID related things looks good!

For the commander packer: we will not change the current commander packet since too many client are currently dependent on it. CRTP has port and channels to route packets, the current plan is to create a new commander packet format on a new channel (see #113). The current usage of thrust as height setpoint is a hack made to work until we have the new commander packet, going forward we should go with well formed packets using SI units.

If you remove the commander part I can merge the PR and lets talk on #113 on how to go forward with the commander packet. I had grand plan to revamp the code but we can just add a new packet quickly to get the functionality in and clean the implementation later. Basically anything as long as we do not break the outside interface :).

[theseankelly]

Huge +1 to not changing the current commander packet!!

[theseankelly]

I'm curious why this was originally 1000

[tobbeanton]

Yep good question. The brushed motors don't start spinning until ~3000 so I think it was initially intended so that power isn't consumed without any movement. When it spinns it can go lower so that is were 1000 comes from.

[stephanbro]

Interesting. Is this a change you'd like me to keep @tobbeanton @ataffanel ?

[theseankelly]

I'd prefer to keep the thrust lock -- it's a good safety feature, especially when using transmitters with a non-spring loaded throttle that might not be at zero on power-on, and with BigQuad builds.

[stephanbro]

Understandable. I'll remove this along with the other commander changes I've made.

[stephanbro]

That makes sense. I can remove the non-PID related changes and close the other pull requests.

[ataffanel]

Hi, Thanks! Tested and merged.