Voltage detection method #13350

freasy · 2024-02-05T10:47:46Z

The isVoltageStable method before compared filtered and unfiltered voltage data to determine if the voltage is stable or not. This lead to multiple instances of wrong detected cell counts, thus profile switching based on cell count was screwed.

This PR changes it so, that the method always uses filtered data to check for stable voltage. Also, the previous value gets set in 1 second intervals. Also, I changed from 200mV to 30mV difference, because filtered values are way more stable.

To test this PR:
Set your quad up for BATTERY debug mode and observe Debug 4, which will go to 1 if the voltage is set stable. It should be some seconds later than before and at a state of really stable voltage.

github-actions · 2024-02-05T10:47:59Z

Do you want to test this code? You can flash it directly from Betaflight Configurator:

Simply put #13350 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

ledvinap · 2024-02-05T11:52:14Z

src/main/sensors/voltage.c

@@ -201,7 +206,11 @@ void voltageMeterADCRefresh(void)
 void voltageMeterADCRead(voltageSensorADC_e adcChannel, voltageMeter_t *voltageMeter)
 {
    voltageMeterADCState_t *state = &voltageMeterADCStates[adcChannel];
-
+
+    if (millis() - voltageMeter->prevDisplayTime > PREV_DISPLAY_FILTERED_TIME_DIFF) {


This is a bit problematic - value is stored at 1s intervals (samples at 50Hz), but comparison is done at batteryUpdatePresence (20Hz) rate. So distance between compared samples will be dependent on synchronization of both tasks (and then increased over 1s interval).

Either compare only once per second(per PREV_DISPLAY_FILTERED_TIME_DIFF) so that compared samples are PREV_DISPLAY_FILTERED_TIME_DIFF apart, or compare each filtered sample (roughly equivalent to current worst case) or use some better statistics (sum sqr() of differences over PREV_DISPLAY_FILTERED_TIME_DIFF interval?)

changed to sync with battery update task, changed a bunch, please review again

ledvinap · 2024-02-05T11:52:36Z

src/main/sensors/voltage.c

@@ -201,7 +206,11 @@ void voltageMeterADCRefresh(void)
 void voltageMeterADCRead(voltageSensorADC_e adcChannel, voltageMeter_t *voltageMeter)
 {
    voltageMeterADCState_t *state = &voltageMeterADCStates[adcChannel];
-
+
+    if (millis() - voltageMeter->prevDisplayTime > PREV_DISPLAY_FILTERED_TIME_DIFF) {


IMO batteryUpdateVoltage is better place for this.,

Also, cmp32 for timestamp with overflow (I know that 49days is a lot of time. Abut eventually someone will get bitten by it)

src/main/sensors/battery.c

freasy · 2024-02-05T19:59:32Z

@ledvinap i thought about the rss approach. I dont know if we actually need this, but i am about to make some examples and test if it is better for the usecase.

src/main/sensors/voltage.c

src/main/sensors/battery.c

ctzsnooze · 2024-02-06T05:56:32Z

With a threshold of 10 (100mv), and the suggested debug change, this code works very well now.

The maximum error from the stable value is probably less than 50mV, since by the time the difference between two values is only 100mV, the curve has become quite flat. In four tests the maximum difference was 30mV.

src/main/sensors/voltage.c

src/main/sensors/voltage.h

… resilient and synced to update battery task

freasy · 2024-02-06T09:53:52Z

squashed and ready to be tested :)

ctzsnooze · 2024-02-06T22:56:48Z

I've tested this carefully. On defaults, it is extremely accurate, and reliable, at detecting average voltage, typically within 30-40mV.

Even if the battery is very weak, and the motor 'chimes' cause significant voltage drops (such a battery would be un-flyable), the smoothing on defaults is good enough that an accurate reading is taken.

It takes between 3.0 and 4.0 seconds to return an accurate value, with the default vbat_display_lpf_period value of 30.

Choosing a shorter vbat_display_lpf_period will speed up the detection time.

Provided that the battery is good enough that there is no bad 'droop' during the motor chime period, and typically that is the case, a value of 15 will cut detection time to 2.0 to 2.5s, and we will have a reading in about the time that it takes for the motor chimes to stop.

If user experience shows that the detection is reliable with set vbat_display_lpf_period = 20, we could use that as the default.

Currently, if the pilot arms before the 'stable' voltage is measured, automatic profile selection will fail. There is no user indication that the stable voltage measurement is complete.

Hence the pilot must ensure that they wait about 4s from connecting the LiPo before arming - a couple of seconds after the motor chimes stop.

We could implement an arming block, with a warning message in OSD, while the user has enabled auto profile switching and the voltage detection period has not yet completed.

I'm not convinced that is necessary. People shouldn't be arming and taking off within 4s of plugging the battery in anyway; gyros need to be calibrated etc. If the display_period is 20, it's closer to 3s, and that should be OK for impatient people.

change voltage detection method and is voltage stable flag to make it resilient and synced to update battery task Co-authored-by: Eike Ahmels <ea@weslink.de>

haslinghuis · 2024-02-10T18:02:07Z

@freasy user on discord reported:

Beeper issue. It acts like the USB configuration is turned on when it isn't. PR13350 seems to have broken it. PR13352 is OK.

Support ID: 389823ab-8305-474c-82ea-0d3192b3e57

nerdCopter · 2024-02-19T14:04:18Z

@freasy , #13367

This reverts commit 744ab45.

ledvinap

Sorry, I missed this before merging

ledvinap · 2024-04-28T16:12:34Z

src/main/sensors/battery.c

@@ -159,8 +159,21 @@ void batteryUpdateVoltage(timeUs_t currentTimeUs)
            break;
    }

+    if (!voltageMeter.isVoltageStable && cmp32(millis(), voltageMeter.prevDisplayFilteredTime) >= PREV_DISPLAY_FILTERED_TIME_DIFF) {
+        if ((voltageState == BATTERY_NOT_PRESENT || voltageState == BATTERY_INIT) && isVoltageFromBattery()) {


Due to isVoltageFromBattery(), isVoltageStable is never set unless battery is connected.

ledvinap · 2024-04-28T16:13:39Z

src/main/sensors/battery.c

@@ -226,11 +232,11 @@ void batteryUpdatePresence(void)
        batteryCriticalHysteresisVoltage = (batteryCriticalVoltage > batteryConfig()->vbathysteresis) ? batteryCriticalVoltage - batteryConfig()->vbathysteresis : 0;
        lowVoltageCutoff.percentage = 100;
        lowVoltageCutoff.startTime = 0;
-    } else if (voltageState != BATTERY_NOT_PRESENT && isVoltageStable() && !isVoltageFromBat()) {
+    } else if (voltageState != BATTERY_NOT_PRESENT && voltageMeter.isVoltageStable && !isVoltageFromBattery()) {


This will never never be true: voltageMeter.isVoltageStable && !isVoltageFromBattery()

Detection logic is refactored - battery voltage delta is tested each 50ms, voltage is considered stable when difference of last 10 samples is smaller than 100mV This makes stable threshold more forgiving than betaflight#13350 (time for comparison is 50 instead of 500ms).

…13599) * battery - fix BATTERY_NOT_PRESENT detection, detection logic change Detection logic is refactored - battery voltage delta is tested each 50ms, voltage is considered stable when difference of last 10 samples is smaller than 100mV This makes stable threshold more forgiving than #13350 (time for comparison is 50 instead of 500ms). * battery - improve stable voltage detection - voltageStablePrevFiltered every time delta is exceeded - voltage within range is ANDed over 100ms periods - voltage is stable if it was within range for 10 out of 11 periods - slowly changing voltage will update threshold, but voltage will be considered stable - 1 update/s (100mV/s) is tolerated * battery - fuix typos, improve comments

…etaflight#13599) * battery - fix BATTERY_NOT_PRESENT detection, detection logic change Detection logic is refactored - battery voltage delta is tested each 50ms, voltage is considered stable when difference of last 10 samples is smaller than 100mV This makes stable threshold more forgiving than betaflight#13350 (time for comparison is 50 instead of 500ms). * battery - improve stable voltage detection - voltageStablePrevFiltered every time delta is exceeded - voltage within range is ANDed over 100ms periods - voltage is stable if it was within range for 10 out of 11 periods - slowly changing voltage will update threshold, but voltage will be considered stable - 1 update/s (100mV/s) is tolerated * battery - fuix typos, improve comments

…change (#13623) battery - fix BATTERY_NOT_PRESENT detection, detection logic change (#13599) * battery - fix BATTERY_NOT_PRESENT detection, detection logic change Detection logic is refactored - battery voltage delta is tested each 50ms, voltage is considered stable when difference of last 10 samples is smaller than 100mV This makes stable threshold more forgiving than #13350 (time for comparison is 50 instead of 500ms). * battery - improve stable voltage detection - voltageStablePrevFiltered every time delta is exceeded - voltage within range is ANDed over 100ms periods - voltage is stable if it was within range for 10 out of 11 periods - slowly changing voltage will update threshold, but voltage will be considered stable - 1 update/s (100mV/s) is tolerated * battery - fuix typos, improve comments Co-authored-by: Petr Ledvina <ledvinap@gmail.com>

sugaarK added the Bugfix label Feb 5, 2024

sugaarK added this to the 4.5 milestone Feb 5, 2024

sugaarK assigned freasy Feb 5, 2024

ctzsnooze requested review from ledvinap and ctzsnooze February 5, 2024 11:11

ledvinap reviewed Feb 5, 2024

View reviewed changes

freasy requested a review from ledvinap February 5, 2024 19:36

haslinghuis reviewed Feb 5, 2024

View reviewed changes

src/main/sensors/battery.c Outdated Show resolved Hide resolved

src/main/sensors/battery.c Outdated Show resolved Hide resolved

freasy requested a review from haslinghuis February 5, 2024 20:09

haslinghuis reviewed Feb 5, 2024

View reviewed changes

src/main/sensors/voltage.c Outdated Show resolved Hide resolved

haslinghuis added RN: BUGFIX and removed Bugfix labels Feb 5, 2024

ctzsnooze reviewed Feb 6, 2024

View reviewed changes

src/main/sensors/battery.c Outdated Show resolved Hide resolved

ctzsnooze reviewed Feb 6, 2024

View reviewed changes

src/main/sensors/battery.c Outdated Show resolved Hide resolved

ledvinap approved these changes Feb 6, 2024

View reviewed changes

src/main/sensors/voltage.c Outdated Show resolved Hide resolved

src/main/sensors/voltage.h Outdated Show resolved Hide resolved

change voltage detection method and is voltage stable flag to make it…

0453ef1

… resilient and synced to update battery task

freasy force-pushed the voltage_detection_method branch from 0b787c7 to 0453ef1 Compare February 6, 2024 09:53

freasy requested review from ctzsnooze and haslinghuis February 6, 2024 09:54

haslinghuis added the Testing Required label Feb 6, 2024

haslinghuis approved these changes Feb 6, 2024

View reviewed changes

haslinghuis requested a review from SteveCEvans February 6, 2024 15:40

ctzsnooze approved these changes Feb 6, 2024

View reviewed changes

haslinghuis added Tested and removed Testing Required labels Feb 6, 2024

haslinghuis merged commit 744ab45 into betaflight:master Feb 6, 2024
23 checks passed

nerdCopter mentioned this pull request Feb 19, 2024

beeper always on when connected to Configurator #13367

Closed

haslinghuis added a commit to haslinghuis/betaflight that referenced this pull request Apr 26, 2024

Revert "Voltage detection method (betaflight#13350)"

8754068

This reverts commit 744ab45.

ledvinap reviewed Apr 28, 2024

View reviewed changes

ledvinap mentioned this pull request Apr 28, 2024

battery - fix BATTERY_NOT_PRESENT detection, detection logic change #13599

Merged

haslinghuis mentioned this pull request May 4, 2024

[4.5.1] battery - fix BATTERY_NOT_PRESENT detection, detection logic change #13623

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Voltage detection method #13350

Voltage detection method #13350

freasy commented Feb 5, 2024

github-actions bot commented Feb 5, 2024

ledvinap Feb 5, 2024

freasy Feb 5, 2024

ledvinap Feb 5, 2024

freasy Feb 5, 2024

freasy commented Feb 5, 2024

ctzsnooze commented Feb 6, 2024

freasy commented Feb 6, 2024

ctzsnooze commented Feb 6, 2024

haslinghuis commented Feb 10, 2024

nerdCopter commented Feb 19, 2024

ledvinap left a comment

ledvinap Apr 28, 2024

ledvinap Apr 28, 2024

Voltage detection method #13350

Voltage detection method #13350

Conversation

freasy commented Feb 5, 2024

github-actions bot commented Feb 5, 2024

ledvinap Feb 5, 2024

Choose a reason for hiding this comment

freasy Feb 5, 2024

Choose a reason for hiding this comment

ledvinap Feb 5, 2024

Choose a reason for hiding this comment

freasy Feb 5, 2024

Choose a reason for hiding this comment

freasy commented Feb 5, 2024

ctzsnooze commented Feb 6, 2024

freasy commented Feb 6, 2024

ctzsnooze commented Feb 6, 2024

haslinghuis commented Feb 10, 2024

nerdCopter commented Feb 19, 2024

ledvinap left a comment

Choose a reason for hiding this comment

ledvinap Apr 28, 2024

Choose a reason for hiding this comment

ledvinap Apr 28, 2024

Choose a reason for hiding this comment