AP_BattMonitor: Do not report remaining battery percentage if battery capacity is set to 0 #13712

Williangalvani · 2020-03-02T20:03:30Z

Sub had a regression from 3.5.4 where QGC now periodically complains about low battery.

According to the Mavlink specs this should be a int8_t and return -1 if there is no percentage counting:
https://mavlink.io/en/messages/common.html#BATTERY_STATUS
https://mavlink.io/en/messages/common.html#SYS_STATUS

WickedShell · 2020-03-02T21:04:24Z

You need to go address the associated spots that access this to cope with a return value of -1. A lot of them use it as a uint8_t and the previous behaviour was okay for them, but showing 255 will be worse/breaking for them. (Mostly it's AP_Notify/OSD and Lua bindings that need to be updated).

Williangalvani · 2020-03-05T14:03:56Z

@WickedShell updated.
I'm not sure I got the right formatting strings for AP_OSD and AP_Notify.

libraries/AP_BattMonitor/AP_BattMonitor_Backend.cpp

libraries/AP_OSD/AP_OSD_Screen.cpp

libraries/AP_BattMonitor/AP_BattMonitor.cpp

libraries/AP_BattMonitor/AP_BattMonitor.h

WickedShell · 2020-03-16T22:55:37Z

Sorry I didn't make the comment when I made my earlier one. The best change here would be to make capacity_remaining_pct return a boolean, and fill in the percentage by reference. This would remove the special case on -1 in a lot of places (although some external places like MAVLink/FrSky would still need to just send out a -1 in that case).

If you don't want to take that on here we can open an issue for it, and tag it as a good first issue.

Williangalvani · 2020-04-16T20:11:47Z

@WickedShell is this what you had in mind?

I have no idea of how to fix the Lua part doing it like this

libraries/AP_Frsky_Telem/AP_Frsky_Telem.cpp

Williangalvani · 2020-04-29T13:53:00Z

@WickedShell can you take a look now?

Williangalvani · 2020-07-30T16:53:53Z

@WickedShell rebased

rmackay9 · 2020-07-30T23:41:07Z

@Williangalvani,

By the way, if you're having troubles getting this in you can also add the DevCallTopic tag or the DevCallEU label and then attend one of the two weekly PR meetings. The EU one is new, there's a blog post with the timing on ardupilot.org.

libraries/AP_BattMonitor/AP_BattMonitor.h

libraries/AP_Frsky_Telem/AP_Frsky_Telem.cpp

libraries/GCS_MAVLink/GCS_Common.cpp

peterbarker

Pretty sure we should change that parameter to be unsigned now. There are several cases in here where bad things are going to happen if it is negative, and making it unsigned in all the corners gives them assurances they don't need to worry about that case.

Apologies for flip-flopping on this.

libraries/AP_Frsky_Telem/AP_Frsky_SPort.cpp

libraries/AP_OSD/AP_OSD_Screen.cpp

peterbarker · 2021-08-30T04:13:39Z

libraries/AP_BattMonitor/AP_BattMonitor_Logging.cpp

+    uint8_t percent = -1;
+    IGNORE_RETURN(capacity_remaining_pct(percent));


This is a change in behaviour - we're now going to log 255 rather than zero here.

I'm actually OK with that change.

Yes, I think there's more information in 255 than a zero that could be mistaked by an empty battery.

peterbarker · 2021-08-30T04:16:11Z

libraries/AP_Frsky_Telem/AP_Frsky_D.cpp

-        send_uint16(DATA_ID_FUEL, (uint16_t)roundf(_battery.capacity_remaining_pct())); // send battery remaining
+        uint8_t percentage = 0;
+        IGNORE_RETURN(_battery.capacity_remaining_pct(percentage));
+        send_uint16(DATA_ID_FUEL, (uint16_t)roundf(percentage)); // send battery remaining


Does this strike you as a bit weird?

We convert an integer to a float, round it, then cast it back into a uint16_t.

Similarly elsewhere.

Pre-existing, so not your problem to fix.

libraries/GCS_MAVLink/GCS_Common.cpp

issues seem to have been addressed now

Williangalvani mentioned this pull request Mar 3, 2020

Low battery warning mavlink message can fill sd card screen instance bluerobotics/companion#317

Closed

Williangalvani force-pushed the fix_percentage branch from 9b7dbb7 to 762ccb2 Compare March 4, 2020 13:18

Williangalvani requested a review from jaxxzer March 4, 2020 14:40

Williangalvani force-pushed the fix_percentage branch from 762ccb2 to 09f1a50 Compare March 4, 2020 16:30