[Feature] add averaging for powermeters#2454
[Feature] add averaging for powermeters#2454spcqike wants to merge 5 commits intohoylabs:developmentfrom
Conversation
Build ArtifactsFirmware built from this pull request's code:
Notice
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds power-meter averaging: new averaging config, serialization/deserialization, provider multi-channel readings, controller sliding-window stats (sample/time modes), REST/admin validation, WebSocket live data fields, and UI/supporting types and translations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebAPI as Web API (ws_live / admin)
participant Controller as Power Meter Controller
participant Provider as Power Meter Provider
loop Provider update
Provider->>Provider: read raw channels (Total, L1, L2, L3)
Provider->>Controller: updatePowerStatsFromProvider(channels)
end
activate Controller
Controller->>Controller: addSample / refresh time-based aging
Controller->>Controller: pruneWindow (samples or time)
Controller->>Controller: recompute ChannelStats (Raw, Avg, Last, Min, Max)
deactivate Controller
Client->>WebAPI: request live/update or admin change
activate WebAPI
WebAPI->>Controller: getPowerStats() or getPowerTotal()
Controller-->>WebAPI: return stats or total (averaging-aware)
WebAPI->>Client: send JSON (conditional fields for optional stats)
deactivate WebAPI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
webapp/src/types/PowerMeterConfig.ts (1)
43-47: Narrowmodeto a typed enum/literal union.Using
numberhere allows invalid states at compile time. Prefer a constrained type (0 | 1or enum) for stronger safety.♻️ Proposed fix
+export type PowerMeterAveragingMode = 0 | 1; + export interface PowerMeterAveragingConfig { enabled: boolean; - mode: number; + mode: PowerMeterAveragingMode; window: number; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webapp/src/types/PowerMeterConfig.ts` around lines 43 - 47, The PowerMeterAveragingConfig currently types mode as number which permits invalid values; change mode to a constrained type (e.g., a literal union `0 | 1` or a dedicated enum like `PowerMeterAveragingMode`) and update any code that constructs or reads PowerMeterAveragingConfig to use the new enum/union values (adjust switch/case comparisons and initializers for PowerMeterAveragingConfig, and any imports/exports referencing PowerMeterAveragingConfig) so the compiler enforces only valid modes.include/powermeter/Provider.h (1)
14-19: Add a direct<optional>include forstd::optionalusage.
PowerChannelsusesstd::optional, but this header does not include<optional>directly. Please avoid relying on transitive includes.♻️ Proposed fix
`#include` <atomic> +#include <optional> `#include` <Configuration.h> `#include` <powermeter/DataPoints.h>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/powermeter/Provider.h` around lines 14 - 19, The header declaring struct PowerChannels uses std::optional but does not include <optional>; add a direct `#include` <optional> at the top of the header so PowerChannels::Total, L1, L2, L3 compile without relying on transitive includes, keeping the include next to the other standard includes and before the PowerChannels definition.src/WebApi_ws_live.cpp (1)
178-183: Use explicitRawsuffixes for the per-phase raw stats.
PowerL1,PowerL2, andPowerL3are populated fromstats.L*.Raw, while the total raw field is exposed asPowerRaw. That asymmetric naming makes the live payload harder to reason about and leaves no obvious slot for a future effective per-phase value analogous toPower. I’d rename these toPowerL1Raw/PowerL2Raw/PowerL3Rawbefore clients start depending on the new keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WebApi_ws_live.cpp` around lines 178 - 183, Rename the per-phase raw power fields to include the Raw suffix so they match the total field naming: change the keys passed to addOptionalField from "PowerL1"/"PowerL2"/"PowerL3" to "PowerL1Raw"/"PowerL2Raw"/"PowerL3Raw" where the values come from stats.L1.Raw, stats.L2.Raw, stats.L3.Raw (keep the corresponding Average fields as-is like "PowerL1Avg" etc.); update any callers or consumers that expect the old keys accordingly to avoid breaking clients.src/powermeter/Controller.cpp (5)
18-23: UnusedSUBTAGvariable.
SUBTAGis defined but never used in this file. The only log statement at line 161 usesDTU_LOGDwhich likely usesTAGimplicitly.♻️ Suggested fix
`#undef` TAG static const char* TAG = "powerMeter"; -static const char* SUBTAG = "Controller";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powermeter/Controller.cpp` around lines 18 - 23, The SUBTAG variable is declared but never used (unused symbol SUBTAG) which should be removed or used consistently; either delete the static const char* SUBTAG = "Controller" declaration to eliminate the unused variable, or update logging calls (e.g., where DTU_LOGD is used) to include SUBTAG instead of relying solely on TAG so both TAG and SUBTAG are purposefully referenced; ensure any change keeps log semantics consistent and compiles without unused-variable warnings.
109-120: Minor redundancy inLastassignment.Line 113 sets
Last = value, thenupdateStats()at line 119 setsLast = window.back().Value. Since the sample was just pushed, these are the same value. The assignment at line 113 is redundant but harmless.♻️ Suggested cleanup
void Controller::addSample(Channel channel, float value, uint32_t timestamp) { auto& state = channelState(channel); state.Stats.Raw = value; - state.Stats.Last = value; state.Window.Samples.push_back({ timestamp, value }); state.Window.Sum += value;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powermeter/Controller.cpp` around lines 109 - 120, Remove the redundant direct assignment to state.Stats.Last in Controller::addSample: since you push the sample into state.Window and then call updateStats(state) (which sets Last from the window back element), drop the line "state.Stats.Last = value" so only state.Stats.Raw = value remains before pushing the sample, keeping pruneWindow(state.Window, millis()) and updateStats(state) as-is.
213-233:const_castworkaround for mutable state update.Using
const_castto callrefreshAgingForTimeBasedWindows()from aconstmethod is a code smell. The function modifies internal state (pruning windows, updating stats), so this method isn't trulyconstin behavior.Consider either:
- Making
getPowerTotal()non-const (may require changes to callers)- Making the window state
mutableif the pruning is considered a logical cache refresh- Accepting this as a pragmatic workaround given existing API constraints
The current implementation works correctly under the mutex, but documents intent poorly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powermeter/Controller.cpp` around lines 213 - 233, The method Controller::getPowerTotal() is marked const but uses const_cast to call refreshAgingForTimeBasedWindows(), which mutates internal state and is a code smell; fix by making the mutability explicit: either remove const from getPowerTotal() so it can call refreshAgingForTimeBasedWindows() directly, or mark the members modified by refreshAgingForTimeBasedWindows() (e.g., window state used by channelState(Channel::Total).Stats or any cached stats/aging containers) as mutable so a true logical-const operation is expressed; update the function signature and any callers if you choose non-const, or mark the specific member fields mutable if treating pruning as a cache refresh, and keep the std::lock_guard<std::mutex> l(_mutex) to protect concurrent access.
70-85: Potential floating-point drift in running sum.The
Sumis maintained by incrementally adding and subtracting sample values. Over many samples, this can accumulate floating-point rounding errors. For critical power calculations, this could lead to slight inaccuracies in the average.Consider periodically recomputing the sum from scratch (e.g., every N samples or when the window is small) to reset accumulated drift, or accept the minor drift as acceptable for this use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powermeter/Controller.cpp` around lines 70 - 85, Controller::pruneWindow currently maintains window.Sum by incrementally subtracting removed sample values which can accumulate floating-point drift; modify pruneWindow (for both Samples mode and Time mode) to periodically recompute window.Sum from scratch from ChannelWindow::Samples instead of relying solely on incremental updates—e.g., track how many removals occurred in the pruning loop (or check if Samples.size() is below a small threshold) and if it exceeds a chosen N (or threshold) recalculate window.Sum = sum(sample.Value for sample in window.Samples) to reset accumulated rounding error; ensure this logic references window.Sum, window.Samples, Controller::pruneWindow, _averagingCfg.WindowMode and _averagingCfg.WindowSize so it runs after the existing pop_front loops.
235-249: Sameconst_castpattern asgetPowerTotal.This method has the same
const_castworkaround. If refactoringgetPowerTotal, apply the same solution here for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/powermeter/Controller.cpp` around lines 235 - 249, getPowerStats uses const_cast to call refreshAgingForTimeBasedWindows from a const method—duplicate of the getPowerTotal issue. Remove the const_cast by making refreshAgingForTimeBasedWindows callable from const: either change its signature to a const method (and update any members it modifies to be mutable) or add a const overload that performs the necessary refresh via mutable state; then remove the const_cast in Controller::getPowerStats (and apply the same change to Controller::getPowerTotal). Ensure channelState and any members modified during refresh are adjusted to permit mutation from a const context (e.g. mark them mutable) so locking with _mutex remains correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Configuration.cpp`:
- Around line 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.
In `@webapp/src/locales/fr.json`:
- Around line 657-660: Fix French typos in the averaging labels: update
AveragingModeSamples from "Base sur les mesures" to "Basé sur les mesures",
AveragingModeTime from "Base sur le temps" to "Basé sur le temps", and
AveragingWindow from "Fenetre de moyennage" to "Fenêtre de moyennage"; the
"samples" key ("mesures") is correct and can remain unchanged.
In `@webapp/src/views/PowerMeterAdminView.vue`:
- Around line 47-55: When the averaging mode changes you must clamp
powerMeterConfigList.averaging.window to the current allowed range because the
<InputElement> max (averagingWindowMax) only constrains input UI and doesn't
update the model; add logic that, on changes to
powerMeterConfigList.averaging.mode (or in the method that switches mode), sets
powerMeterConfigList.averaging.window = Math.max(1,
Math.min(powerMeterConfigList.averaging.window, averagingWindowMax)); apply the
same clamp in the other place referenced (lines ~414-420) so the model is always
within server-accepted bounds before POSTing. Ensure you reference
averagingWindowMax and powerMeterConfigList.averaging.window when implementing
the clamp.
---
Nitpick comments:
In `@include/powermeter/Provider.h`:
- Around line 14-19: The header declaring struct PowerChannels uses
std::optional but does not include <optional>; add a direct `#include` <optional>
at the top of the header so PowerChannels::Total, L1, L2, L3 compile without
relying on transitive includes, keeping the include next to the other standard
includes and before the PowerChannels definition.
In `@src/powermeter/Controller.cpp`:
- Around line 18-23: The SUBTAG variable is declared but never used (unused
symbol SUBTAG) which should be removed or used consistently; either delete the
static const char* SUBTAG = "Controller" declaration to eliminate the unused
variable, or update logging calls (e.g., where DTU_LOGD is used) to include
SUBTAG instead of relying solely on TAG so both TAG and SUBTAG are purposefully
referenced; ensure any change keeps log semantics consistent and compiles
without unused-variable warnings.
- Around line 109-120: Remove the redundant direct assignment to
state.Stats.Last in Controller::addSample: since you push the sample into
state.Window and then call updateStats(state) (which sets Last from the window
back element), drop the line "state.Stats.Last = value" so only state.Stats.Raw
= value remains before pushing the sample, keeping pruneWindow(state.Window,
millis()) and updateStats(state) as-is.
- Around line 213-233: The method Controller::getPowerTotal() is marked const
but uses const_cast to call refreshAgingForTimeBasedWindows(), which mutates
internal state and is a code smell; fix by making the mutability explicit:
either remove const from getPowerTotal() so it can call
refreshAgingForTimeBasedWindows() directly, or mark the members modified by
refreshAgingForTimeBasedWindows() (e.g., window state used by
channelState(Channel::Total).Stats or any cached stats/aging containers) as
mutable so a true logical-const operation is expressed; update the function
signature and any callers if you choose non-const, or mark the specific member
fields mutable if treating pruning as a cache refresh, and keep the
std::lock_guard<std::mutex> l(_mutex) to protect concurrent access.
- Around line 70-85: Controller::pruneWindow currently maintains window.Sum by
incrementally subtracting removed sample values which can accumulate
floating-point drift; modify pruneWindow (for both Samples mode and Time mode)
to periodically recompute window.Sum from scratch from ChannelWindow::Samples
instead of relying solely on incremental updates—e.g., track how many removals
occurred in the pruning loop (or check if Samples.size() is below a small
threshold) and if it exceeds a chosen N (or threshold) recalculate window.Sum =
sum(sample.Value for sample in window.Samples) to reset accumulated rounding
error; ensure this logic references window.Sum, window.Samples,
Controller::pruneWindow, _averagingCfg.WindowMode and _averagingCfg.WindowSize
so it runs after the existing pop_front loops.
- Around line 235-249: getPowerStats uses const_cast to call
refreshAgingForTimeBasedWindows from a const method—duplicate of the
getPowerTotal issue. Remove the const_cast by making
refreshAgingForTimeBasedWindows callable from const: either change its signature
to a const method (and update any members it modifies to be mutable) or add a
const overload that performs the necessary refresh via mutable state; then
remove the const_cast in Controller::getPowerStats (and apply the same change to
Controller::getPowerTotal). Ensure channelState and any members modified during
refresh are adjusted to permit mutation from a const context (e.g. mark them
mutable) so locking with _mutex remains correct.
In `@src/WebApi_ws_live.cpp`:
- Around line 178-183: Rename the per-phase raw power fields to include the Raw
suffix so they match the total field naming: change the keys passed to
addOptionalField from "PowerL1"/"PowerL2"/"PowerL3" to
"PowerL1Raw"/"PowerL2Raw"/"PowerL3Raw" where the values come from stats.L1.Raw,
stats.L2.Raw, stats.L3.Raw (keep the corresponding Average fields as-is like
"PowerL1Avg" etc.); update any callers or consumers that expect the old keys
accordingly to avoid breaking clients.
In `@webapp/src/types/PowerMeterConfig.ts`:
- Around line 43-47: The PowerMeterAveragingConfig currently types mode as
number which permits invalid values; change mode to a constrained type (e.g., a
literal union `0 | 1` or a dedicated enum like `PowerMeterAveragingMode`) and
update any code that constructs or reads PowerMeterAveragingConfig to use the
new enum/union values (adjust switch/case comparisons and initializers for
PowerMeterAveragingConfig, and any imports/exports referencing
PowerMeterAveragingConfig) so the compiler enforces only valid modes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 792d2a12-eb7c-4cfa-a242-4922a9e9c84b
📒 Files selected for processing (18)
include/Configuration.hinclude/defaults.hinclude/powermeter/Controller.hinclude/powermeter/Provider.hsrc/Configuration.cppsrc/WebApi_powermeter.cppsrc/WebApi_ws_live.cppsrc/powermeter/Controller.cppsrc/powermeter/Provider.cppsrc/powermeter/json/mqtt/Provider.cppsrc/powermeter/sml/Provider.cppwebapp/src/components/InputElement.vuewebapp/src/locales/de.jsonwebapp/src/locales/en.jsonwebapp/src/locales/fr.jsonwebapp/src/types/LiveDataStatus.tswebapp/src/types/PowerMeterConfig.tswebapp/src/views/PowerMeterAdminView.vue
| 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); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
does this need a fix? as its the same for all other inputs, too. one can always modify and destroy its own config.
There was a problem hiding this comment.
@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.
|
@AndreasBoehm ich sehe, du hast ein Label gesetzt. Soll ich die 3 Punkte noch nachpflegen? oder hast du andere Änderungen angedacht? (Oder willst du die selber einspielen? das sollte ja gehen, oder? ich habe oben in den Log noch ein Diagramm reingemacht, in dem man den zeitlichen Verlauf der min/max/avg/last Werte für die 3 Sekunden sieht. In dem Auszug springt es zwar nur um 60W, bei anderen Lasten halt auch gern mehr :) |
|
@spcqike Generell übernimmt @coderabbitai das erste Pull-Request Review, also bitte die Kommentare abklären, fixen bzw erklären warum diese keinen fix benötigen, KI kann ja auch mal daneben liegen ;) Erst wenn @coderabbitai grünes Licht gibt werde ich mir, wann es meine Zeit zu lässt, den PR genauer anschauen. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainThe first PR description image shows the "Stromzähler Einstellungen" (Electricity Meter Settings) UI — a dark-themed configuration page with two collapsible sections: one for general meter configuration (including toggles for enabling the meter and averaging, plus dropdowns for averaging mode set to "Zeitbasiert"/Time-based, averaging window set to 1 second, and meter type set to Victron VM-3P75CT Modbus UDP), and one for device-specific settings (query interval set to 0.1 seconds). The second PR description image shows a line chart titled "Diagrammtitel" with four series — min (dark blue), max (orange), avg (dark green), and last (light blue) — plotted over 26 time steps. Values range roughly from ~5748 W to ~5828 W, illustrating how the sliding-window statistics tracker smooths out the noisy "last" readings while min converges downward and max converges downward over time as the window slides. Let me fetch the current open review comments to provide a useful summary. 🏁 Script executed: Length of output: 19534
🔴 Actionable Comments (must fix or explain)
🟡 Nitpick Comments (optional but recommended)
The three actionable items are the most important ones to fix. Feel free to reply here if any of the findings don't apply or if you need clarification on any of them — AI can certainly be wrong! 😊 |
|
@coderabbitai sind die drei Hauptpunkte erledigt? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me check the current state of all three files on the PR branch. 🏁 Script executed: Length of output: 23275
❌ Punkt 1 noch offen:
|
|
@coderabbitai prüfe Punkt 1 erneut unter dem Aspekt deiner Antwort hier |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me fetch the linked review comment and the current state of the relevant files. 🏁 Script executed: Length of output: 6698
In meiner Antwort am verlinkten Kommentar (r3079632034) hatte ich den Befund bereits zurückgezogen, aus folgenden Gründen:
Damit sind alle drei Hauptpunkte meines ursprünglichen Reviews behoben:
Der PR kann nun von |
|
für alle Interessierten, hier noch ein Diagramm über 1 Minute, mit 3s Mittelwertbildung bei 0.1s Abfrageintervall das Victron. 
man sieht gut, dass der Victron durch seine Geschwindigkeit kurze Spitzen (< 0.5s) von +-60W erfasst, die die Hysterese des DPL triggern würden. mit anderen aktiven Verbrauchern, die taktend Strom ziehen (bspw. das Heizbett und die Düse im 3D Drucker, der aber grad nicht läuft), sind die Sprünge halt noch krasser. ein langsameres Abfragen des Victron würde das Regelverhalten zwar auch beruhigen, aber eben potentiell unsauber sein. Man würde nur einen x-beliebigen Punkt auf der "blauen Linie" erfassen, darauf regeln und bis zum nächsten Abfragen so einspeisen. das kann passen und den tatsächlichen Verbrauch abbilden, muss es aber nicht. hier die Rohdaten, für die, die es selber auswerten und nachvollziehen wollen 😄 DetailsAchtung: das Diagramm zeigt die Werte um "+350W" verschoben. sonst wäre die Y-Achse so niedrig und die Werte weit weg von der Beschriftung :)
[09:46:05.585] D (2396297) powerMeter: [Controller] Total stats min=-408.00 avg=-392.61 max=-359.00 last=-408.00 |
|
Nachtrag, weils mich jetzt doch mal genauer interessiert hat, hier ne Darstellung mit aktivem Drucker. (und ohne aktivem DPL) 
Nach dem Hochheizen (Ende ~ 11:10:30) geht er ins Takten und Halten der Temperaturen (Düse & Druckbett) Rohdaten
[11:09:54.358] D (7425022) powerMeter: [Controller] Total stats min=443.00 avg=502.83 max=544.00 last=544.00 beim Takten springt die Leistung einfach mal um +-500W hoch und runter , und das teils im 0.1s Abstand. Chart Leistungsänderung zwischen Einzelwerten (0.1s)
|
2 participants
add an option to create average values based on time or counts.
it adds min, avg, max and last value for each phase and total, if available, which can be optained by
getPowerStats()Log mit Chart
[22:49:10.451] D (513803) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.89, I1=9.09, P1=2066.00, V2=234.29, I2=7.94, P2=1840.00, V3=236.76, I3=7.95, P3=1870.00, Ptotal=5776.00
[22:49:10.451] D (513804) powerMeter: [Controller] Total stats min=5770.00 avg=5800.22 max=5827.00 last=5776.00
[22:49:10.578] D (513917) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.89, I1=9.09, P1=2081.00, V2=234.28, I2=7.94, P2=1840.00, V3=236.75, I3=7.95, P3=1869.00, Ptotal=5790.00
[22:49:10.578] D (513918) powerMeter: [Controller] Total stats min=5770.00 avg=5798.56 max=5827.00 last=5790.00
[22:49:10.578] D (514002) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.88, I1=9.09, P1=2067.00, V2=234.27, I2=7.94, P2=1852.00, V3=236.74, I3=7.95, P3=1871.00, Ptotal=5790.00
[22:49:10.769] D (514003) powerMeter: [Controller] Total stats min=5776.00 avg=5800.78 max=5827.00 last=5790.00
[22:49:10.769] D (514106) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.88, I1=9.09, P1=2072.00, V2=234.26, I2=7.94, P2=1844.00, V3=236.74, I3=7.95, P3=1865.00, Ptotal=5781.00
[22:49:10.769] D (514107) powerMeter: [Controller] Total stats min=5776.00 avg=5798.78 max=5827.00 last=5781.00
[22:49:10.883] D (514207) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.87, I1=9.09, P1=2064.00, V2=234.25, I2=7.93, P2=1833.00, V3=236.73, I3=7.95, P3=1905.00, Ptotal=5802.00
[22:49:10.883] D (514208) powerMeter: [Controller] Total stats min=5776.00 avg=5799.78 max=5827.00 last=5802.00
[22:49:10.883] D (514310) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.86, I1=9.10, P1=2063.00, V2=234.25, I2=7.93, P2=1842.00, V3=236.73, I3=7.95, P3=1887.00, Ptotal=5792.00
[22:49:11.054] D (514311) powerMeter: [Controller] Total stats min=5776.00 avg=5796.78 max=5827.00 last=5792.00
[22:49:11.054] D (514407) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.85, I1=9.10, P1=2067.00, V2=234.24, I2=7.93, P2=1841.00, V3=236.71, I3=7.95, P3=1867.00, Ptotal=5775.00
[22:49:11.054] D (514408) powerMeter: [Controller] Total stats min=5775.00 avg=5794.11 max=5827.00 last=5775.00
[22:49:11.174] D (514509) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.84, I1=9.10, P1=2058.00, V2=234.24, I2=7.93, P2=1843.00, V3=236.70, I3=7.95, P3=1856.00, Ptotal=5757.00
[22:49:11.174] D (514510) powerMeter: [Controller] Total stats min=5757.00 avg=5790.40 max=5827.00 last=5757.00
[22:49:11.174] D (514607) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.83, I1=9.10, P1=2069.00, V2=234.23, I2=7.93, P2=1854.00, V3=236.69, I3=7.95, P3=1864.00, Ptotal=5786.00
[22:49:11.354] D (514608) powerMeter: [Controller] Total stats min=5757.00 avg=5787.60 max=5827.00 last=5786.00
[22:49:11.355] D (514706) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.82, I1=9.10, P1=2065.00, V2=234.22, I2=7.92, P2=1842.00, V3=236.68, I3=7.95, P3=1900.00, Ptotal=5806.00
[22:49:11.355] D (514707) powerMeter: [Controller] Total stats min=5757.00 avg=5785.50 max=5806.00 last=5806.00
[22:49:11.486] D (514809) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.82, I1=9.10, P1=2083.00, V2=234.21, I2=7.92, P2=1845.00, V3=236.68, I3=7.95, P3=1879.00, Ptotal=5807.00
[22:49:11.486] D (514810) powerMeter: [Controller] Total stats min=5757.00 avg=5788.60 max=5807.00 last=5807.00
[22:49:11.486] D (514909) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.80, I1=9.10, P1=2087.00, V2=234.21, I2=7.92, P2=1837.00, V3=236.67, I3=7.95, P3=1874.00, Ptotal=5799.00
[22:49:11.673] D (514910) powerMeter: [Controller] Total stats min=5757.00 avg=5789.55 max=5807.00 last=5799.00
[22:49:11.674] D (515029) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2062.00, V2=234.20, I2=7.92, P2=1836.00, V3=236.66, I3=7.95, P3=1860.00, Ptotal=5758.00
[22:49:11.674] D (515030) powerMeter: [Controller] Total stats min=5757.00 avg=5786.30 max=5807.00 last=5758.00
[22:49:11.800] D (515129) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2079.00, V2=234.20, I2=7.92, P2=1833.00, V3=236.65, I3=7.95, P3=1872.00, Ptotal=5785.00
[22:49:11.800] D (515130) powerMeter: [Controller] Total stats min=5757.00 avg=5786.70 max=5807.00 last=5785.00
[22:49:11.800] D (515227) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2064.00, V2=234.20, I2=7.93, P2=1841.00, V3=236.65, I3=7.95, P3=1863.00, Ptotal=5768.00
[22:49:11.976] D (515228) powerMeter: [Controller] Total stats min=5757.00 avg=5783.30 max=5807.00 last=5768.00
[22:49:11.976] D (515337) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2073.00, V2=234.20, I2=7.93, P2=1855.00, V3=236.65, I3=7.95, P3=1867.00, Ptotal=5795.00
[22:49:11.976] D (515338) powerMeter: [Controller] Total stats min=5757.00 avg=5783.60 max=5807.00 last=5795.00
[22:49:12.097] D (515431) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2065.00, V2=234.21, I2=7.93, P2=1839.00, V3=236.65, I3=7.95, P3=1863.00, Ptotal=5767.00
[22:49:12.097] D (515432) powerMeter: [Controller] Total stats min=5757.00 avg=5782.80 max=5807.00 last=5767.00
[22:49:12.097] D (515529) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2057.00, V2=234.21, I2=7.93, P2=1841.00, V3=236.66, I3=7.95, P3=1872.00, Ptotal=5770.00
[22:49:12.277] D (515530) powerMeter: [Controller] Total stats min=5758.00 avg=5784.10 max=5807.00 last=5770.00
[22:49:12.277] D (515629) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.10, P1=2042.00, V2=234.21, I2=7.93, P2=1847.00, V3=236.66, I3=7.95, P3=1861.00, Ptotal=5750.00
[22:49:12.278] D (515630) powerMeter: [Controller] Total stats min=5750.00 avg=5780.50 max=5807.00 last=5750.00
[22:49:12.401] D (515732) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.78, I1=9.11, P1=2065.00, V2=234.21, I2=7.93, P2=1835.00, V3=236.66, I3=7.95, P3=1868.00, Ptotal=5768.00
[22:49:12.401] D (515733) powerMeter: [Controller] Total stats min=5750.00 avg=5776.70 max=5807.00 last=5768.00
[22:49:12.401] D (515829) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.78, I1=9.10, P1=2071.00, V2=234.21, I2=7.93, P2=1845.00, V3=236.66, I3=7.95, P3=1868.00, Ptotal=5783.00
[22:49:12.579] D (515830) powerMeter: [Controller] Total stats min=5750.00 avg=5774.30 max=5799.00 last=5783.00
[22:49:12.580] D (515931) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.79, I1=9.11, P1=2069.00, V2=234.21, I2=7.94, P2=1837.00, V3=236.64, I3=7.95, P3=1870.00, Ptotal=5776.00
[22:49:12.580] D (515932) powerMeter: [Controller] Total stats min=5750.00 avg=5772.00 max=5795.00 last=5776.00
[22:49:12.692] D (516034) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.80, I1=9.11, P1=2059.00, V2=234.21, I2=7.93, P2=1839.00, V3=236.63, I3=7.95, P3=1870.00, Ptotal=5769.00
[22:49:12.692] D (516034) powerMeter: [Controller] Total stats min=5750.00 avg=5773.10 max=5795.00 last=5769.00
[22:49:12.692] D (516132) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.81, I1=9.11, P1=2077.00, V2=234.21, I2=7.93, P2=1844.00, V3=236.61, I3=7.95, P3=1869.00, Ptotal=5790.00
[22:49:12.888] D (516133) powerMeter: [Controller] Total stats min=5750.00 avg=5773.60 max=5795.00 last=5790.00
[22:49:12.888] D (516236) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.81, I1=9.11, P1=2061.00, V2=234.21, I2=7.93, P2=1842.00, V3=236.59, I3=7.95, P3=1872.00, Ptotal=5775.00
[22:49:12.888] D (516236) powerMeter: [Controller] Total stats min=5750.00 avg=5774.30 max=5795.00 last=5775.00
[22:49:13.014] D (516343) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.81, I1=9.11, P1=2072.00, V2=234.21, I2=7.93, P2=1837.00, V3=236.57, I3=7.95, P3=1875.00, Ptotal=5785.00
[22:49:13.014] D (516344) powerMeter: [Controller] Total stats min=5750.00 avg=5773.30 max=5790.00 last=5785.00
[22:49:13.014] D (516441) powerMeter: [Modbus/UDP/Victron] Import=2849.170, Export=235.300, V1=230.82, I1=9.11, P1=2077.00, V2=234.20, I2=7.94, P2=1845.00, V3=236.55, I3=7.95, P3=1866.00, Ptotal=5788.00
[22:49:13.189] D (516442) powerMeter: [Controller] Total stats min=5750.00 avg=5775.40 max=5790.00 last=5788.00
this is useful for users with very fast powermeters, which may give back a random power sample when queried slowly (like the victron, which i described here)
it also may be usefull, if we are going to implement some kind of "summer/winter mode", which was discussed somewhere (like using lowest value within x seconds as baseline in winter, and highest value in summer, to reduce unnecessary export in winter where solar energy is low, and to not have import / allow more export in summer, where solar is available)