Conversation
|
@cr0i Magst Du Dir die Änderung ansehen bzw vielleicht sogar den Feature-Branch testen? |
d53b29c to
9b2e7c6
Compare
|
Enthält es auch einen Fix für Batterie Modbus Peaks? :-) Vielen lieben Dank |
Ich habe den Feature Branch mal ausprobiert, power und soc werden artig abgerufen. Die Speichersteuerung funktioniert damit noch nicht. Ist es vielleicht das hier? |
dcb6ddf to
15951ab
Compare
|
Danke für Deine Rückmeldung.
Nein, das ist nicht gewollt. Ich habe nicht gesehen, das Copilot das beim Review eingebaut hat. Ist entfernt.
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.
Habe ich mit einem Rebase auf den Master behoben. Würdest Du bitte nochmal probieren? |
ist in Arbeit, hat aber nichts mit dem Thema dieses PRs zu tun. |
|
Hallo @LKuemmel , jetzt kommt zwar keine Fehlermeldung mehr, aber trotzdem sehe ich keine Logeinträge von set_power_limit. |
|
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. |
45bb2fd to
5a43adf
Compare
|
Die Fehlerbehandlung ist nun implementiert. Ich habe den Feature-Branch auf die aktuelle Master rebased. |
|
Habe auf manuellen Regellimit (Hausverbrauch) gestellt, nun kommt auch ein Fehler: |
There was a problem hiding this comment.
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_bulkand centralize register addresses via anIntEnum. - 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/exceptblocks 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 |
There was a problem hiding this comment.
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.
| 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.") |
| ) | ||
|
|
||
| values = self.__tcp_client.read_holding_registers_bulk( | ||
| Registers.STORAGE_CONTROL_MODE, 14, mapping=bulk, unit=unit) |
There was a problem hiding this comment.
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.
| Registers.STORAGE_CONTROL_MODE, 14, mapping=bulk, unit=unit) | |
| Registers.STORAGE_CONTROL_MODE, 14, mapping=bulk, wordorder=Endian.Little, unit=unit) |
|
Danke für Deine schnelle Rückmeldung. |
|
Jetzt sieht es gut aus. |
|
Danke fürs Testen und die schnelle Rückmeldung! |
Bei #3166 ist mir aufgefallen, dass der SolarEdge-Speicher keine Register-Blöcke ausliest. SolarEdge empfiehlt das aber.