InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.71k stars 926 forks source link

Battery percentage spikes in GadgetBridge #608

Closed Riksu9000 closed 2 years ago

Riksu9000 commented 3 years ago

When disconnecting and reconnecting, Gadgetbridge will sometimes log incorrect values in the battery percentage graph. I don't know if this is an issue with Gadgetbridge or InfiniTime. EDIT: Seems like an issue with bluetooth communication in InfiniTime https://github.com/JF002/InfiniTime/issues/608#issuecomment-905805818

I'm running #580 which makes the watch notify the correct percentage every 10 minutes even if it's sleeping. This issue happens with or without this PR.

I opened an issue on Gadgetbridge side as well. https://codeberg.org/Freeyourgadget/Gadgetbridge/issues/2377

  1. Disconnection
  2. Reconnection. Wrong percentage logged
  3. Correct percentage sent by InfiniTime
  4. Disconnection and reconnection. Wrong percentage logged
  5. Correct percentage sent by InfiniTime

percentage_error

aemkai commented 3 years ago

The same issue here, also when connection gets lost (out of range) and reconnects. Remarkable is, that the wrong values seem to cover only some discrete values, at mine these are: 89 %, 78 %, 54 % and 41 % for top peaks and sometimes 57 % for down peaks. At first glance there is no pattern between these numbers, also on binary representation. (Maybe it could be approached a row with stepwidth 12: 90 - 78 - (66) - 54 - 42 - (30 - 18 - 6), but also this doesn't seem to make any sense.)

Akku GB

Riksu9000 commented 3 years ago

I think the peak might be "determined" at a disconnect or reconnect. As you can see from my screenshot, 1, 2 and 4 are at the same height, so somehow it remembers the percentage at 1. Similarly in your screenshot the peak is first reached normally, and then later it jumps back up to it for some reason.

This sounds like it could be an issue with Gadgetbridge, but wouldn't they have noticed such a glaring issue if it happened with watches other than the PineTime?

aemkai commented 3 years ago

Your right, it might be the first level after restart. Due to bluetooth issue (20h), I restart the watch every day. 7 resets a week give 7 levels in my screenshot.

I didnt checked code now, but does Infinitime store the batterie level after reconnect? Maybe Infinitime sends a default value, before ADC has sampled?

Riksu9000 commented 3 years ago

I didnt checked code now, but does Infinitime store the batterie level after reconnect? Maybe Infinitime sends a default value, before ADC has sampled?

BatteryController only stores the current percentage and I have checked that the value Gadgetbridge shows on reconnect isn't always the same as the one InfiniTime shows, so if the issue is in InfiniTime, it's probably somewhere in the bluetooth code.

aemkai commented 3 years ago

I found voltage only used in BatteryController.cpp, and on screens "BatterieInfo" and "SystemInfo". I don't understand init code of ADC to 100% and im not firm with this controller type, but on some controllers the first ADC sample has to be thrown away. I can't find code to discard the first sample - maybe this is the reason?

Riksu9000 commented 3 years ago

I doubt it's the ADC, because if the ADC randomly returns much higher percentages, this would be noticable in the BatteryInfo screen.

Also I noticed while the BatteryInfo screen was showing ~50% or something and then I manualy reconnected Gadgetbridge, GB showed ~60%, so it must have pulled the value from somewhere other than percentRemaining.

aemkai commented 3 years ago

I meant really the first conversion after ADC Init - bit in datasheet there isn't a corresponding notice. Despite there is mentioned a critical timing between DONE flag and data provision (nRF52832_PS_v1.4.pdf, pp 359 ff. : "In this mode, the RESULTDONE event has the same meaning as DONE when no oversampling takes place. Note that both events may occur before the actual value has been transferred into RAM by EasyDMA. For more information, see EasyDMA on page 361." Maybe data is read before available in RAM?

In BatteryInformationService (line 46 ff.) there the batteryValue is initialized from battery controller, there seems to be no room for mistakes. So it must be ADC or something a mishandling in bluetooth protocoll - I don'T see any other solution at the moment.

Riksu9000 commented 3 years ago

The ADC is uninitialized after each read and reinitialized at the start of each read, so if there was an issue, it would affect every read.

The SDK simplifies these events with nrfx_saadc_evt_type_t, which is different than nrf_saadc_event_t, so NRF_SAADC_EVENT_DONE is different than NRFX_SAADC_EVT_DONE, which means the data is ready to read.

The results of the ADC are only put into voltage and percentRemaining variables. Like I mentioned above, the bluetooth notification contains Gadgetbridge shows a different percentage than what is shown in BatteryInfo, so how would the current value and some old value exist in percentRemaining at the same time? The issue must be somewhere else.

tmilburn commented 3 years ago

I think it would be useful to make a note of which version of Gadget bridge you are running. Mine is 0.59.0 and like @aemkai it shows curved lines where there was no connection to the watch whereas @Riksu9000 yours has straight lines. My feeling is that this is an issue with Gadgetbridge and how it displays the logged data.

Riksu9000 commented 3 years ago

I have 0.59.1. Sometimes it shows curved lines for me too. Technically each line is curved, but the edge shows that a wrong value was reported at 2 in my first screenshot. If 2 wasn't logged, then everything would be correct and the line would be smoothed from 1 to 3. Here's a zoomed in picture that shows each point.

zoomed

aemkai commented 3 years ago

@Riksu9000 I think were at the same, that the infinitime code has no storage for old values For me, PercentRemaining are mostly the same: the watch differs sometimes some percentage points, but I think its jitter and the higher update rate on the watch compared to the update rate of GB. @tmilburn i have also such plateaus, depending on zoom in GB and length of not connected time GB-Verson is 0.59.0

Riksu9000 commented 3 years ago

the watch differs sometimes some percentage points, but I think its jitter and the higher update rate on the watch compared to the update rate of GB.

The reason that they can differ some percentage points is because they're not updated in sync, but with the PR I mentioned #580, they're always* in sync. I'm not sure if jitter is relevant here as the peaks and the 10% difference I mentioned (which are the same issue) aren't caused by jitter.

aemkai commented 3 years ago

@Riksu9000 "they're not updated in sync" This i meant with "jitter", the watch is updated faster and has more often different values caused by noise than transmitted to GB. My diction was wrong in that way, that jitter is a timing issue. Also i don't have differences in the order of 10%, but only single percentages.

Riksu9000 commented 3 years ago

Reading the battery level from nRF Connect also shows too high value, so I think this is most likely an issue with bluetooth communication in InfiniTime.

geekbozu commented 2 years ago

Should be fixed in PR #711.

geekbozu commented 2 years ago

Closed with PR #711