Feature hausverbrauch counter - #3738
Conversation
c74eb4c to
c1d5187
Compare
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy single home_consumption_source_id setting with a per-counter flag (is_home_consumption_counter) and adapts datastore migration, validation, logging/debug output, and home-consumption calculation accordingly.
Changes:
- Bump datastore version and add migrations to introduce
openWB/counter/<id>/config/is_home_consumption_counterand migrate legacyopenWB/counter/config/home_consumption_source_id. - Update runtime handling (MQTT validation, debug output, measurement logging) to use
is_home_consumption_counter. - Rework home-consumption calculation logic and expand unit tests for additional hierarchy scenarios.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/helpermodules/update_config.py | Datastore version bump; add upgrade steps 137/138; topic allowlist/default adjustments |
| packages/helpermodules/setdata.py | Validate new counter config topic is_home_consumption_counter |
| packages/helpermodules/measurement_logging/write_log.py | Exclude home-consumption counters based on new flag |
| packages/helpermodules/create_debug.py | Display counter type based on new flag; minor formatting changes |
| packages/control/counter.py | Add default config key + dataclass field for is_home_consumption_counter |
| packages/control/counter_all.py | Remove legacy config field and implement new home-consumption calculation |
| packages/control/counter_home_consumption_test.py | Expand tests/fixtures for new home-consumption-counter behavior |
| packages/conftest.py | Update test fixture counter mocks with new config flag |
| data/config/mosquitto/public/default-dynamic-security.json | Adjust ACLs for new topic; remove legacy topic ACLs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| max_currents: List[float] = field(default_factory=currents_list_factory, metadata={ | ||
| "topic": "get/max_currents"}) | ||
| max_total_power: float = field(default=0, metadata={"topic": "get/max_total_power"}) | ||
| is_home_consumption_counter: bool = field(default=0, metadata={"topic": "get/is_home_consumption_counter"}) |
| if evu_is_home: | ||
| return evu - not_home_consumption - not_home_consumption_evu, elements_to_sum_up | ||
| else: | ||
| return evu - not_home_consumption - not_home_consumption_evu - home_consumption_evu, elements_to_sum_up |
| child_is_home = is_home or component.data.config.is_home_consumption_counter | ||
|
|
||
| if child["children"]: | ||
| # Alles was unter dem Counter hängt | ||
| home, not_home, child_branch_is_home = self._calc_home_consumption_child(child, child_is_home) | ||
|
|
||
| is_home_local = is_home_local or child_branch_is_home | ||
|
|
||
| # Wurde in den Kindern ein Hausverbrauchszähler gefunden, | ||
| # dann wird der Hausverbrauch aus den Kindern übernommen. | ||
| if child_branch_is_home: | ||
| home_consumption += home | ||
| not_home_consumption += not_home | ||
| else: | ||
| home_consumption += 0.0 | ||
| not_home_consumption += home+not_home | ||
|
|
||
| # Der aktuelle Counter selber | ||
| if child_is_home: | ||
| home_consumption += float(component.data.get.power) - home - not_home | ||
| else: | ||
| not_home_consumption += float(component.data.get.power) - home - not_home |
| source_id = decode_payload(self.all_received_topics[old_topic]) | ||
| if source_id is not None: | ||
| source_id = int(source_id) | ||
| self.__update_topic(f"openWB/counter/{source_id}/config/is_home_consumption_counter", True) |
LKuemmel
left a comment
There was a problem hiding this comment.
Es gibt noch einen Artikel im Wiki zum Hausverbrauch. Der müsste auch angepasst werden.
| max_currents: List[float] = field(default_factory=currents_list_factory, metadata={ | ||
| "topic": "get/max_currents"}) | ||
| max_total_power: float = field(default=0, metadata={"topic": "get/max_total_power"}) | ||
| is_home_consumption_counter: bool = field(default=False, metadata={"topic": "get/is_home_consumption_counter"}) |
There was a problem hiding this comment.
| is_home_consumption_counter: bool = field(default=False, metadata={"topic": "get/is_home_consumption_counter"}) | |
| is_home_consumption_counter: bool = field(default=False, metadata={"topic": "config/is_home_consumption_counter"}) |
Das Topic bitte auch bei max_currents und max_total_power korrigieren.
| def set_home_consumption(self) -> None: | ||
| try: | ||
| self._validate_home_consumption_counter() | ||
| # self._validate_home_consumption_counter() |
There was a problem hiding this comment.
Toten Code bitte entfernen.
| home_consumption = 0 | ||
| not_home_consumption = 0 | ||
| not_home_consumption_evu = 0 | ||
| id_source = self.get_id_evu_counter() |
There was a problem hiding this comment.
Das ist ja immer die ID vom EVU-Zähler. Dann sollte das auch aus dem Namen hervorgehen.
| id_source = self.get_id_evu_counter() | |
| evu_id = self.get_id_evu_counter() |
| if is_home_branch: | ||
| home_consumption += float(component.data.get.power) - home - not_home | ||
| else: | ||
| not_home_consumption += float(component.data.get.power) - home - not_home | ||
|
|
||
| if component.data.get.fault_state < 2: | ||
| # Power über alles | ||
| power += component.data.get.power | ||
|
|
||
| # Power von allen Komponenten aus der ersten ebene des EVU | ||
| if element["type"] != ComponentType.COUNTER.value: | ||
| not_home_consumption_evu += component.data.get.power |
There was a problem hiding this comment.
Irgendwie ist die Berechnung recht kompliziert geworden. Die Komponenten werden zweimal eingerechnet und dann wieder abgezogen?...
Macht es die Berechnung nachvollziehbarer, wenn Du noch Methoden zur Hierarchie-Analyse hinzufügst? Eine Methode, die alle Ladepunkte, WR und Speicher eines Zählers unabhängig in welcher Unterebene heraussucht.
Eine Generator-Methode, die von oben die Hierarchie durchgeht und sich durch die Children arbeitet, bis ein Zäher HV = True ist. Alle weiteren Child-Zähler können dann ignoriert werden, weil deren Leistung ist ja bereits im übergeordneten HV-Zähler enthalten.
Dann könnte man die Berechnung so aufbauen:
EVU misst HV:
bisherige Berechnung + Leistung der HV-Zähler, die die Generator-Funktion zurück gibt - Leistung, die die erste Methode zurückgibt, wenn man ihr als Parameter den Zähler aus der Generator-Funktion übergibt.
EVU misst kein HV:
Leistung der HV-Zähler, die die Generator-Funktion zurück gibt - Leistung, die die erste Methode zurückgibt, wenn man ihr als Parameter den Zähler aus der Generator-Funktion übergibt.
Hier zahlt sich dann auch der Test aus, den Du geschrieben hast. Du änderst ja nichts daran, welche Werte berücksichtigt werden sollen, sondern nur die Berechnung an sich. dh der Test sollte danach immer noch durchlaufen.
| "cp3": Mock(spec=Chargepoint, data=Mock(spec=ChargepointData, | ||
| config=Mock(spec=Config, phase_1=1), | ||
| get=Mock(spec=Get, currents=[30, 0, 0], power=6900, | ||
| daily_imported=10000, daily_exported=0, imported=56000, | ||
| fault_state=0), | ||
| set=Mock(spec=Set, loadmanagement_available=True)), |
There was a problem hiding this comment.
Bei Mock(spec= muss man nich alle Attribute setzen, nur die, die für den Test benötigt werden. Hier also die Leistung und bei den Zählern ist_home_consumption, das macht die Testdaten sehr viel übersichtlicher.
7e543b4 to
1e46081
Compare
| try: | ||
| home_consumption_source_id = data.data.counter_all_data.data.config.home_consumption_source_id | ||
| if (home_consumption_source_id is None or counter.num != home_consumption_source_id): | ||
| if not counter.data.config.is_home_consumption_counter: |
There was a problem hiding this comment.
Das bedeutet, das die einzelnen Zähler nicht mehr in den Auswertungs-Diagrammen und Strommix-Anteilen einzeln aufgelistet werden. Wenn man mehrere Zähler installiert hat, sollten die auch auftauchen.
In der vorherigen Implementierung entsprach der HV exakt dem ausgewählten Zähler, dann gab es zwei Kurven die sich überlagert haben, dann braucht man nur eine.
Wenn mehrere Zähler jeweils einen Teil des Hausverbrauchs messen, sollten die auch einzeln in den Diagrammen erscheinen.
|
|
||
| return not_home_consumption | ||
|
|
||
| def _get_home_consumption_counter(self, elements, evu_is_HC=False) -> Tuple[float, List]: |
There was a problem hiding this comment.
Bitte noch Type-Hints für die Parameter hinzufügen.
| if element["type"] != ComponentType.COUNTER.value: | ||
| # Wenn kein Counter, dann get power davon -> not_home_consumption | ||
| if element["type"] == ComponentType.CHARGEPOINT.value: | ||
| component = data.data.cp_data[f"cp{element['id']}"] | ||
| elif element["type"] == ComponentType.BAT.value: | ||
| component = data.data.bat_data[f"bat{element['id']}"] | ||
| elif element["type"] == ComponentType.INVERTER.value: | ||
| component = data.data.pv_data[f"pv{element['id']}"] | ||
|
|
||
| if component.data.get.fault_state < 2: | ||
| not_home_consumption += component.data.get.power |
There was a problem hiding this comment.
Kannst Du die Berechnung noch etwas umstrukturieren? Diesen Teil gibt es in _calc_home_consumption fast genau so schon mal, sodass es den nur einmal gibt.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/helpermodules/update_config.py:3548
- Wenn der alte Topic
openWB/counter/config/home_consumption_source_idexistiert, abernull/Noneist (das war bisher der Default für "von openWB berechnen"), werden zwar die neuen Topics angelegt, aber kein Zähler als Hausverbrauchs-Zähler markiert. Damit liefert die neue Berechnung effektiv 0W Hausverbrauch, bis der Nutzer manuell konfiguriert. Bitte in diesem Fall den EVU-Zähler (hierarchy[0]) als Quelle migrieren, damit das bisherige Default-Verhalten erhalten bleibt.
if source_id is not None:
try:
source_id = int(source_id)
except (TypeError, ValueError):
log.warning(f"Invalid '{old_topic}' value: {source_id!r}; skipping migration")
packages/control/counter_all.py:183
- Die Rekursion in
_calc_home_consumption_from_counterhängt aktuell vonisinstance(comp, Counter)ab. In Tests (Mocks) und potenziell auch bei Wrapper-/Simulationsobjekten indata.data.counter_dataist das nicht zuverlässig und führt dazu, dass verschachtelte Counter-Branches nicht weiter traversiert werden. Da die Hierarchie bereits das Feldtypeenthält, sollte darüber entschieden werden.
for child in element["children"]:
comp = self._get_component(child)
if comp.data.get.fault_state < 2:
if isinstance(comp, Counter):
home_consumption += self._calc_home_consumption_from_counter(child, child_home_consumption)
packages/control/counter_all.py:190
_calc_home_consumptiongibt als zweites Tupel-Element jetztevu_element(ein Dict mitid/type/children) zurück, aber der Rückgabetyp ist weiterhin alsTuple[float, List]annotiert. Bitte die Typannotation anpassen, damit Call-Sites und Static-Checks nicht in die Irre laufen.
def _calc_home_consumption(self) -> Tuple[float, List]:
| NOT_HOME_CONSUMPTION = 0 | ||
| HOME_CONSUMPTION = 1 | ||
| AUTO_HOME_CONSUMPTION = 2 |
There was a problem hiding this comment.
Man kann als Enum-Werte auch Strings verwenden. Dann hat man im Broker und UI-Code einen lesbaren Wert, ohne erst im Python-Code nachschauen zu müssen, was denn nun "1" bedeutet.
https://github.com/openWB/internal-projects/issues/266
openWB/openwb-ui-settings#1032