From c8c8a2e227ee4844622e7a0717cd119fdf4d11c9 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Fri, 5 Apr 2024 16:26:34 +0200 Subject: [PATCH] Fix overlay merge error Fixes https://github.com/COVESA/vehicle_signal_specification/issues/736 Signed-off-by: Erik Jaegervall --- .../test_overlay_on_instance/expected.json | 149 ++++++++++++++++++ .../test_overlay_on_instance/overlay_1.vspec | 8 +- .../test_overlay_on_instance/overlay_2.vspec | 45 ++++++ .../vspec/test_overlay_on_instance/test.vspec | 33 ++++ vspec/__init__.py | 19 ++- 5 files changed, 251 insertions(+), 3 deletions(-) diff --git a/tests/vspec/test_overlay_on_instance/expected.json b/tests/vspec/test_overlay_on_instance/expected.json index 16821f01..955d68fe 100644 --- a/tests/vspec/test_overlay_on_instance/expected.json +++ b/tests/vspec/test_overlay_on_instance/expected.json @@ -202,6 +202,91 @@ "description": "Changed description, shall inherit comment", "type": "sensor", "unit": "km" + }, + "TT": { + "children": { + "Left": { + "children": { + "TTT": { + "children": { + "T4": { + "children": { + "TTTTR": { + "datatype": "float", + "description": "A very nasty sensor", + "type": "sensor" + } + }, + "description": "A very nasty branchzhs", + "type": "branch" + }, + "TTTT": { + "datatype": "float", + "description": "Some temperature.", + "my_id": "Make sure description and unit is kept", + "type": "sensor", + "unit": "celsius" + }, + "TTTY": { + "datatype": "float", + "description": "Auch some temperature.", + "my_id": "Extended attribute", + "type": "sensor", + "unit": "celsius" + } + }, + "description": "Another branch level", + "type": "branch" + } + }, + "description": "Second level instance", + "type": "branch" + }, + "Right": { + "children": { + "TTT": { + "children": { + "T4": { + "children": { + "TTTTR": { + "comment": "hjdsjksdkj", + "datatype": "float", + "description": "A very nasty sensor", + "type": "sensor" + } + }, + "description": "A very nasty branchzhs", + "type": "branch" + }, + "T5": { + "children": { + "TTTTZ": { + "datatype": "float", + "description": "Zome temperature.", + "type": "sensor", + "unit": "celsius" + } + }, + "description": "yet another branch", + "type": "branch" + }, + "TTTT": { + "datatype": "float", + "description": "Some temperature.", + "type": "sensor", + "unit": "celsius" + } + }, + "description": "Another branch level", + "type": "branch" + } + }, + "description": "Second level instance", + "type": "branch" + } + }, + "description": "Second level instance", + "type": "branch" } }, "description": "Branch with instances, not in list", @@ -227,6 +312,70 @@ "description": "Signal A.S.T", "type": "sensor", "unit": "km" + }, + "TT": { + "children": { + "Left": { + "children": { + "TTT": { + "children": { + "T4": { + "children": { + "TTTTR": { + "datatype": "float", + "description": "A very nasty sensor", + "type": "sensor" + } + }, + "description": "A very nasty branchzhs", + "type": "branch" + }, + "TTTT": { + "datatype": "float", + "description": "Some temperature.", + "type": "sensor", + "unit": "celsius" + } + }, + "description": "Another branch level", + "type": "branch" + } + }, + "description": "Second level instance", + "type": "branch" + }, + "Right": { + "children": { + "TTT": { + "children": { + "T4": { + "children": { + "TTTTR": { + "datatype": "float", + "description": "A very nasty sensor", + "type": "sensor" + } + }, + "description": "A very nasty branchzhs", + "type": "branch" + }, + "TTTT": { + "datatype": "float", + "description": "Some temperature.", + "type": "sensor", + "unit": "celsius" + } + }, + "description": "Another branch level", + "type": "branch" + } + }, + "description": "Second level instance", + "type": "branch" + } + }, + "description": "Second level instance", + "type": "branch" } }, "description": "Branch with instances, not in list", diff --git a/tests/vspec/test_overlay_on_instance/overlay_1.vspec b/tests/vspec/test_overlay_on_instance/overlay_1.vspec index b8a6aa3f..5cf16e70 100644 --- a/tests/vspec/test_overlay_on_instance/overlay_1.vspec +++ b/tests/vspec/test_overlay_on_instance/overlay_1.vspec @@ -1,4 +1,10 @@ - +# Copyright (c) 2023 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 # Add a third row A.B: diff --git a/tests/vspec/test_overlay_on_instance/overlay_2.vspec b/tests/vspec/test_overlay_on_instance/overlay_2.vspec index ad98f87f..aaa65d2d 100644 --- a/tests/vspec/test_overlay_on_instance/overlay_2.vspec +++ b/tests/vspec/test_overlay_on_instance/overlay_2.vspec @@ -1,3 +1,10 @@ +# Copyright (c) 2023 Contributors to COVESA +# +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 # And add a new signal only for left instance in second row A.B.Row2.Left.E: @@ -91,3 +98,41 @@ A.S.Up.T: type: sensor unit: km description: New description, shall not inherit anything + + +# Complex overlay - changing description for one of TTTT +A.S.Front.TT.Left.TTT.TTTT: + datatype: float + type: sensor + my_id: "Make sure description and unit is kept" + + +# A totally new one +A.S.Front.TT.Left.TTT.TTTY: + datatype: float + type: sensor + unit: celsius + description: Auch some temperature. + my_id: "Extended attribute" + + + + +# Just adding a comment deep down - check that two level branch (TTT.T4) after instantiation works + +A.S.Front.TT.Right.TTT.T4.TTTTR: + datatype: float + type: sensor + comment: "hjdsjksdkj" + +# And a totally new branch with a totally new signal + +A.S.Front.TT.Right.TTT.T5: + type: branch + description: yet another branch + +A.S.Front.TT.Right.TTT.T5.TTTTZ: + datatype: float + type: sensor + unit: celsius + description: Zome temperature. diff --git a/tests/vspec/test_overlay_on_instance/test.vspec b/tests/vspec/test_overlay_on_instance/test.vspec index 702fc755..e059df8b 100644 --- a/tests/vspec/test_overlay_on_instance/test.vspec +++ b/tests/vspec/test_overlay_on_instance/test.vspec @@ -1,4 +1,11 @@ +# Copyright (c) 2023 Contributors to COVESA # +# This program and the accompanying materials are made available under the +# terms of the Mozilla Public License 2.0 which is available at +# https://www.mozilla.org/en-US/MPL/2.0/ +# +# SPDX-License-Identifier: MPL-2.0 + A: type: branch description: Branch A. @@ -33,3 +40,29 @@ A.S.T: unit: km description: Signal A.S.T comment: Orig comment. + +# More complex example + +A.S.TT: + instances: ["Left","Right"] + type: branch + description: Second level instance + +A.S.TT.TTT: + type: branch + description: Another branch level + +A.S.TT.TTT.TTTT: + datatype: float + type: sensor + unit: celsius + description: Some temperature. + +A.S.TT.TTT.T4: + type: branch + description: "A very nasty branchzhs" + +A.S.TT.TTT.T4.TTTTR: + datatype: float + type: sensor + description: "A very nasty sensor" diff --git a/vspec/__init__.py b/vspec/__init__.py index 2bf4de5d..324fb4bd 100755 --- a/vspec/__init__.py +++ b/vspec/__init__.py @@ -454,8 +454,23 @@ def create_instantiated_branch(branch_name, parent, nodes_to_expand): # shall have precedence over the expanded instance # This is handled by removing the old node from tree and # instead merging it to the new node - existing_item.parent = None expand_node.merge(existing_item) + # Also make sure that any children already existing are moved or merged to the new tree + + def merge_children(target_node, input_node): + for existing_child in input_node.children: + found = False + for expanded_child in target_node.children: + if expanded_child.name == existing_child.name: + expanded_child.merge(existing_child) + merge_children(expanded_child, existing_child) + found = True + if not found: + # Totally new node/branch, no need to continue deeper, just take it + existing_child.parent = target_node + input_node.parent = None + + merge_children(expand_node, existing_item) break expand_node.parent = instantiated_branch return instantiated_branch @@ -828,7 +843,6 @@ def render_subtree( def merge_elem(base, overlay_element): r = Resolver() element_name = "/" + overlay_element.qualified_name("/") - if not VSSNode.node_exists(base, element_name): # The node in the overlay does not exist, so we connect it # print(f"Not exists {overlay_element.qualified_name()} does not exist, creating.") @@ -843,6 +857,7 @@ def merge_elem(base, overlay_element): other_node: VSSNode = r.get(base, element_name) try: other_node.merge(overlay_element) + except ImpossibleMergeException as e: logging.error(f"Merging impossible: {e}") sys.exit(-1)