-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Configurable battery compensation #936
Comments
TLDR: This is important functionality, that should be updated and kept. This functionality is extremely important, especially when moving to a proper system ID. The goal for CF (and Bolt) in my mind is to have somewhere parameters that we can specify (inertia matrix, mass, arm-length, etc.). Controllers should output desired thrust and moment in SI units (or normalized SI units, e.g., 0..1 like in PX4). Then, a user can pre-tune the system in simulation. The system ID in the current firmware is unfortunately not very good. What is missing is an estimation of the total possible thrust (a function of the battery state actually). In my experiments, it also didn't match reality well - perhaps because it was measured a long time ago manually. The system-ID board that Tobi developed provides much more accurate results. The data from that is also what we currently use in the lab, with another controller that uses SI units, see IMRCLab/crazyflie-firmware@master...IMRCLab:rai-crazyswarm2-demo for the firmware. Using SI units has the advantage that its much easier to go between simulation and real drone. For Bolt, it might be nicer to switch to some sort of normalized SI units. |
Yes you have a good point 😄 but it seemed to be based on the voltage of the battery already is it not? But this is also talking about the location of this system id currently. Currently this is part of the motor.c, so a bit adhoc after the controller has done it's work already. Then it should be more intergrated into the controller framework to make it more logical. |
In my experiments, it not only depends on the voltage of the battery, but also the current PWM (low battery voltage can be because the battery is old, because it has little capacity left, or because the motor load is very high). Agreed that there should be some more central place where we define the system ID (mass, inertia matrix, arm length, etc.). Other parts can then simply include that file if they need parts of the system ID. |
Yes agreed. I've changed the title of the issue so we will be reminded to look at this eventually. Since we wanted to convert parameters to persistent, we noticed these hardcoded values here, so at one point we need to find a solution for it. One of the reason why I started the discussion in the first place. |
Mind that this battery compensation no longer holds for the thrust upgrade kit. |
So based on the discussion of PR #931 we looked at the motor battery compensation variables looking a bit hardcoded and very specific for the crazyflie:
crazyflie-firmware/src/drivers/src/motors.c
Lines 109 to 173 in b0c72f2
In history this functionality has been mostly used to achieve more stable hovering when using only Baro (or none at all) for attitude, so that the motors will give the same thrust while the battery is depleting. Now is the question if you want to keep this same functionality for other platforms as the Bolt.
However, now we live in the age of positioning, and technically this function would no longer be necessary with the z-ranger, flowdeck or any other deck that gives altitude positioning. If it happens to decline in height because of battery▶️ the state estimator catches that ▶️ the controller compensates for it ▶️ the motors start spinning faster.
This issue is about discussing about the functionality..... is it obsolete and should we remove it in the future? Or would this actually be useful for the Bolt or CF2.1/BQ deck combo and should we make the polynomial values persistent parameters with different default values.
The text was updated successfully, but these errors were encountered: