Improve SITL battery estimation #15034
Conversation
I agree. |
|
I reckon you could do it with a plugin too. But I don't see a problem with using the parameters also for battery simulation. |
| float ibatt = -1.0f; // no current sensor in simulation | ||
|
|
||
| battery_percentage = math::max(battery_percentage, _param_bat_min_pct.get() / 100.f); | ||
| battery_percentage = 1.0f - consumed_mah / _battery.capacity(); |
There was a problem hiding this comment.
Works, with the default capacity of -1 you end up with 1 - (0 / -1) = 1. If you only set an average current and no capacity you end up with a percentage > 1 which is capped in the next line. 👍
| battery_percentage = 1.0f - consumed_mah / _battery.capacity(); | |
| battery_percentage = 1.0f - (consumed_mah / _battery.capacity()); |
|
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
|
@ThomasRigi Would you want to rebase so we get this in? Please also check the review comments. |
|
If we don't want to introduce new params on the SITL side then we could also make use of the new RTL flight time param - for VTOL we though atm do not have any distinction between hover and cruise flight. |
|
Yes, I could rebase. Should I rename and move the parameters to https://github.com/PX4/PX4-Autopilot/blob/master/src/modules/simulator/battery_simulator/battery_simulator_params.c as they are only used in SITL? On our side we don't have any plans right now to do other things with them as we're doing most diagnostics in our ROS framework on the offboard computer. And as I see the battery-based RTL has already a different implementation now. |
|
@ThomasRigi Thanks for this PR! Any plans on updating it & getting it into the |
Describe problem solved by this pull request
Previously, the battery estimation in SITL wasn't realistic at all. There was no current and the battery drained at a fixed rate (SIM_BAT_DRAIN) until it reached a minimum level (SIM_BAT_MIN_PCT).
Describe your solution
This pull request uses new average current parameters (taken from @sfuhrer's #14819), integrates the current and compares the result with the total battery capacity to get the battery percentage.
If not armed, the current is set to zero.
If the average current parameters are set at their default value -1.0, the battery remains full. The same is true for the default battery capacity of -1.
Describe possible alternatives
Using average currents based on the vehicle type is a simple way to already increase the precision of battery estimation in SITL, but it's still only a rough approximation.
For the future you could scale the current with thrust for example (for climbing or flying faster), but I wanted to keep it simple for now.
Test data / coverage
Log of flight with gazebo_standard_vtol: https://logs.px4.io/plot_app?log=d1d132cf-69a2-494a-9af3-bb5d8ba1ae5f
gazebo_iris: https://logs.px4.io/plot_app?log=d0c07c75-3b73-4e75-9020-09bcfe63f704
gazebo_plane: https://logs.px4.io/plot_app?log=7504cccf-50a9-4bd1-bc69-6421f4d4fb71
gazebo_rover: https://logs.px4.io/plot_app?log=2687f5c7-3643-442a-8127-71fb0d6156b4
Iris with default battery capacity (-1): https://logs.px4.io/plot_app?log=3eead2b3-fbde-4009-8340-27ab4839802d
Additional context
When this gets merged the dev guide needs to be updated to reflect this change