Verbraucher: Fix verknüpfte Zähler nicht löschbar und in Hierarchie-Validierung berücksichtigen

packages/control/counter_all/counter_all_data.py

@@ -77,8 +77,8 @@ def _add_item_aside(self, child: Dict, current_entry: Dict, id_to_find: int,
     def _add_item_below(self, child: Dict, current_entry: Dict, id_to_find: int,
                         new_id: int, new_type: ComponentType) -> bool: ...
 
-    def _add_missing_entries(self): ...
-    def _delete_obsolete_entries(self): ...
+    def _add_missing_entries(self, linked_extra_meter_counter_ids: List[int]): ...
+    def _delete_obsolete_entries(self, linked_extra_meter_counter_ids: List[int]): ...
     def _edit_element_in_hierarchy(self, current_entry: Dict, id_to_find: int, func: Callable, *args) -> bool: ...
     def _get_all_counter_in_branch(self, child: Dict, id_to_find: int) -> bool: ...
     def _get_all_cp_connected_to_counter(self, child: Dict) -> None: ...

packages/control/counter_all/hierarchy.py

@@ -341,19 +341,29 @@ def get_non_hybrid_bat_ids(self: HierarchyProtocol) -> List[int]:
 
     def validate_hierarchy(self: HierarchyProtocol):
         try:
-            self._delete_obsolete_entries()
-            self._add_missing_entries()
+            linked_extra_meter_counter_ids = {
+                consumer.data.extra_meter
+                for consumer in data.data.consumer_data.values()
+                if consumer.data.extra_meter is not None
+            }
+            self._delete_obsolete_entries(linked_extra_meter_counter_ids)
+            self._add_missing_entries(linked_extra_meter_counter_ids)
         except Exception:
             log.exception("Fehler bei der Validierung der Hierarchie")
 
-    def _delete_obsolete_entries(self: HierarchyProtocol):
+    def _delete_obsolete_entries(self: HierarchyProtocol, linked_extra_meter_counter_ids: List[int]):
         def check_and_remove(name, type_name: ComponentType, data_structure):
             if element["type"] == type_name.value:
                 if f"{name}{element['id']}" not in data_structure:
                     self.hierarchy_remove_item(element["id"])
                     pub_system_message({}, f"{component_type_to_readable_text(type_name)} mit ID {element['id']} wurde"
                                        " aus der Hierarchie entfernt, da keine gültige Konfiguration gefunden wurde.",
                                        MessageType.WARNING)
+                elif type_name == ComponentType.COUNTER and element["id"] in linked_extra_meter_counter_ids:
+                    self.hierarchy_remove_item(element["id"])
+                    pub_system_message({}, f"Zähler mit ID {element['id']} wurde aus der Hierarchie entfernt, da er "
+                                       "einem Verbraucher als separate Leistungsmessung zugeordnet ist.",
+                                       MessageType.WARNING)
 
         for level in self.get_list_of_elements_per_level():
             for element in level:
@@ -363,13 +373,7 @@ def check_and_remove(name, type_name: ComponentType, data_structure):
                 check_and_remove("pv", ComponentType.INVERTER, data.data.pv_data)
                 check_and_remove("consumer", ComponentType.CONSUMER, data.data.consumer_data)
 
-    def _add_missing_entries(self: HierarchyProtocol):
-        linked_extra_meter_counter_ids = {
-            consumer.data.extra_meter
-            for consumer in data.data.consumer_data.values()
-            if consumer.data.extra_meter is not None
-        }
-
+    def _add_missing_entries(self: HierarchyProtocol, linked_extra_meter_counter_ids: List[int]):
         def check_and_add(type_name: ComponentType, data_structure):
             for entry in data_structure:
                 break_flag = False

packages/control/counter_all/hierarchy_test.py

@@ -326,7 +326,7 @@ def test_delete_obsolete_entries(hierarchy, data_):
     counter_all.data.get.hierarchy = hierarchy
 
     # execution
-    counter_all._delete_obsolete_entries()
+    counter_all._delete_obsolete_entries(set())
 
     # evaluation
     assert counter_all.data.get.hierarchy == hierarchy_nested().data.get.hierarchy
@@ -398,12 +398,41 @@ def test_add_missing_entries(hierarchy, expected_hierarchy, data_, monkeypatch):
     monkeypatch.setattr(CounterAll, "get_evu_counter", mock_get_evu_counter)
 
     # execution
-    counter_all._add_missing_entries()
+    counter_all._add_missing_entries(set())
 
     # evaluation
     assert counter_all.data.get.hierarchy == expected_hierarchy
 
 
+@pytest.mark.parametrize("linked_meter_in_hierarchy", [False, True])
+def test_validate_hierarchy_removes_linked_counter_and_keeps_children(linked_meter_in_hierarchy, monkeypatch):
+    data.data_init(Mock())
+    counters = {f"counter{i}": Mock(spec=Counter, num=i) for i in (0, 6, 8)}
+    consumers = {"consumer7": Mock(spec=Consumer, data=Mock(spec=ConsumerData, extra_meter=6))}
+    data.data.counter_data = counters
+    data.data.consumer_data = consumers
+    data.data.bat_data = {}
+    data.data.cp_data = {}
+    data.data.pv_data = {}
+    children = [
+        {"id": 7, "type": "consumer", "children": []},
+        {"id": 8, "type": "counter", "children": []},
+    ]
+    counter_all = CounterAll()
+    counter_all.data.get.hierarchy = [{
+        "id": 0, "type": "counter", "children": (
+            [{"id": 6, "type": "counter", "children": children}] if linked_meter_in_hierarchy else children),
+    }]
+    monkeypatch.setattr(CounterAll, "get_evu_counter", Mock(return_value=counters["counter0"]))
+
+    counter_all.validate_hierarchy()
+    counter_all.validate_hierarchy()
+
+    assert counter_all.data.get.hierarchy == [{"id": 0, "type": "counter", "children": children}]
+    assert data.data.counter_data == counters
+    assert data.data.consumer_data["consumer7"].data.extra_meter == 6
+
+
 def test_add_missing_entries_re_adds_removed_linked_counter(monkeypatch):
     # setup
     data.data_init(Mock())
@@ -429,7 +458,7 @@ def test_add_missing_entries_re_adds_removed_linked_counter(monkeypatch):
     monkeypatch.setattr(CounterAll, "get_evu_counter", Mock(return_value=c))
 
     # execution
-    counter_all._add_missing_entries()
+    counter_all._add_missing_entries({6})
 
     # evaluation
     assert counter_all.data.get.hierarchy == [{

packages/helpermodules/command.py

@@ -215,6 +215,11 @@ def removeDevice(self, connection_id: str, payload: dict) -> None:
         """ löscht ein Device.
         """
         if self.max_id_device >= payload["data"]["id"]:
+            device = SubData.system_data.get(f'device{payload["data"]["id"]}')
+            if device is not None:
+                for component in device.components.values():
+                    if self._prevent_linked_meter_removal(connection_id, payload, component.component_config.id):
+                        return
             ProcessBrokerBranch(f'system/device/{payload["data"]["id"]}/').remove_topics()
             pub_user_message(payload, connection_id, f'Gerät mit ID \'{payload["data"]["id"]}\' gelöscht.',
                              MessageType.SUCCESS)
@@ -727,12 +732,26 @@ def set_default(topic: str, defaults: dict):
             f'Neue Komponente vom Typ \'{payload["data"]["type"]}\' mit ID \'{new_id}\' hinzugefügt.',
             MessageType.SUCCESS)
 
+    def _prevent_linked_meter_removal(self, connection_id: str, payload: dict, component_id: int) -> bool:
+        for consumer in SubData.consumer_data.values():
+            if consumer.data.extra_meter is not None and consumer.data.extra_meter == component_id:
+                pub_user_message(
+                    payload, connection_id,
+                    f'Zähler mit ID \'{component_id}\' kann nicht gelöscht werden, da er beim Verbraucher '
+                    f'\'{consumer.data.module.name if consumer.data.module is not None else "Verbraucher"}\' als '
+                    'separate Leistungsmessung zugeordnet ist. Bitte zuerst die Zuordnung beim Verbraucher ändern.',
+                    MessageType.ERROR)
+                return True
+        return False
+
     def removeComponent(self, connection_id: str, payload: dict) -> None:
         """ löscht eine Komponente.
         """
         if self.max_id_hierarchy < payload["data"]["id"]:
             log.error(payload, connection_id,
                       "Die ID ist größer als die maximal vergebene ID.", MessageType.ERROR)
+        if self._prevent_linked_meter_removal(connection_id, payload, payload["data"]["id"]):
+            return
         branch = f'system/device/{payload["data"]["deviceId"]}/component/{payload["data"]["id"]}/'
         ProcessBrokerBranch(branch).remove_topics()
         # remove ACL roles for component access, if user management is active

packages/helpermodules/command_test.py

@@ -21,6 +21,54 @@
 pytest_plugins = ["helpermodules.measurement_logging.process_log_testdata", "helpermodules.command_test_data"]
 
 
+@pytest.mark.parametrize("command_name", ["removeComponent", "removeDevice"])
+@pytest.mark.parametrize("meter_id", [0, 4])
+@pytest.mark.parametrize("linked_meter", [None, 0, 4, 9])
+def test_remove_linked_consumer_meter(command_name, meter_id, linked_meter, monkeypatch):
+    c = Command.__new__(Command)
+    c.max_id_device = 2
+    c.max_id_hierarchy = 10
+    monkeypatch.setattr(SubData, "consumer_data", {
+        "consumer8": SimpleNamespace(data=SimpleNamespace(
+            extra_meter=linked_meter, module=SimpleNamespace(name="Wärmepumpe"))),
+    })
+    monkeypatch.setattr(SubData, "system_data", {
+        "device2": SimpleNamespace(components={
+            "component7": SimpleNamespace(component_config=SimpleNamespace(id=7)),
+            f"component{meter_id}": SimpleNamespace(component_config=SimpleNamespace(id=meter_id)),
+        }),
+        "system": SimpleNamespace(data={"security": {"user_management_active": False}}),
+    })
+    broker_branch = Mock()
+    message = Mock()
+    remove_role = Mock()
+    monkeypatch.setattr(command, "ProcessBrokerBranch", broker_branch)
+    monkeypatch.setattr(command, "pub_user_message", message)
+    monkeypatch.setattr(command, "remove_acl_role", remove_role)
+    data = {"id": 2} if command_name == "removeDevice" else {
+        "deviceId": 2, "id": meter_id, "type": "counter",
+    }
+    payload = {"data": data}
+
+    getattr(c, command_name)("test", payload)
+
+    if linked_meter == meter_id:
+        broker_branch.assert_not_called()
+        remove_role.assert_not_called()
+        message.assert_called_once()
+        args = message.call_args.args
+        assert args[:2] == (payload, "test")
+        assert args[3] == command.MessageType.ERROR
+        assert "Wärmepumpe" in args[2]
+        assert "separate Leistungsmessung" in args[2]
+    else:
+        branch = "system/device/2/" if command_name == "removeDevice" else (
+            f"system/device/2/component/{meter_id}/")
+        broker_branch.assert_called_once_with(branch)
+        broker_branch.return_value.remove_topics.assert_called_once_with()
+        assert message.call_args.args[3] == command.MessageType.SUCCESS
+
+
 @pytest.fixture
 def subdata_fixture() -> None:
     SubData(*([Mock()]*16))

packages/helpermodules/subdata.py

@@ -1316,6 +1316,9 @@ def process_consumer_topic(self, client: mqtt.Client, var: Dict[str, Consumer],
                         self.set_json_payload_class(var[f"consumer{index}"].data, msg)
                         if self.event_subdata_initialized.is_set() and old_extra_meter != var[
                                 f"consumer{index}"].data.extra_meter:
+                            # Only restore the previous meter if its configuration still exists.
+                            if f"counter{old_extra_meter}" not in self.counter_data:
+                                old_extra_meter = None
                             if self.counter_all_data.update_linked_counter_hierarchy(
                                     int(index),
                                     old_extra_meter,

packages/helpermodules/subdata_test.py

@@ -130,7 +130,9 @@ def test_process_consumer_extra_meter_removes_linked_counter_from_hierarchy(monk
     mock_pub.pub.assert_called_once_with("openWB/set/counter/get/hierarchy", counter_all_data.data.get.hierarchy)
 
 
-def test_process_consumer_extra_meter_adds_unlinked_counter_to_hierarchy(monkeypatch: pytest.MonkeyPatch,
+@pytest.mark.parametrize("counter_exists", [True, False])
+def test_process_consumer_extra_meter_adds_unlinked_counter_to_hierarchy(counter_exists: bool,
+                                                                         monkeypatch: pytest.MonkeyPatch,
                                                                          mock_pub: Mock):
     subdata = SubData.__new__(SubData)
     subdata.event_subdata_initialized = Mock()
@@ -145,6 +147,7 @@ def test_process_consumer_extra_meter_adds_unlinked_counter_to_hierarchy(monkeyp
     consumers = {"consumer2": Consumer(2)}
     consumers["consumer2"].data.extra_meter = 5
     msg = Mock(topic="openWB/consumer/2/extra_meter", payload=b"null")
+    monkeypatch.setattr(SubData, "counter_data", {"counter5": Mock()} if counter_exists else {})
 
     subdata.process_consumer_topic(Mock(), consumers, msg)
 
@@ -154,13 +157,17 @@ def test_process_consumer_extra_meter_adds_unlinked_counter_to_hierarchy(monkeyp
         "type": "counter",
         "children": [
             {"id": 2, "type": "consumer", "children": []},
-            {"id": 5, "type": "counter", "children": []},
-        ],
+        ] + ([{"id": 5, "type": "counter", "children": []}] if counter_exists else []),
     }]
-    mock_pub.pub.assert_called_once_with("openWB/set/counter/get/hierarchy", counter_all_data.data.get.hierarchy)
+    if counter_exists:
+        mock_pub.pub.assert_called_once_with("openWB/set/counter/get/hierarchy", counter_all_data.data.get.hierarchy)
+    else:
+        mock_pub.pub.assert_not_called()
 
 
-def test_process_consumer_extra_meter_switches_linked_counter_in_hierarchy(monkeypatch: pytest.MonkeyPatch,
+@pytest.mark.parametrize("counter_exists", [True, False])
+def test_process_consumer_extra_meter_switches_linked_counter_in_hierarchy(counter_exists: bool,
+                                                                           monkeypatch: pytest.MonkeyPatch,
                                                                            mock_pub: Mock):
     subdata = SubData.__new__(SubData)
     subdata.event_subdata_initialized = Mock()
@@ -178,6 +185,9 @@ def test_process_consumer_extra_meter_switches_linked_counter_in_hierarchy(monke
     consumers = {"consumer2": Consumer(2)}
     consumers["consumer2"].data.extra_meter = 5
     msg = Mock(topic="openWB/consumer/2/extra_meter", payload=b"6")
+    monkeypatch.setattr(SubData, "counter_data", {
+        "counter6": Mock(), **({"counter5": Mock()} if counter_exists else {}),
+    })
 
     subdata.process_consumer_topic(Mock(), consumers, msg)
 
@@ -187,8 +197,7 @@ def test_process_consumer_extra_meter_switches_linked_counter_in_hierarchy(monke
         "type": "counter",
         "children": [
             {"id": 2, "type": "consumer", "children": []},
-            {"id": 5, "type": "counter", "children": []},
-        ],
+        ] + ([{"id": 5, "type": "counter", "children": []}] if counter_exists else []),
     }]
     mock_pub.pub.assert_called_once_with("openWB/set/counter/get/hierarchy", counter_all_data.data.get.hierarchy)