Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

battery - fix BATTERY_NOT_PRESENT detection, detection logic change #13599

Merged
merged 3 commits into from
May 4, 2024
Merged

battery - fix BATTERY_NOT_PRESENT detection, detection logic change #13599

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72fe040
battery - fix BATTERY_NOT_PRESENT detection, detection logic change
ledvinap Apr 28, 2024
6495218
battery - improve stable voltage detection
ledvinap Apr 29, 2024
4ec6328
battery - fuix typos, improve comments
ledvinap Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 11 additions & 15 deletions src/main/sensors/battery.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@
*
*/

#define VBAT_STABLE_MAX_DELTA 10 // 100mV
#define LVC_AFFECT_TIME 10000000 //10 secs for the LVC to slowly kick in

// Battery monitoring stuff
Expand Down Expand Up @@ -159,20 +158,14 @@ 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()) {
voltageMeter.isVoltageStable = abs(voltageMeter.prevDisplayFiltered - voltageMeter.displayFiltered) <= VBAT_STABLE_MAX_DELTA;
}

voltageMeter.prevDisplayFiltered = voltageMeter.displayFiltered;
voltageMeter.prevDisplayFilteredTime = millis();
}
voltageStableUpdate(&voltageMeter);

DEBUG_SET(DEBUG_BATTERY, 0, voltageMeter.unfiltered);
DEBUG_SET(DEBUG_BATTERY, 1, voltageMeter.displayFiltered);
DEBUG_SET(DEBUG_BATTERY, 4, voltageMeter.isVoltageStable ? 1 : 0);
DEBUG_SET(DEBUG_BATTERY, 3, voltageMeter.voltageStableBits);
DEBUG_SET(DEBUG_BATTERY, 4, voltageIsStable(&voltageMeter) ? 1 : 0);
DEBUG_SET(DEBUG_BATTERY, 5, isVoltageFromBattery() ? 1 : 0);
DEBUG_SET(DEBUG_BATTERY, 6, voltageMeter.prevDisplayFiltered);
DEBUG_SET(DEBUG_BATTERY, 6, voltageMeter.voltageStablePrevFiltered);
DEBUG_SET(DEBUG_BATTERY, 7, voltageState);
}

Expand Down Expand Up @@ -205,7 +198,9 @@ bool isVoltageFromBattery(void)

void batteryUpdatePresence(void)
{
if ((voltageState == BATTERY_NOT_PRESENT || voltageState == BATTERY_INIT) && isVoltageFromBattery() && voltageMeter.isVoltageStable) {
if ((voltageState == BATTERY_NOT_PRESENT || voltageState == BATTERY_INIT)
&& isVoltageFromBattery()
&& voltageIsStable(&voltageMeter)) {
// Battery has just been connected - calculate cells, warning voltages and reset state

consumptionState = voltageState = BATTERY_OK;
Expand All @@ -232,11 +227,12 @@ void batteryUpdatePresence(void)
batteryCriticalHysteresisVoltage = (batteryCriticalVoltage > batteryConfig()->vbathysteresis) ? batteryCriticalVoltage - batteryConfig()->vbathysteresis : 0;
lowVoltageCutoff.percentage = 100;
lowVoltageCutoff.startTime = 0;
} else if (voltageState != BATTERY_NOT_PRESENT && voltageMeter.isVoltageStable && !isVoltageFromBattery()) {
} else if (voltageState != BATTERY_NOT_PRESENT
&& voltageIsStable(&voltageMeter)
&& !isVoltageFromBattery()) {
/* battery has been disconnected - can take a while for filter cap to disharge so we use a threshold of batteryConfig()->vbatnotpresentcellvoltage */
consumptionState = voltageState = BATTERY_NOT_PRESENT;

voltageMeter.isVoltageStable = false;

batteryCellCount = 0;
batteryWarningVoltage = 0;
batteryCriticalVoltage = 0;
Expand Down
31 changes: 28 additions & 3 deletions src/main/sensors/voltage.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <stdbool.h>
#include <stdint.h>
#include <string.h>
#include <stdlib.h>

#include "platform.h"

Expand Down Expand Up @@ -89,10 +90,10 @@ const uint8_t supportedVoltageMeterCount = ARRAYLEN(voltageMeterIds);
void voltageMeterReset(voltageMeter_t *meter)
{
meter->displayFiltered = 0;
meter->prevDisplayFiltered = 0;
meter->prevDisplayFilteredTime = 0;
meter->voltageStablePrevFiltered = 0;
meter->voltageStableLastUpdate = 0;
meter->voltageStableBits = 0;
meter->unfiltered = 0;
meter->isVoltageStable = false;
#if defined(USE_BATTERY_VOLTAGE_SAG_COMPENSATION)
meter->sagFiltered = 0;
#endif
Expand Down Expand Up @@ -362,3 +363,27 @@ void voltageMeterRead(voltageMeterId_e id, voltageMeter_t *meter)
voltageMeterReset(meter);
}
}

// update voltageStableBits
// new bit is shifted in every VOLTAGE_STABLE_UPDATE_MS
// bit 1 is shifted in when voltage did not change more than VOLTAGE_STABLE_MAX_DELTA
// bit 0 is shifted in othervise
void voltageStableUpdate(voltageMeter_t* vm)
{
const uint32_t now = millis();
if (abs(vm->voltageStablePrevFiltered - vm->displayFiltered) > VOLTAGE_STABLE_MAX_DELTA) {
// reset stable voltage reference
vm->voltageStablePrevFiltered = vm->displayFiltered;
vm->voltageStableBits &= ~BIT(0); // woltage threshold exceeded in this period
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: woltage
i would rather define a enum, ~BIT(0) is not a 100% selfexplenatory term: IS_STABLE in this case.

typedef enum voltageStable_e {
  VOLTAGE_UNSTABLE = 0,
  VOLTAGE_IS_STABLE = ~VOLTAGE_UNSTABLE
} voltageStable_t;

or something like this. Or do we already have a global enum type for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just clearing of lowest bit. IMO &= ~(bits) is quite idiomatic in C and used in lot of places of BF.
I'll update the comment

}
if (cmp32(now, vm->voltageStableLastUpdate) >= VOLTAGE_STABLE_TICK_MS) {
vm->voltageStableBits = (vm->voltageStableBits << 1) | BIT(0); // start with 'stable' state
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is | BIT(0) really necessary as << 1 shifts a 0 already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It it bit zero (LSB), not bit with value of 0. New 1 bit is shifted in on each update and possibly cleared above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

vm->voltageStableLastUpdate = now;
}
}

// voltage is stable when it was within VOLTAGE_STABLE_MAX_DELTA for 10 update periods
bool voltageIsStable(voltageMeter_t* vm)
{
return __builtin_popcount(vm->voltageStableBits & (BIT(VOLTAGE_STABLE_BITS_TOTAL + 1) - 1)) >= VOLTAGE_STABLE_BITS_THRESHOLD;
}
23 changes: 17 additions & 6 deletions src/main/sensors/voltage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
#define SLOW_VOLTAGE_TASK_FREQ_HZ 50
#define FAST_VOLTAGE_TASK_FREQ_HZ 200

#define PREV_DISPLAY_FILTERED_TIME_DIFF 500 // ms
#define VOLTAGE_STABLE_TICK_MS 100
#define VOLTAGE_STABLE_BITS_TOTAL 11 // number of samples to test
#define VOLTAGE_STABLE_BITS_THRESHOLD 10 // number of samples tham must be within delta (~1s)
#define VOLTAGE_STABLE_MAX_DELTA 10 // 100mV

STATIC_ASSERT(VOLTAGE_STABLE_BITS_TOTAL < sizeof(uint16_t) * 8, "not enough voltageStableBits");

//
// meters
Expand All @@ -43,11 +48,11 @@ extern const char * const voltageMeterSourceNames[VOLTAGE_METER_COUNT];
// WARNING - do not mix usage of VOLTAGE_METER_* and VOLTAGE_SENSOR_*, they are separate concerns.

typedef struct voltageMeter_s {
uint16_t displayFiltered; // voltage in 0.01V steps
uint16_t prevDisplayFiltered; // voltage in 0.01V steps
timeMs_t prevDisplayFilteredTime;
bool isVoltageStable;
uint16_t unfiltered; // voltage in 0.01V steps
uint16_t displayFiltered; // voltage in 0.01V steps
uint16_t voltageStablePrevFiltered; // last filtered voltage sample
timeMs_t voltageStableLastUpdate;
uint16_t voltageStableBits; // rolling bitmask, bit 1 if battery difference is within threshold, shifted left
uint16_t unfiltered; // voltage in 0.01V steps
#if defined(USE_BATTERY_VOLTAGE_SAG_COMPENSATION)
uint16_t sagFiltered; // voltage in 0.01V steps
#endif
Expand Down Expand Up @@ -116,6 +121,12 @@ void voltageMeterESCRefresh(void);
void voltageMeterESCReadCombined(voltageMeter_t *voltageMeter);
void voltageMeterESCReadMotor(uint8_t motor, voltageMeter_t *voltageMeter);

//
// api for battery stable voltage detection
//

void voltageStableUpdate(voltageMeter_t* vm);
bool voltageIsStable(voltageMeter_t* vm);

//
// API for reading/configuring current meters by id.
Expand Down