converted PI rate controller to floating point #1624
Conversation
| struct Int32Rates sum_err_increment; | ||
| RATES_SDIV(sum_err_increment, _error, 5); | ||
| struct FloatRates sum_err_increment; | ||
| RATES_SDIV(sum_err_increment, _error, 512.0); |
There was a problem hiding this comment.
Better use PERIODIC_FREQUENCY than hardcoded 512Hz..
|
Yes, for the message change, you will need to make a PR in pprzlink. You wanted to make the gains the "same" as in attitude stabilization in #1601, is that still the case? |
EwoudSmeur
force-pushed
the
rate_to_float
branch
from
3eab847
to
2dc5d02
Compare
|
Float attitude stabilization also has no scaling, so they are compatible. Stab att int still has scaling, so that is not directly compatible. In this PR I changed the integrator scaling, and changed the gains appropriately in the airframe files. Nothing changed to the P gains. |
|
So, can we merge this? @dewagter you still want to comment? |
|
For me it's fine to merge after paparazzi/pprzlink#12 |
|
also fine for me |
|
This was about "readability versus efficiency" ? -Christophe On Mon, Apr 25, 2016 at 6:29 PM, Felix Ruess notifications@github.com
|
|
Not only readability and simplification: If you have an MCU with FPU, then the float impl is actually faster as there are less operations... |
As requested, the rate controller in floating point. I also changed the integrator scale to be divided by 512.0 to allow easy operation with the (I believe) standard 512.0 Hz control frequency. Anyway, the idea being that you can easily calculate the command after 1 sec 1 rad/s error with the current I gain, for instance compared to the P gain. Now I think about it... maybe dividing by periodic frequency is even better...
On a side note, the messages are now also in float, so do I have to make a pull request to pprzlink as well? Why is this separate by the way?