Add Universeller Modbus by AlexanderHa98 · Pull Request #3699 · openWB/core

AlexanderHa98 · 2026-07-23T07:48:04Z

UI: openWB/openwb-ui-settings#1023
Ticket: https://github.com/openWB/internal-projects/issues/327

Copilot

Pull request overview

Adds a new “Universeller Modbus” device module under packages/modules/devices/generic/modbus to support configurable Modbus TCP-based counter, inverter, and battery components (configurable register mapping, datatype and endianness), aligned with the referenced UI/ticket work.

Changes:

Introduces a configurable Modbus TCP device (device.py) that instantiates counter/bat/inverter components.
Adds component implementations for counter, inverter, and battery that read values via Modbus input registers and publish *State to the stores.
Adds configuration models (config.py) and a small validation helper (helper.py) for register configuration completeness.

Reviewed changes

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

Show a summary per file

File	Description
packages/modules/devices/generic/modbus/device.py	New configurable device wiring for Modbus TCP client + component creation/update.
packages/modules/devices/generic/modbus/config.py	New device/component configuration classes for universal Modbus mapping.
packages/modules/devices/generic/modbus/helper.py	Adds `check_data()` validation for register config completeness.
packages/modules/devices/generic/modbus/counter.py	New generic Modbus counter component reading power/energy and optional phase metrics.
packages/modules/devices/generic/modbus/inverter.py	New generic Modbus inverter component reading power/energy and optional currents/DC power.
packages/modules/devices/generic/modbus/bat.py	New generic Modbus battery component reading power/energy and optional SOC/currents.

Comments suppressed due to low confidence (3)

packages/modules/devices/generic/modbus/config.py:32

gerneric_modbus ist als mutable Dataclass mit nicht-optional typisierten Feldern deklariert, wird aber überall mit None initialisiert. Dadurch stimmen Typen/Defaults nicht, und als Default-Argument wiederverwendete Instanzen können unbeabsichtigt geteilt werden. Optional-Felder mit Defaults (und idealerweise Immutability) vermeiden das.

@dataclass
class gerneric_modbus:
    reg_address: int
    reg_type: str
    byteorder: str
    wordorder: str

packages/modules/devices/generic/modbus/bat.py:137

Optionalfelder werden nachträglich per locals() am BatState gesetzt. Dadurch werden Defaulting/Validierungen in BatState.__init__ (z.B. Strom-Sign-Prüfung) umgangen. Besser: Optionalwerte direkt im Konstruktor übergeben.

        bat_state = BatState(
            imported=imported,
            exported=exported,
            power=power,
        )

packages/modules/devices/generic/modbus/counter.py:226

Beim Simulieren von importiert/exportiert werden die simulierten Zählerstände erneut durch den PeakFilter geschickt. Der PeakFilter kann dabei (z.B. beim Startup) bewusst None zurückgeben und so die simulierten Werte verwerfen. In anderen Modulen wird bei SimCounter nur die Leistung geprüft und die Zähler nicht gefiltert.

        if power is not None:
            self.peak_filter.check_values(power)
            if imported is None or exported is None:
                imported, exported = self.sim_counter.sim_count(power)
            imported, exported = self.peak_filter.check_values(power, imported, exported)

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

+class GenericModbusConfiguration:
+    def __init__(self, ip_address: str = "192.168.1.230", port: int = 502):
+        self.ip_address = ip_address
+        self.port = port


LKuemmel · 2026-07-23T10:22:22Z

+        if power is not None:
+            self.peak_filter.check_values(power)
+            if imported is None or exported is None:
+                imported, exported = self.sim_counter.sim_count(power)
+            imported, exported = self.peak_filter.check_values(power, imported, exported)


Bitte noch umsetzen.

+        inverter_state = InverterState(
+            power=power,
+            exported=exported,
+            imported=imported,
+        )
+
+        if "dc_power" in locals():
+            inverter_state.dc_power = dc_power
+        if "currents" in locals():
+            inverter_state.currents = currents
+        if 'serial_number' in locals():
+            inverter_state.serial_number = serial_number


+        if power is not None:
+            self.peak_filter.check_values(power)
+            if imported is None or exported is None:
+                imported, exported = self.sim_counter.sim_count(power)
+            imported, exported = self.peak_filter.check_values(power, imported, exported)


+        counter_state = CounterState(
+            imported=imported,
+            exported=exported,
+            power=power
+        )
+
+        if "voltages" in locals():
+            counter_state.voltages = voltages
+        if "currents" in locals():
+            counter_state.currents = currents
+        if 'powers' in locals():
+            counter_state.powers = powers
+        if "power_factors" in locals():
+            counter_state.power_factors = power_factors
+        if 'frequency' in locals():
+            counter_state.frequency = frequency
+        if 'serial_number' in locals():
+            counter_state.serial_number = serial_number


+                    byteorder=self.component_config.configuration.powers_L3.byteorder,
+                    wordorder=self.component_config.configuration.powers_L3.wordorder, unit=unit)
+
+        # Power Factors‚


LKuemmel · 2026-07-23T10:22:43Z

+        self.__modbus_id: int = self.kwargs['device_id']
+        self.client: ModbusTcpClient_ = self.kwargs['client']
+        self.store = get_bat_value_store(self.component_config.id)
+        self.fault_state = FaultState(ComponentInfo.from_component_config(self.component_config))
+        self.peak_filter = PeakFilter(ComponentType.BAT, self.component_config.id, self.fault_state)
+        self.sim_counter = SimCounter(self.__modbus_id, self.component_config.id, prefix="speicher")


Bitte noch umsetzen.

LKuemmel · 2026-07-23T10:02:18Z

+    def initialize(self) -> None:
+        self.__modbus_id: int = self.kwargs['device_id']
+        self.client: ModbusTcpClient_ = self.kwargs['client']
+        self.store = get_bat_value_store(self.component_config.id)


Suggested change

self.store = get_bat_value_store(self.component_config.id)

self.store = get_component_value_store(self.component_config.type, self.component_config.id)

Der Aufruf sieht einheitlich für alle Komponenten so aus.

LKuemmel · 2026-07-23T10:02:52Z

+        if self.component_config.configuration.power.reg_address is not None:
+            check_data(self.component_config.configuration.power)
+            power = self.client.read_input_registers(
+                int(self.component_config.configuration.power.reg_address), ModbusDataType[


Suggested change

int(self.component_config.configuration.power.reg_address), ModbusDataType[

self.component_config.configuration.power.reg_address, ModbusDataType[

reg_address und die anderen Adressen sollten doch schon ein int sein.

LKuemmel · 2026-07-23T10:09:41Z

+            soc = self.client.read_input_registers(
+                int(self.component_config.configuration.soc.reg_address), ModbusDataType[
+                    self.component_config.configuration.soc.reg_type],
+                byteorder=self.component_config.configuration.soc.byteorder,
+                wordorder=self.component_config.configuration.soc.wordorder, unit=unit)


Man könnte den Aufruf in eine eigene Methode oder Nested Function packen und self.component_config.configuration.soc als Variable übergeben. Der Typ ist ja immer gerneric_modbus.

LKuemmel · 2026-07-23T10:11:18Z

+
+
+@dataclass
+class gerneric_modbus:


Bitte an die Naming Convention für Klassennamen halten. Hier würde zB RegisterConfig oä passen.

LKuemmel · 2026-07-23T10:14:45Z

+                 serial_number: gerneric_modbus = gerneric_modbus(
+                     reg_address=None, reg_type=None, byteorder=None, wordorder=None)):
+
+        self.modbus_id = modbus_id


GenericModbusCounterConfiguration kannst Du als dataclass anlegen, dann musst Du nach init nicht nochmal alle Parameter zuweisen.

LKuemmel · 2026-07-23T10:15:46Z

+
+class GenericModbusCounterConfiguration:
+    def __init__(self, modbus_id: int = 105,
+                 voltage_L1: gerneric_modbus = gerneric_modbus(


Wenn Du gerneric_modbus Default-Parameter spendierst, kannst Du sie hier weglassen, dann ist die Liste nicht so lang.

LKuemmel · 2026-07-23T10:17:50Z

+        if (self.component_config.configuration.voltage_L1.reg_address is not None or
+            self.component_config.configuration.voltage_L2.reg_address is not None or
+                self.component_config.configuration.voltage_L3.reg_address is not None):
+            # Kein register angegeben


Der Kommentar ist nicht mehr aktuell.

LKuemmel · 2026-07-23T14:16:57Z

+        if soc_value is not None:
+            soc = soc_value


Suggested change

if soc_value is not None:

soc = soc_value

Unten setzt Du auch auf None, wenn es die Variable nicht in locals gibt. Das ist dann doppelt.

LKuemmel · 2026-07-23T14:18:17Z

+            f"Unvollständige Konfiguration für Universeller-Modbus: Register-Adresse {wert.reg_address}")
+
+
+def read_value(client: ModbusTcpClient_, unit: int, register_config: Any) -> Any:


Suggested change

def read_value(client: ModbusTcpClient_, unit: int, register_config: Any) -> Any:

def read_value(client: ModbusTcpClient_, unit: int, register_config: RegisterConfig) -> Any:

AlexanderHa98 requested review from LKuemmel and Copilot July 23, 2026 09:06

Copilot started reviewing on behalf of AlexanderHa98 July 23, 2026 09:06 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

AlexanderHa98 force-pushed the feature_universelles_modbus_device branch from d43ce0c to bdd9550 Compare July 23, 2026 09:17

AlexanderHa98 added 3 commits July 23, 2026 11:28

Add Universeller Modbus

7caffa9

fix

4189fca

fix

adc6fed

AlexanderHa98 force-pushed the feature_universelles_modbus_device branch from bdd9550 to adc6fed Compare July 23, 2026 09:28

Fix imports

c1986d7

LKuemmel requested changes Jul 23, 2026

View reviewed changes

Add suggestions and refactor

ea2c51c

AlexanderHa98 requested a review from LKuemmel July 23, 2026 14:10

LKuemmel requested changes Jul 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Universeller Modbus#3699

Add Universeller Modbus#3699
AlexanderHa98 wants to merge 5 commits into
openWB:masterfrom
AlexanderHa98:feature_universelles_modbus_device

AlexanderHa98 commented Jul 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

LKuemmel Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	self.store = get_bat_value_store(self.component_config.id)
	self.store = get_component_value_store(self.component_config.type, self.component_config.id)

	int(self.component_config.configuration.power.reg_address), ModbusDataType[
	self.component_config.configuration.power.reg_address, ModbusDataType[

		f"Unvollständige Konfiguration für Universeller-Modbus: Register-Adresse {wert.reg_address}")


		def read_value(client: ModbusTcpClient_, unit: int, register_config: Any) -> Any:

Uh oh!

Conversation

AlexanderHa98 commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AlexanderHa98 commented Jul 23, 2026 •

edited

Loading