Skip to content

AlphaEss: calculate currents from powers#3173

Open
ndrsnhs wants to merge 7 commits intoopenWB:masterfrom
ndrsnhs:alphaess-counter-currents
Open

AlphaEss: calculate currents from powers#3173
ndrsnhs wants to merge 7 commits intoopenWB:masterfrom
ndrsnhs:alphaess-counter-currents

Conversation

@ndrsnhs
Copy link
Copy Markdown
Contributor

@ndrsnhs ndrsnhs commented Mar 3, 2026

Der Faktor der Phasenströme wurde wegen Problemen im Lastmanagement (zu niedrige Ströme) angepasst. Die Anpassung führt aber bei anderen Anlagen zu Problemen (zu hohe Ströme).
AlphaEss Anlagen haben unterschiedliche Faktoren für die Ströme einzelner Phasen. Je nach Modell unterscheiden sich die Werte um den Faktor 10.
Eine Unterscheidung nach Firmware oder Modell ist aktuell nicht möglich. Workaround ist die Berechnung der Phasenströme aus der übermittelten Leistung einzelner Phasen.

Copy link
Copy Markdown
Contributor

@LKuemmel LKuemmel left a comment

Choose a reason for hiding this comment

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

Bitte in einem Kommentar festhalten, warum die Ströme entfernt wurden.

Comment on lines 50 to 52
currents = [val / 230 for val in powers]
voltages = self.__tcp_client.read_holding_registers(
0x0014, [ModbusDataType.UINT_16]*3, unit=self.__modbus_id)
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.

Wenn die Spannungen bekannt sind, wird es genauer, wenn damit gerechnet wird, statt mit 230V.

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.

Da powers die Wirkleistung ist, können die Ströme so nicht berechnet werden.
Ich vermute, dass der Faktor für die Ströme auch per Modbus ausgelesen werden kann.

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.

In der Doku ist anscheinend doch fix der Faktor 0,1 angegeben. Wenn das wirklich nicht passt, sollten die Ströme anhand der Scheinleistungen berechnet werden.

apparent_powers = self.__tcp_client.read_holding_registers(0x002b, [ModbusDataType.INT_32]*3, unit=self.__modbus_id)

Copy link
Copy Markdown
Contributor Author

@ndrsnhs ndrsnhs Mar 4, 2026

Choose a reason for hiding this comment

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

Der Faktor ist laut Doku fix auf 0,1. Wir haben bisher über Jahre 0,001 genutzt. Es sind aber Anlagen aufgetaucht wo es Probleme beim Lastmanagement gab, da der Faktor dort nicht stimmte.

Das Modul/ der Faktor wurde an die AlphaEss Doku angepasst. Jetzt tauchen aber vermehrt Anlagen auf bei denen das Lastmanagement die Ladung verhindert, da die Ströme zu hoch sind (ebenfalls falscher Faktor).
Ob das jetzt modell- oder firmwarespezifisch ist bleibt unklar.

Das mit der Leistung sehe ich ein, das konkrete Problem dabei ist aber, dass nur die Wirkleistung vorzeichenbehaftet ist. Scheinleistung und Blindleistung sind bei der Referenzanlage immer positiv.

Würdest du dann mit der Scheinleistung arbeiten und das Vorzeichen der Wirkleistung setzen?
Ansonsten wären die Ströme nämlich ebenfalls immer positiv...

Edit: Die zweite Anlage bei der der Faktor 0.001 statt 0.1 ist liefert für die Scheinleistung auf jeder Phase 0. Für die gesamte Scheinleistung ebenfalls 0.

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.

Dann haben wir absolut keine Basis, um mit Strömen arbeiten zu können.
Die Ermittlung von currents sollte dann meiner Meinung nach aus dem Modul entfernt werden.

Copy link
Copy Markdown
Contributor Author

@ndrsnhs ndrsnhs Apr 7, 2026

Choose a reason for hiding this comment

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

Ich habe mir eine alternative Strategie überlegt. Wir errechnen uns aus der Spannung, den Strömen und der Leistung einen Faktor und runden diesen auf den nächsten korrekten Wert.
factor = currents * voltages / powers
Mögliche Werte sind: -1000, -100, -10, 10, 100, 1000
Mit dem ermittelten Faktor pro Phase kann man dann den Strom berechnen. Vorteil zur bisherigen Implementierung ist, dass die Ströme nun ebenfalls vorzeichenbehaftet sind - das war bisher nicht der Fall.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts AlphaESS counter phase-current reporting to avoid model-/firmware-dependent current scaling issues by deriving a per-phase scaling factor from the relationship between reported current, voltage, and per-phase power.

Changes:

  • Reworks AlphaESS counter update logic to rescale phase currents using per-phase power readings.
  • Introduces scale_currents() helper to choose an appropriate scaling factor per phase.
  • Removes PeakFilter usage from the AlphaESS counter component.
Comments suppressed due to low confidence (1)

packages/modules/devices/alpha_ess/alpha_ess/counter.py:36

  • The PeakFilter was removed from the counter component (import, initialization, and check_values(power, imported, exported) call). This also removes plausibility checks for power peaks and sudden imported/exported counter jumps, which can lead to storing implausible values (or losing warnings that other components still emit). If the intention is only to rescale phase currents, keep PeakFilter and continue calling it after reading power/imported/exported.
    def initialize(self) -> None:
        self.__tcp_client: modbus.ModbusTcpClient_ = self.kwargs['tcp_client']
        self.__device_config: AlphaEssConfiguration = self.kwargs['device_config']
        self.__modbus_id: int = self.kwargs['modbus_id']
        self.store = get_counter_value_store(self.component_config.id)
        self.fault_state = FaultState(ComponentInfo.from_component_config(self.component_config))

    def update(self):
        time.sleep(0.1)
        if self.__device_config.source == 0 and self.__device_config.version == 0:
            power, exported, imported = self.__tcp_client.read_holding_registers(

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +77 to +80
rounded_factor = 0
else:
factor = c * v / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

scale_currents() sets rounded_factor = 0 when p == 0, but still executes scaled_currents.append(c / rounded_factor), which will raise ZeroDivisionError at runtime whenever a phase reports 0W power. Handle the p == 0 case by avoiding division (e.g., append 0/current unchanged) or by choosing a non-zero default factor.

Suggested change
rounded_factor = 0
else:
factor = c * v / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))
scaled_currents.append(0)
continue
factor = c * v / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))

Copilot uses AI. Check for mistakes.
imported, exported = self.peak_filter.check_values(power, imported, exported)
currents = self.__tcp_client.read_holding_registers(
0x0017, [ModbusDataType.INT_16]*3, unit=self.__modbus_id)
voltages = [230, 230, 230]
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

Voltages are now hard-coded to [230, 230, 230] and the Modbus voltage read (previously from 0x0014) was removed. This reduces measurement accuracy and also affects the scaling heuristic (it uses c * v / p). Consider restoring the voltage register read (and applying the correct unit scaling) so both CounterState.voltages and the current-scaling logic are based on real voltages.

Suggested change
voltages = [230, 230, 230]
voltages = [
val / 10 for val in self.__tcp_client.read_holding_registers(
0x0014, [ModbusDataType.UINT_16] * 3, unit=self.__modbus_id
)]

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Das ist tatsächlich gewollt. Laut AlphaEss Doku ist der Faktor der Spannungen 0.1.

Allerdings ist laut Doku der Faktor der Ströme ebenfalls 0.1. In der Realität sind die Werte hier aber 0.1, 0.01 und 0.001. Die Spannung ist weit weniger interessant als die Ströme. Einfach um Probleme zu vermeiden und überhaupt eine Berechnungsgrundlage für die Ströme zu haben wird sie hier statisch angenommen.

0x001b, [ModbusDataType.INT_32]*3, unit=self.__modbus_id)
voltages = self.__tcp_client.read_holding_registers(
0x0014, [ModbusDataType.UINT_16]*3, unit=self.__modbus_id)
# currents = [val / 230 for val in powers]
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

There is a leftover commented line # currents = [val / 230 for val in powers] where powers is not defined yet in this branch. Please remove this dead/commented-out code to avoid confusion during future maintenance.

Suggested change
# currents = [val / 230 for val in powers]

Copilot uses AI. Check for mistakes.
@ndrsnhs ndrsnhs requested a review from Copilot April 8, 2026 09:04
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 26 to 32
def initialize(self) -> None:
self.__tcp_client: modbus.ModbusTcpClient_ = self.kwargs['tcp_client']
self.__device_config: AlphaEssConfiguration = self.kwargs['device_config']
self.__modbus_id: int = self.kwargs['modbus_id']
self.store = get_counter_value_store(self.component_config.id)
self.fault_state = FaultState(ComponentInfo.from_component_config(self.component_config))
self.peak_filter = PeakFilter(ComponentType.COUNTER, self.component_config.id, self.fault_state)

Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

PeakFilter usage was removed from the counter (no initialization and no check_values(...) call). This drops the plausibility checks for power and the imported/exported energy counters that other devices (and AlphaEss bat.py / inverter.py) still apply, so spikes/unplausible values will now be written to the store instead of being filtered/warned. Consider restoring PeakFilter for the counter and re-applying check_values(power, imported, exported) before constructing/storing the CounterState.

Copilot uses AI. Check for mistakes.
scaled_currents.append(0)
else:
factor = c * 230 / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

scale_currents() selects the nearest scaling factor via min(factors, key=...). In exact tie cases (e.g. computed factor exactly midway between 10 and 100), min(...) will pick the earlier entry in factors, which currently biases toward the smaller magnitude and can mis-scale currents by 10×. Consider making the selection deterministic for ties (e.g. prefer the larger magnitude) or switching to threshold-based rounding on abs(factor) so boundary cases don’t produce large errors.

Suggested change
rounded_factor = min(factors, key=lambda z: abs(factor - z))
rounded_factor = min(factors, key=lambda z: (abs(factor - z), -abs(z)))

Copilot uses AI. Check for mistakes.
Comment on lines 55 to 64
@@ -65,11 +60,23 @@
)
if 'powers' in locals():
counter_state.powers = powers
if 'voltages' in locals():
counter_state.voltages = voltages
if 'frequency' in locals():
counter_state.frequency = frequency
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

CounterState computes/normalizes currents, powers, and voltages in its constructor, but here powers (and frequency) are set on the instance after construction via if 'powers' in locals(): .... This bypasses the constructor’s normalization/sign-correction logic and can leave currents/powers inconsistent if scaling/rounding is off. Prefer passing powers=powers (and frequency=frequency) directly into CounterState(...) and avoid mutating these fields after initialization.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 41 to 44
exported *= 10
imported *= 10
imported, exported = self.peak_filter(power, imported, exported)
currents = [val / 230 for val in self.__tcp_client.read_holding_registers(
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

PeakFilter instances are not callable (they expose check_values(...)). Calling self.peak_filter(...) will raise TypeError at runtime. Use self.peak_filter.check_values(power, imported, exported) here (and keep the imported, exported assignment order consistent with check_values’ return).

Copilot uses AI. Check for mistakes.
Comment on lines 56 to 59
0x0010, [ModbusDataType.INT_32] * 2, unit=self.__modbus_id
)]
currents = [val / 10 for val in self.__tcp_client.read_holding_registers(
0x0017, [ModbusDataType.INT_16]*3, unit=self.__modbus_id)]
powers = self.__tcp_client.read_holding_registers(
0x001b, [ModbusDataType.INT_32]*3, unit=self.__modbus_id)
voltages = self.__tcp_client.read_holding_registers(
0x0014, [ModbusDataType.UINT_16]*3, unit=self.__modbus_id)
imported, exported = self.peak_filter(power, imported, exported)
frequency = self.__tcp_client.read_holding_registers(
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

Same issue as above: PeakFilter has no __call__; self.peak_filter(power, imported, exported) will crash at runtime. Replace with self.peak_filter.check_values(power, imported, exported).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +79 to +87
scaled_currents = []
for c, p in zip(currents, powers):
if p == 0:
scaled_currents.append(0)
else:
factor = c * 230 / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))
scaled_currents.append(c / rounded_factor)

Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

In scale_currents, when a phase power p is 0 you currently return a scaled current of 0, discarding the measured current register value. This will under-report current for phases with ~0W active power (e.g., reactive loads / low power factor, or rounding to 0W) even though non-zero current can still flow. Consider falling back to the last known scaling factor per phase (cache it when p != 0) or a conservative default factor instead of hard-setting the current to 0.

Suggested change
scaled_currents = []
for c, p in zip(currents, powers):
if p == 0:
scaled_currents.append(0)
else:
factor = c * 230 / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))
scaled_currents.append(c / rounded_factor)
default_factor = min(factors, key=lambda z: abs(abs(z) - 1))
last_factors = getattr(scale_currents, "_last_factors", [])
if len(last_factors) != len(currents):
last_factors = [None] * len(currents)
scaled_currents = []
for index, (c, p) in enumerate(zip(currents, powers)):
if p == 0:
rounded_factor = last_factors[index] if last_factors[index] is not None else default_factor
else:
factor = c * 230 / p
rounded_factor = min(factors, key=lambda z: abs(factor - z))
last_factors[index] = rounded_factor
scaled_currents.append(c / rounded_factor)
scale_currents._last_factors = last_factors

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Wegen Verlust und/ oder Verzögerung können Ströme und Leistung geringfügig vom realen Wert abweichen.
Eine Leistung von 0 impliziert immer einen Strom von 0.
Es wäre möglich in solchen Fällen den gelesenen Strom auszugeben, allerdings ist zur Berechnung des korrekten Faktors eine Leistung ungleich null Voraussetzung.

Angenommen wir haben für Ströme den tatsächlichen Faktor 0.001, die übergebene Leistung ist 0 - der korrekte Faktor kann dann nicht ermittelt werden.
Auf Phase 1 wird ein Strom von real 0.1A gemessen. Aufgrund des Faktors steht aber der Wert 100 im Register.
(100 * 0.001 = 0.1A)
Ströme zu nullen wenn man sie nicht korrekt ermitteln kann ist der einzige Weg.

Comment on lines +65 to +66
currents = scale_currents(currents, powers)
counter_state = CounterState(
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The new scale_currents() heuristic is central to the load-management fix but is currently untested. The repo has extensive device/counter unit tests (e.g. packages/modules/devices/fems/fems/counter_test.py) that validate computed CounterState.currents; adding similar tests for AlphaEss would help prevent regressions across the different scaling factors (10/100/1000) and sign combinations.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants