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
13 changes: 13 additions & 0 deletions include/Configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@ struct POWERMETER_UDP_VICTRON_CONFIG_T {
};
using PowerMeterUdpVictronConfig = struct POWERMETER_UDP_VICTRON_CONFIG_T;

struct POWERMETER_AVERAGING_CONFIG_T {
bool Enabled;

enum Mode { Samples = 0, Time = 1 };
Mode WindowMode;

uint16_t WindowSize;
};
using PowerMeterAveragingConfig = struct POWERMETER_AVERAGING_CONFIG_T;

struct POWERLIMITER_INVERTER_CONFIG_T {
uint64_t Serial;
bool IsGoverned;
Expand Down Expand Up @@ -430,6 +440,7 @@ struct CONFIG_T {
struct PowerMeterConfig {
bool Enabled;
uint32_t Source;
PowerMeterAveragingConfig Averaging;
PowerMeterMqttConfig Mqtt;
PowerMeterSerialSdmConfig SerialSdm;
PowerMeterHttpJsonConfig HttpJson;
Expand Down Expand Up @@ -490,6 +501,7 @@ class ConfigurationClass {
static void serializePowerMeterHttpJsonConfig(PowerMeterHttpJsonConfig const& source, JsonObject& target, bool includeCredentials);
static void serializePowerMeterHttpSmlConfig(PowerMeterHttpSmlConfig const& source, JsonObject& target, bool includeCredentials);
static void serializePowerMeterUdpVictronConfig(PowerMeterUdpVictronConfig const& source, JsonObject& target);
static void serializePowerMeterAveragingConfig(PowerMeterAveragingConfig const& source, JsonObject& target);
static void serializeBatteryConfig(BatteryConfig const& source, JsonObject& target);
static void serializeBatteryZendureConfig(BatteryZendureConfig const& source, JsonObject& target);
static void serializeBatteryMqttConfig(BatteryMqttConfig const& source, JsonObject& target);
Expand All @@ -508,6 +520,7 @@ class ConfigurationClass {
static void deserializePowerMeterHttpJsonConfig(JsonObject const& source, PowerMeterHttpJsonConfig& target);
static void deserializePowerMeterHttpSmlConfig(JsonObject const& source, PowerMeterHttpSmlConfig& target);
static void deserializePowerMeterUdpVictronConfig(JsonObject const& source, PowerMeterUdpVictronConfig& target);
static void deserializePowerMeterAveragingConfig(JsonObject const& source, PowerMeterAveragingConfig& target);
static void deserializeBatteryConfig(JsonObject const& source, BatteryConfig& target);
static void deserializeBatteryZendureConfig(JsonObject const& source, BatteryZendureConfig& target);
static void deserializeBatteryMqttConfig(JsonObject const& source, BatteryMqttConfig& target);
Expand Down
2 changes: 2 additions & 0 deletions include/defaults.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@
#define POWERMETER_POLLING_INTERVAL 10
#define POWERMETER_SOURCE 0
#define POWERMETER_SDMADDRESS 1
#define POWERMETER_AVERAGING_ENABLED false
#define POWERMETER_AVERAGING_WINDOW 10

#define HTTP_REQUEST_TIMEOUT_MS 1000

Expand Down
50 changes: 50 additions & 0 deletions include/powermeter/Controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,27 +3,77 @@

#include <powermeter/Provider.h>
#include <TaskSchedulerDeclarations.h>
#include <array>
#include <cstdint>
#include <deque>
#include <memory>
#include <mutex>
#include <optional>

namespace PowerMeters {

class Controller {
public:
struct ChannelStats {
std::optional<float> Raw;
std::optional<float> Average;
std::optional<float> Last;
std::optional<float> Minimum;
std::optional<float> Maximum;
};

struct PowerStats {
ChannelStats Total;
ChannelStats L1;
ChannelStats L2;
ChannelStats L3;
};

void init(Scheduler& scheduler);

void updateSettings();

float getPowerTotal() const;
PowerStats getPowerStats() const;
uint32_t getLastUpdate() const;
bool isDataValid() const;

private:
enum class Channel : uint8_t { Total = 0, L1 = 1, L2 = 2, L3 = 3 };

struct Sample {
uint32_t Timestamp;
float Value;
};

struct ChannelWindow {
std::deque<Sample> Samples;
float Sum = 0.0f;
};

struct ChannelState {
ChannelWindow Window;
ChannelStats Stats;
};

void resetPowerStats();
void updatePowerStatsFromProvider();
void addSample(Channel channel, float value, uint32_t timestamp);
void pruneWindow(ChannelWindow& window, uint32_t now) const;
void updateStats(ChannelState& state);
void refreshAgingForTimeBasedWindows();
ChannelState& channelState(Channel channel);
ChannelState const& channelState(Channel channel) const;
void sanitizeAveragingConfig(PowerMeterAveragingConfig& cfg) const;

void loop();

Task _loopTask;
mutable std::mutex _mutex;
std::unique_ptr<Provider> _upProvider = nullptr;
uint32_t _lastProviderUpdate = 0;
PowerMeterAveragingConfig _averagingCfg = {};
std::array<ChannelState, 4> _powerStats = {};
};

} // namespace PowerMeters
Expand Down
8 changes: 8 additions & 0 deletions include/powermeter/Provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ class Provider {
public:
virtual ~Provider() { }

struct PowerChannels {
std::optional<float> Total;
std::optional<float> L1;
std::optional<float> L2;
std::optional<float> L3;
};

enum class Type : unsigned {
MQTT = 0,
SDM1PH = 1,
Expand All @@ -29,6 +36,7 @@ class Provider {
virtual bool isDataValid() const;

float getPowerTotal() const;
PowerChannels getPowerChannels() const;
uint32_t getLastUpdate() const { return _dataCurrent.getLastUpdate(); }
void mqttLoop() const;

Expand Down
20 changes: 20 additions & 0 deletions src/Configuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <LittleFS.h>
#include <esp_log.h>
#include <nvs_flash.h>
#include <algorithm>

#undef TAG
static const char* TAG = "configuration";
Expand Down Expand Up @@ -130,6 +131,13 @@ void ConfigurationClass::serializePowerMeterUdpVictronConfig(PowerMeterUdpVictro
target["ip_address"] = IPAddress(source.IpAddress).toString();
}

void ConfigurationClass::serializePowerMeterAveragingConfig(PowerMeterAveragingConfig const& source, JsonObject& target)
{
target["enabled"] = source.Enabled;
target["mode"] = source.WindowMode;
target["window"] = source.WindowSize;
}

void ConfigurationClass::serializeBatteryConfig(BatteryConfig const& source, JsonObject& target)
{
target["enabled"] = config.Battery.Enabled;
Expand Down Expand Up @@ -417,6 +425,9 @@ bool ConfigurationClass::write()
powermeter["enabled"] = config.PowerMeter.Enabled;
powermeter["source"] = config.PowerMeter.Source;

JsonObject powermeter_averaging = powermeter["averaging"].to<JsonObject>();
serializePowerMeterAveragingConfig(config.PowerMeter.Averaging, powermeter_averaging);

JsonObject powermeter_mqtt = powermeter["mqtt"].to<JsonObject>();
serializePowerMeterMqttConfig(config.PowerMeter.Mqtt, powermeter_mqtt);

Expand Down Expand Up @@ -563,6 +574,14 @@ void ConfigurationClass::deserializePowerMeterUdpVictronConfig(JsonObject const&
target.IpAddress[3] = ip[3];
}

void ConfigurationClass::deserializePowerMeterAveragingConfig(JsonObject const& source, PowerMeterAveragingConfig& target)
{
target.Enabled = source["enabled"] | POWERMETER_AVERAGING_ENABLED;
target.WindowMode = source["mode"] | PowerMeterAveragingConfig::Mode::Samples;
target.WindowSize = source["window"] | POWERMETER_AVERAGING_WINDOW;
target.WindowSize = std::max<uint16_t>(1, target.WindowSize);
}
Comment on lines +577 to +583
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 6, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Sanitize invalid averaging values when loading config.json.

WindowMode is blindly cast here, and only the lower bound of WindowSize is enforced. A restored or hand-edited config can still load mode=99 or window=500, which then gets echoed back through /api/powermeter/config and leaves the admin form in an invalid state. Clamp the enum and the mode-specific upper bound during deserialization as well.

♻️ Proposed fix
 void ConfigurationClass::deserializePowerMeterAveragingConfig(JsonObject const& source, PowerMeterAveragingConfig& target)
 {
     target.Enabled = source["enabled"] | POWERMETER_AVERAGING_ENABLED;
-    target.WindowMode = static_cast<PowerMeterAveragingConfig::Mode>(source["mode"] | POWERMETER_AVERAGING_MODE);
-    target.WindowSize = source["window"] | POWERMETER_AVERAGING_WINDOW;
-    target.WindowSize = std::max<uint16_t>(1, target.WindowSize);
+    auto mode = source["mode"] | POWERMETER_AVERAGING_MODE;
+    if (mode != PowerMeterAveragingConfig::Mode::Samples
+            && mode != PowerMeterAveragingConfig::Mode::Time) {
+        mode = PowerMeterAveragingConfig::Mode::Samples;
+    }
+    target.WindowMode = static_cast<PowerMeterAveragingConfig::Mode>(mode);
+
+    target.WindowSize = std::max<uint16_t>(1, source["window"] | POWERMETER_AVERAGING_WINDOW);
+    uint16_t maxWindow = (target.WindowMode == PowerMeterAveragingConfig::Mode::Samples) ? 200 : 120;
+    target.WindowSize = std::min<uint16_t>(target.WindowSize, maxWindow);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Configuration.cpp` around lines 572 - 578,
deserializePowerMeterAveragingConfig currently casts source["mode"] directly and
only enforces a lower bound on WindowSize; update it to sanitize both fields:
read the raw mode value, clamp it into the valid PowerMeterAveragingConfig::Mode
range (e.g., between the first enum value and the last valid enum value) before
assigning target.WindowMode, and clamp target.WindowSize to a mode-specific
valid range (enforce an upper bound as well as the existing lower bound, e.g.,
using POWERMETER_AVERAGING_WINDOW_MAX or equivalent constants for each mode).
Make these changes inside
ConfigurationClass::deserializePowerMeterAveragingConfig, touching
target.WindowMode and target.WindowSize assignments so restored/hand-edited
configs cannot set invalid mode numbers or out-of-range window sizes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

does this need a fix? as its the same for all other inputs, too. one can always modify and destroy its own config.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@spcqike, you're right — this is consistent with the project-wide convention in src/Configuration.cpp. Based on a previously recorded learning from this codebase, all deserialization functions here intentionally trust the client payload without backend range validation, and that pattern is applied uniformly across the file (e.g., deserializeBatteryConfig, deserializePowerLimiterConfig, deserializeGridChargerConfig).

Additionally, sanitizeAveragingConfig in Controller.cpp already clamps the mode and window size at runtime, so invalid values in config.json are corrected before they affect behavior anyway. The suggestion to add extra validation in the deserialization layer is unnecessary here. I'll withdraw this comment.


🧠 Learnings used
Learnt from: SW-Niko
Repo: hoylabs/OpenDTU-OnBattery PR: 2465
File: src/Configuration.cpp:577-578
Timestamp: 2026-04-11T08:02:49.268Z
Learning: In `src/Configuration.cpp`, all deserialization functions (e.g., `deserializeBatteryConfig`, `deserializePowerLimiterConfig`, `deserializeGridChargerConfig`) follow a project-wide convention of trusting the client payload without backend range validation. Do not flag missing backend range/sanity validation for individual fields in these deserialization functions, as the pattern is intentional and consistent across the entire file.


void ConfigurationClass::deserializeBatteryConfig(JsonObject const& source, BatteryConfig& target)
{
target.Enabled = source["enabled"] | BATTERY_ENABLED;
Expand Down Expand Up @@ -894,6 +913,7 @@ bool ConfigurationClass::read()
JsonObject powermeter = doc["powermeter"];
config.PowerMeter.Enabled = powermeter["enabled"] | POWERMETER_ENABLED;
config.PowerMeter.Source = powermeter["source"] | POWERMETER_SOURCE;
deserializePowerMeterAveragingConfig(powermeter["averaging"], config.PowerMeter.Averaging);

deserializePowerMeterMqttConfig(powermeter["mqtt"], config.PowerMeter.Mqtt);
deserializePowerMeterSerialSdmConfig(powermeter["serial_sdm"], config.PowerMeter.SerialSdm);
Expand Down
46 changes: 46 additions & 0 deletions src/WebApi_powermeter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ void WebApiPowerMeterClass::generateStatus(AsyncWebServerRequest* request, bool

root["enabled"] = config.PowerMeter.Enabled;
root["source"] = config.PowerMeter.Source;
auto averaging = root["averaging"].to<JsonObject>();
Configuration.serializePowerMeterAveragingConfig(config.PowerMeter.Averaging, averaging);

auto mqtt = root["mqtt"].to<JsonObject>();
Configuration.serializePowerMeterMqttConfig(config.PowerMeter.Mqtt, mqtt);
Expand Down Expand Up @@ -123,6 +125,46 @@ void WebApiPowerMeterClass::onAdminPost(AsyncWebServerRequest* request)
return true;
};

if (root["averaging"].is<JsonObject>()) {
JsonObject averaging = root["averaging"];
if (!(averaging["enabled"].is<bool>() && averaging["mode"].is<uint8_t>() && averaging["window"].is<uint16_t>())) {
retMsg["message"] = "Averaging settings are invalid!";
response->setLength();
request->send(response);
return;
}

uint8_t mode = averaging["mode"].as<uint8_t>();
uint16_t window = averaging["window"].as<uint16_t>();
if (mode > PowerMeterAveragingConfig::Mode::Time) {
retMsg["message"] = "Averaging mode is invalid!";
response->setLength();
request->send(response);
return;
}

if (window < 1) {
retMsg["message"] = "Averaging window must be greater than 0!";
response->setLength();
request->send(response);
return;
}

if (mode == PowerMeterAveragingConfig::Mode::Samples && window > 200) {
retMsg["message"] = "Sample-based averaging supports max 200 samples.";
response->setLength();
request->send(response);
return;
}

if (mode == PowerMeterAveragingConfig::Mode::Time && window > 120) {
retMsg["message"] = "Time-based averaging supports max 120 seconds.";
response->setLength();
request->send(response);
return;
}
}

if (static_cast<::PowerMeters::Provider::Type>(root["source"].as<uint8_t>()) == ::PowerMeters::Provider::Type::HTTP_JSON) {
JsonObject httpJson = root["http_json"];
JsonArray valueConfigs = httpJson["values"];
Expand Down Expand Up @@ -180,6 +222,10 @@ void WebApiPowerMeterClass::onAdminPost(AsyncWebServerRequest* request)
auto& config = guard.getConfig();
config.PowerMeter.Enabled = root["enabled"].as<bool>();
config.PowerMeter.Source = root["source"].as<uint8_t>();
if (root["averaging"].is<JsonObject>()) {
Configuration.deserializePowerMeterAveragingConfig(root["averaging"].as<JsonObject>(),
config.PowerMeter.Averaging);
}

Configuration.deserializePowerMeterMqttConfig(root["mqtt"].as<JsonObject>(),
config.PowerMeter.Mqtt);
Expand Down
18 changes: 18 additions & 0 deletions src/WebApi_ws_live.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,25 @@ void WebApiWsLiveClass::generateOnBatteryJsonResponse(JsonVariant& root, bool al
powerMeterObj["enabled"] = config.PowerMeter.Enabled;

if (config.PowerMeter.Enabled) {
auto addOptionalField = [&](char const* name, std::optional<float> const& value) {
if (!value) { return; }
addTotalField(powerMeterObj, name, *value, "W", 1);
};

auto stats = PowerMeter.getPowerStats();
addTotalField(powerMeterObj, "Power", PowerMeter.getPowerTotal(), "W", 1);
addOptionalField("PowerRaw", stats.Total.Raw);
addOptionalField("PowerAvg", stats.Total.Average);
addOptionalField("PowerLast", stats.Total.Last);
addOptionalField("PowerMin", stats.Total.Minimum);
addOptionalField("PowerMax", stats.Total.Maximum);

addOptionalField("PowerL1", stats.L1.Raw);
addOptionalField("PowerL1Avg", stats.L1.Average);
addOptionalField("PowerL2", stats.L2.Raw);
addOptionalField("PowerL2Avg", stats.L2.Average);
addOptionalField("PowerL3", stats.L3.Raw);
addOptionalField("PowerL3Avg", stats.L3.Average);
}

if (!all) { _lastPublishPowerMeter = millis(); }
Expand Down
Loading