Skip to content

Solaredge: use modbus bulk reader for bat#3198

Open
LKuemmel wants to merge 5 commits intomasterfrom
feature_solaredge_bat_modbus_bulk_reader
Open

Solaredge: use modbus bulk reader for bat#3198
LKuemmel wants to merge 5 commits intomasterfrom
feature_solaredge_bat_modbus_bulk_reader

Conversation

@LKuemmel
Copy link
Copy Markdown
Contributor

Bei #3166 ist mir aufgefallen, dass der SolarEdge-Speicher keine Register-Blöcke ausliest. SolarEdge empfiehlt das aber.

@LKuemmel
Copy link
Copy Markdown
Contributor Author

@cr0i Magst Du Dir die Änderung ansehen bzw vielleicht sogar den Feature-Branch testen?

@LKuemmel LKuemmel force-pushed the feature_solaredge_bat_modbus_bulk_reader branch from d53b29c to 9b2e7c6 Compare March 11, 2026 11:26
@SchaafAlexander
Copy link
Copy Markdown

Enthält es auch einen Fix für Batterie Modbus Peaks? :-)

#3113

Vielen lieben Dank

@cr0i
Copy link
Copy Markdown
Contributor

cr0i commented Mar 11, 2026

@cr0i Magst Du Dir die Änderung ansehen bzw vielleicht sogar den Feature-Branch testen?

Ich habe den Feature Branch mal ausprobiert, power und soc werden artig abgerufen.
Du hast einen Default-SoC eingebaut (50%), der immer ausgegeben wird, wenn etwas mit dem SoC nicht stimmt.
Ich dachte, die Fehlerbehandlung passiert in den übergeordneten Modulen?
So würde man ja gar nicht merken, wenn etwas nicht stimmt, da einfach ein SoC von 50% ausgegeben wird.
Ist das wirklich so gewünscht?

Die Speichersteuerung funktioniert damit noch nicht.
Es wirkt auf mich so, als wenn set_power_limit gar nicht aufgerufen wird, es gibt keine Log-Ausgabe ausgelesener Register.
Ich habe von 21:30Uhr - 21:33 Uhr auf Ladung mit 5000W gestellt:
https://paste.openwb.de/K425MyQHQJR3yxG

Ist es vielleicht das hier?
2026-03-11 21:30:24,820 - {helpermodules.changed_values_handler:134} - {ERROR:MainThread} - 'int' object is not iterable Traceback (most recent call last): File "/var/www/html/openWB/packages/helpermodules/changed_values_handler.py", line 119, in _update_value dict_prev = dict(asdict(previous_value)) TypeError: 'int' object is not iterable

@LKuemmel LKuemmel force-pushed the feature_solaredge_bat_modbus_bulk_reader branch from dcb6ddf to 15951ab Compare March 12, 2026 07:22
@LKuemmel
Copy link
Copy Markdown
Contributor Author

Danke für Deine Rückmeldung.

Ich habe den Feature Branch mal ausprobiert, power und soc werden artig abgerufen. Du hast einen Default-SoC eingebaut (50%), der immer ausgegeben wird, wenn etwas mit dem SoC nicht stimmt. Ich dachte, die Fehlerbehandlung passiert in den übergeordneten Modulen? So würde man ja gar nicht merken, wenn etwas nicht stimmt, da einfach ein SoC von 50% ausgegeben wird. Ist das wirklich so gewünscht?

Nein, das ist nicht gewollt. Ich habe nicht gesehen, das Copilot das beim Review eingebaut hat. Ist entfernt.

Die Speichersteuerung funktioniert damit noch nicht. Es wirkt auf mich so, als wenn set_power_limit gar nicht aufgerufen wird, es gibt keine Log-Ausgabe ausgelesener Register. Ich habe von 21:30Uhr - 21:33 Uhr auf Ladung mit 5000W gestellt: https://paste.openwb.de/K425MyQHQJR3yxG

Die Fehlerbehandlung war nicht getrennt von den IO-Modulen und das hat einen Fehler geworfen und dann wurde auch das Speicher-Limit nicht mehr gesetzt. Ich habe es korrigiert.

Ist es vielleicht das hier? 2026-03-11 21:30:24,820 - {helpermodules.changed_values_handler:134} - {ERROR:MainThread} - 'int' object is not iterable Traceback (most recent call last): File "/var/www/html/openWB/packages/helpermodules/changed_values_handler.py", line 119, in _update_value dict_prev = dict(asdict(previous_value)) TypeError: 'int' object is not iterable

Habe ich mit einem Rebase auf den Master behoben.

Würdest Du bitte nochmal probieren?

@LKuemmel
Copy link
Copy Markdown
Contributor Author

Enthält es auch einen Fix für Batterie Modbus Peaks? :-)

#3113

Vielen lieben Dank

ist in Arbeit, hat aber nichts mit dem Thema dieses PRs zu tun.

@cr0i
Copy link
Copy Markdown
Contributor

cr0i commented Mar 12, 2026

Hallo @LKuemmel ,

jetzt kommt zwar keine Fehlermeldung mehr, aber trotzdem sehe ich keine Logeinträge von set_power_limit.
Ich hatte erst Keine Speichersteuerung, dann Hausverbrauch und zuletzt auf Ladung mit 5000W gestellt:
https://paste.openwb.de/aGfMARV8wG9D23H

@LKuemmel
Copy link
Copy Markdown
Contributor Author

Für set_power_limit fehlt noch eine Fehlerbehandlung, da wird offenbar der Fehler verschluckt. Wir bauen das erst ein, dann schaue ich hier nochmal, dann ist die Fehlersuche leichter.
Vielen Dank für Deine Mitarbeit bis hierher :)

@LKuemmel LKuemmel marked this pull request as draft March 20, 2026 12:34
@LKuemmel LKuemmel force-pushed the feature_solaredge_bat_modbus_bulk_reader branch from 45bb2fd to 5a43adf Compare April 21, 2026 12:04
@LKuemmel
Copy link
Copy Markdown
Contributor Author

Die Fehlerbehandlung ist nun implementiert. Ich habe den Feature-Branch auf die aktuelle Master rebased.
@cr0i Würdest Du bitte nochmal testen?

@cr0i
Copy link
Copy Markdown
Contributor

cr0i commented Apr 21, 2026

Habe auf manuellen Regellimit (Hausverbrauch) gestellt, nun kommt auch ein Fehler:
https://paste.openwb.de/2KeSMRCV28PNEr6

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

This PR updates the SolarEdge battery module to read contiguous Modbus register blocks via the existing bulk Modbus reader (as recommended by SolarEdge), and adds localized exception handling in the main control Process loop to prevent single-component failures from aborting the whole iteration.

Changes:

  • SolarEdge Bat: replace per-register reads with read_holding_registers_bulk and centralize register addresses via an IntEnum.
  • SolarEdge Bat: simplify register writing by mapping register addresses to data types.
  • Control process: wrap battery-limit thread creation and IO action/output preparation in try/except blocks with contextual logging.

Reviewed changes

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

File Description
packages/modules/devices/solaredge/solaredge/bat.py Switches battery reads/writes to bulk Modbus access and introduces a register enum/type map.
packages/control/process.py Adds per-battery / per-IO-action / per-IO-device exception handling during control-loop preparation.

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

)
power = self.__tcp_client.read_holding_registers(
power_reg, ModbusDataType.FLOAT_32, wordorder=Endian.Little, unit=unit
battery_index = self.component_config.configuration.battery_index
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

battery_index is no longer validated: any value other than 1 will silently select Battery 2 registers. This changes behavior vs. the previous explicit ValueError and can mask configuration mistakes (e.g. battery_index=0 reads the wrong battery). Consider validating that battery_index is either 1 or 2 (raise a clear exception or fall back to 1) before selecting the register base.

Suggested change
battery_index = self.component_config.configuration.battery_index
battery_index = self.component_config.configuration.battery_index
if battery_index not in (1, 2):
raise ValueError(f"Invalid battery_index {battery_index}. Expected 1 or 2.")

Copilot uses AI. Check for mistakes.
)

values = self.__tcp_client.read_holding_registers_bulk(
Registers.STORAGE_CONTROL_MODE, 14, mapping=bulk, unit=unit)
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

read_holding_registers_bulk defaults to wordorder=Endian.Big (see modules/common/modbus.py), but the previous per-register reads in this method used wordorder=Endian.Little for the FLOAT_32 registers. Without passing wordorder=Endian.Little here, REMOTE_CONTROL_CHARGE_LIMIT / REMOTE_CONTROL_DISCHARGE_LIMIT may be decoded incorrectly, which can cause wrong control decisions and writes. Pass the correct wordorder (and byteorder if needed) to the bulk read.

Suggested change
Registers.STORAGE_CONTROL_MODE, 14, mapping=bulk, unit=unit)
Registers.STORAGE_CONTROL_MODE, 14, mapping=bulk, wordorder=Endian.Little, unit=unit)

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

Danke für Deine schnelle Rückmeldung.
Ich habe es korrigiert. Kannst Du bitte nochmal testen?

@cr0i
Copy link
Copy Markdown
Contributor

cr0i commented Apr 22, 2026

Jetzt sieht es gut aus.
Speicher sperren, Hausverbrauch entladen und Speicher laden erfolgreich getestet.

@LKuemmel
Copy link
Copy Markdown
Contributor Author

Danke fürs Testen und die schnelle Rückmeldung!

@LKuemmel LKuemmel marked this pull request as ready for review April 23, 2026 13:24
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.

5 participants