Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,008 changes: 442 additions & 566 deletions usermods/Battery/Battery.cpp

Large diffs are not rendered by default.

61 changes: 47 additions & 14 deletions usermods/Battery/UMBattery.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,61 @@ class UMBattery
{
private:

public:
/**
* Lookup table entry for voltage-to-percentage mapping.
* Table must be sorted descending by voltage.
*/
struct LutEntry { float voltage; float percent; };

protected:
float minVoltage;
float maxVoltage;
float voltage;
int8_t level = 100;
float calibration; // offset or calibration value to fine tune the calculated voltage
float voltageMultiplier; // ratio for the voltage divider

float linearMapping(float v, float min, float max, float oMin = 0.0f, float oMax = 100.0f)
{
return (v-min) * (oMax-oMin) / (max-min) + oMin;
}

float lutInterpolate(float v, const LutEntry* lut, uint8_t size)
{
if (size == 0) return 0.0f;

LutEntry first, last;
memcpy_P(&first, &lut[0], sizeof(LutEntry));
memcpy_P(&last, &lut[size-1], sizeof(LutEntry));

if (v >= first.voltage) return first.percent;
if (v <= last.voltage) return last.percent;

for (uint8_t i = 0; i < size - 1; i++) {
LutEntry hi, lo;
memcpy_P(&hi, &lut[i], sizeof(LutEntry));
memcpy_P(&lo, &lut[i+1], sizeof(LutEntry));

if (v >= lo.voltage) {
float span = hi.voltage - lo.voltage;
if (fabsf(span) < 1e-6f) return hi.percent;
float ratio = (v - lo.voltage) / span;
return lo.percent + ratio * (hi.percent - lo.percent);
}
}
return last.percent;
}

public:
UMBattery()
{
this->setVoltageMultiplier(USERMOD_BATTERY_VOLTAGE_MULTIPLIER);
this->setCalibration(USERMOD_BATTERY_CALIBRATION);
}

virtual ~UMBattery() = default;

virtual void update(batteryConfig cfg)
{
if(cfg.minVoltage) this->setMinVoltage(cfg.minVoltage);
Expand All @@ -42,15 +77,14 @@ class UMBattery

/**
* Corresponding battery curves
* calculates the level in % (0-100) with given voltage and possible voltage range
* calculates the level in % (0-100) with given voltage
*/
virtual float mapVoltage(float v, float min, float max) = 0;
// {
// example implementation, linear mapping
// return (v-min) * 100 / (max-min);
// };
virtual float mapVoltage(float v) = 0;

virtual void calculateAndSetLevel(float voltage) = 0;
void calculateAndSetLevel(float voltage)
{
this->setLevel(this->mapVoltage(voltage));
}



Expand Down Expand Up @@ -104,20 +138,19 @@ class UMBattery
*/
void setVoltage(float voltage)
{
// this->voltage = ( (voltage < this->getMinVoltage() * 0.85f) || (voltage > this->getMaxVoltage() * 1.1f) )
// ? -1.0f
// : voltage;
this->voltage = voltage;
this->voltage = ( (voltage < this->getMinVoltage() * 0.85f) || (voltage > this->getMaxVoltage() * 1.1f) )
? -1.0f
: voltage;
Comment on lines +141 to +143

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Propagate rejected readings to level too.

Line 141 only invalidates voltage. Battery.cpp still calls calculateAndSetLevel(filteredVoltage) immediately afterward and gates auto-off, Coulomb init/recalibration, MQTT, and UI on bat->getLevel() >= 0, so an out-of-range spike can still become 0 or 100 and trigger downstream behavior. Make invalid readings force level = -1 as part of the same contract, or have calculateAndSetLevel() bail out when the stored voltage is invalid.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@usermods/Battery/UMBattery.h` around lines 141 - 143, The rejected battery
reading currently only invalidates voltage in UMBattery::setVoltage, but
Battery.cpp still runs calculateAndSetLevel(filteredVoltage) and uses getLevel()
to gate auto-off, Coulomb init/recalibration, MQTT, and UI updates, so the spike
can still propagate as a valid level. Update the battery model contract so
invalid readings also force level to -1 when voltage is rejected, or make
UMBattery::calculateAndSetLevel bail out immediately if the stored voltage is
invalid; use the setVoltage and calculateAndSetLevel methods to keep both values
in sync.

}

float getLevel()
int8_t getLevel()
{
return this->level;
}

void setLevel(float level)
{
this->level = constrain(level, 0.0f, 110.0f);
this->level = (int8_t)constrain(level, 0.0f, 110.0f);
}
Comment on lines +146 to 154

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP 'String\s*\([^)]*getLevel\s*\(\)\s*,\s*0\s*\)' --type=cpp --type=h

Repository: wled/WLED

Length of output: 151


🏁 Script executed:

rg -n "getLevel\s*\(\)" --type=cpp --type=h usermods/Battery/

Repository: wled/WLED

Length of output: 975


🏁 Script executed:

sed -n '395,405p' usermods/Battery/Battery.cpp

Repository: wled/WLED

Length of output: 530


🏁 Script executed:

rg -n "umLevel\s*=" usermods/Battery/Battery.cpp -B2 -A2

Repository: wled/WLED

Length of output: 357


🏁 Script executed:

rg -n "infoPercentage" usermods/Battery/Battery.cpp -B2 -A2 | head -30

Repository: wled/WLED

Length of output: 713


Fix the String(bat->getLevel(), 0) formatting call in Battery.cpp.

The return-type change of getLevel() from float to int8_t breaks the MQTT battery publication at line 400. The String(value, 0) constructor now resolves to the String(int, base) overload with an invalid base 0, instead of the float decimal-places overload.

Update usermods/Battery/Battery.cpp:400 from:

publishMqtt("battery", String(bat->getLevel(), 0).c_str());

to:

publishMqtt("battery", String(bat->getLevel()).c_str());

All other getLevel() call sites in the module are compatible with the int8_t return type.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@usermods/Battery/UMBattery.h` around lines 147 - 155, The getLevel() accessor
was changed from float to int8_t causing the String(bat->getLevel(), 0) call in
Battery.cpp (the publishMqtt("battery", ...) site) to resolve to the int
overload with an invalid base; update that call to use the single-argument
String constructor so it treats the value as a number string (replace
String(bat->getLevel(), 0) with String(bat->getLevel()) at the publishMqtt
call). Ensure you only change the publish call and leave getLevel()/setLevel()
signatures as-is.


/*
Expand Down
44 changes: 27 additions & 17 deletions usermods/Battery/battery_defaults.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,12 @@


/* Default Battery Type
* 0 = unkown
* 1 = Lipo
* 2 = Lion
* 3 = LiFePO4
*/
#ifndef USERMOD_BATTERY_DEFAULT_TYPE
#define USERMOD_BATTERY_DEFAULT_TYPE 0
#endif
/*
*
* Unkown 'Battery' defaults
*
*/
#ifndef USERMOD_BATTERY_UNKOWN_MIN_VOLTAGE
// Extra save defaults
#define USERMOD_BATTERY_UNKOWN_MIN_VOLTAGE 3.3f
#endif
#ifndef USERMOD_BATTERY_UNKOWN_MAX_VOLTAGE
#define USERMOD_BATTERY_UNKOWN_MAX_VOLTAGE 4.2f
#define USERMOD_BATTERY_DEFAULT_TYPE 1
#endif

/*
Expand Down Expand Up @@ -73,6 +61,18 @@
#define USERMOD_BATTERY_LION_MAX_VOLTAGE 4.2f
#endif

/*
*
* Lithium Iron Phosphate (LiFePO4) defaults
*
*/
#ifndef USERMOD_BATTERY_LIFEPO4_MIN_VOLTAGE
#define USERMOD_BATTERY_LIFEPO4_MIN_VOLTAGE 2.8f
#endif
#ifndef USERMOD_BATTERY_LIFEPO4_MAX_VOLTAGE
#define USERMOD_BATTERY_LIFEPO4_MAX_VOLTAGE 3.6f
#endif

// the default ratio for the voltage divider
#ifndef USERMOD_BATTERY_VOLTAGE_MULTIPLIER
#ifdef ARDUINO_ARCH_ESP32
Expand Down Expand Up @@ -117,12 +117,22 @@
#define USERMOD_BATTERY_LOW_POWER_INDICATOR_DURATION 5
#endif

// battery capacity in mAh (used for runtime estimation with INA226 current sensor)
#ifndef USERMOD_BATTERY_CAPACITY
#define USERMOD_BATTERY_CAPACITY 3000
#endif

// Enable remote battery config updates via JSON API and MQTT
// Uncomment below or define in my_config.h / build flags to allow runtime config changes
// #define USERMOD_BATTERY_ALLOW_REMOTE_UPDATE

// battery types
typedef enum
// fixed uint8_t base so the enum is exactly one byte (exposed via getUMData() as UMT_BYTE)
typedef enum : uint8_t
{
unknown=0,
lipo=1,
lion=2
lion=2,
lifepo4=3
} batteryType;

// used for initial configuration after boot
Expand Down
Loading
Loading