Disable secondary power source by default, fix battery param migration (take 2) #15985
Conversation
updateBatteryStatus() already publishes
There was a problem hiding this comment.
I'd like to get feedback from @DonLakeFlyer and test myself, but otherwise the changes look good.
|
This is the first I'm hearing about a new BAT#_SOURCE parameter. The QGC Power setup currently does have that in the ui. I assume I need to add that. And then after that this looks like exactly what is needed. But where's the change to not send telemetry for a battery with BAT#_SOURCE=disabled. You can verify that QGC will be happy with telemetry by using daily build and you get the correct number of batteries in the toolbar. Also could someone run daily and look at Power setup to see if there is other stuff missing there? |
|
Are the voltage/current divider params only valid for some selection of BAT#_SOURCE? Or is that needs for all source types? |
|
I suggest adding a BAT_can_N_CELLS parameter to fix the uavcan battery bug. |
Fixes param migration, e.g. if BAT_N_CELLS is set, migrates to BAT1_N_CELLS.
Avoid a GCS showing 2 battery indicators. Alternatively we could also check the 'connected' flag, but this is more explicit.
The battery migration interferes with the tests.
bkueng
force-pushed
the
multi_battery_fixes
branch
from
367e042 to
ff3bcdb
Compare
No further change needed, that's already the effect of
I tested it against that. The default will now only show 1 battery, and enabling
Currently only for |
There was a problem hiding this comment.
I wonder how that happened, I remember testing all cases on hardware. Must have been a last minute change after that 😬
9c4e26c#diff-0eb86e4eec709144f8f3fa953148871cd33a1b6a2332316777c39260e22848a3L191-R195
Thanks for fixing the check @bkueng !
Apparently #15616 was still incorrect.
@DonLakeFlyer does this work for you?
Fixes #15929