Battery: fix MAVLink wrap when battery percentage is above 100%

[durka]

Bug report

Issue details

I've got a poor current sensor that usually reports a negative current (~ -2A) at idle. The battery backend integrates current to keep track of consumed mAh, then subtracts this from the known pack capacity to get a percentage. If the current is negative, this consumed mAh value is nonsense.

This produces cascading problems because capacity_remaining_pct caps the returned percentage to 255, but GCS_Common.cpp casts to an int8, which means with my bad sensor the percentage will climb to 127%, then go negative, eventually ending up at -1 which is supposed to be the sentinel value for "I don't have a battery".

Since the sensor isn't producing meaningful data on the ground I don't see a better solution than thresholding it at zero so at least we don't integrate negative values.

Version
Old master (forked 2020-06-15), but the relevant files (AP_BattMonitor_Backend.cpp, AP_BattMonitor_Analog.cpp) doesn't seem to have meaningfully changed

Platform
[x] All
[ ] AntennaTracker
[ ] Copter
[ ] Plane
[ ] Rover
[ ] Submarine

Airframe type
Quad-X

Hardware type
CubeBlack

Logs
Log attached:
log_64_2020-11-18-12-40-20.bin.zip
The battery status starts out at CurrTot=-5772.76, with the pack capacity at 16000, so we are going to calculate 136%, which gets reported over mavlink as -120% I believe.

[IamPete1]

I don't think we can just ignore negative current, it probably would be fair to constrain the capacity to not go over the set capacity. Ie limit to a max of 100%

[durka]

I think both are nonsensical (percentage over 100% and negative integrated energy). Unless a current sensor really could read negative if the battery were plugged into a charger or something?

[magicrub]

Onboard charging is a thing and measuring it is very important. We have gas powered electric generator libraries for had planes and solar aircraft are getting more popular

[durka]

OK fair enough. Capping it at 100% seems reasonable. I think the algorithm should be enhanced in other ways, like if the voltage is close to BATT_LOW_VOLT it's clearly not 100% but that's another topic.

[IamPete1]

Thinking about this more, I think percentages over 100 should also be allowed if your generator goes mad and starts overcharging it might happen.

But we should certainly fix the wrap issue it MAVLink. I'm going to update the title to reflect that.

[IamPete1]

If anyone fancys taking this one the code is here:

ardupilot/libraries/GCS_MAVLink/GCS_Common.cpp

Line 263 in 8bd04c1

battery.capacity_remaining_pct(instance),

We should add a check just like the current, mah and wh ones a few lines above.

[peterbarker]

Pretty sure this one's already being tackled here: #13712

[Allan0820]

@peterbarker Is this issue still open ? Would like to try solving it.

[IamPete1]

@Allan0820 Yes, this is still a issue, a fix would be great!

[shiv-tyagi]

Is anyone working on this issue? If not, I would love to give it a try.

[IamPete1]

@shiv-tyagi I don't think we have a open PR for this, if you could fix that would be great. Thanks for helping us close all these issues!

[shiv-tyagi]

@IamPete1 Pleasure is all mine. I added a check in GCS_Common. I hope this fixes this issue. Please review the PR #18731

[IamPete1]

Closed by #18731, Thanks @shiv-tyagi!