COVESA / vehicle_signal_specification

Vehicle Signal Specification - standardized way to describe automotive data
Mozilla Public License 2.0
320 stars 164 forks source link

Overlay childrens are not merged causing extensions_attributes missing #736

Closed DanielLazarHTDM closed 5 months ago

DanielLazarHTDM commented 5 months ago

Currently if my overlay contains following:

Vehicle.Cabin.Seat.Row1.DriverSide.Switch.IsForwardEngaged:
   datatype: boolean
   type: actuator
   dbc2vss:
      signal: VCLEFT_SeatDriverSwitchFwEngaged
   vss2dbc:
      signal: VCLEFT_SeatDriverSwitchFwEngaged
   dbc2name:
      name: SeatDemoFrame

and I do call:

vss-tools/vspec2json.py -e vss2dbc,dbc2vss,dbc2name -o dbc_overlay_seat_demo.vspec -u spec/units.yaml --json-pretty ./spec/VehicleSignalSpecification.vspec vss_dbc.json

final json file does not include extended data dbc2vss, vss2dbc, dbc2name. I assume that this should be considered as bug.

I did research and found root of this problem. In the file:

vss-tools/vspec/__init__.py
diff --git a/vspec/__init__.py b/vspec/__init__.py
index 2bf4de5..18f8d29 100755
--- a/vspec/__init__.py
+++ b/vspec/__init__.py
@@ -456,6 +456,13 @@ def expand_tree_instances(tree: VSSNode):
                     # instead merging it to the new node
                     existing_item.parent = None
                     expand_node.merge(existing_item)
+                    def merge_children(node1, node2):
+                        for c in node1.children:
+                            for cc in node2.children:
+                                if c.name == cc.name:
+                                    c.merge(cc)
+                                    merge_children(c, cc)
+                    merge_children(expand_node, existing_item)
                     break
             expand_node.parent = instantiated_branch
         return instantiated_branc

So I've put little recursion to merge also childrens resulting extended_attributes are correctly copied to final JSON file. Since I don't have deep knowledge about everything how things are generated could someone please validate it ?

erikbosch commented 5 months ago

I can try to reproduce it and check the solution. It seems a bit similar to my pull request in https://github.com/COVESA/vss-tools/pull/346 as both concern overlays for specific instances and the proposed fix mentioned here and my proposed fix touch the same place, but are somewhat different. I will tomorrow try to reproduce the problem in a small "test tree" like the test trees used in the PR referenced above and see if they both concern the same problem or if it is a different problem.

erikbosch commented 5 months ago

I have now finished my analysis, our "solutions" apparently covered different scenarios, your covered updating nodes "below" instances. mine covered adding node "below" instances. I have now updated my PR at https://github.com/COVESA/vss-tools/pull/346 to integrate both solutions and also added test cases which I think shall cover all scenarios.

Feel free to test it out.

DanielLazarHTDM commented 5 months ago

Thank you, it works as expected now.