Update static UID fka functionality. (#326)

* add strict mode for case sensitivity Signed-off-by: Niclas Wesemann <[email protected]> Signed-off-by: Adnan Bekan <[email protected]>
COVESA · May 3, 2024 · 1708ddd · 1708ddd
1 parent ccbba1f
commit 1708ddd
Show file tree

Hide file tree

Showing 18 changed files with 242 additions and 81 deletions.
diff --git a/docs/vspec2id.md b/docs/vspec2id.md
@@ -1,6 +1,6 @@
 # vspec2id - vspec static UID generator and validator
 
-The vspecID.py script is used to generate and validate static UIDs for all nodes in the tree.
+The vspec2id.py script is used to generate and validate static UIDs for all nodes in the tree.
 They will be used as unique identifiers to transmit data between nodes. The static UIDs are
 implemented to replace long strings like `Vehicle.Body.Lights.DirectionIndicator.Right.IsSignaling`
 with a 4-byte identifier.
@@ -9,8 +9,8 @@ with a 4-byte identifier.
 
 ```bash
 usage: vspec2id.py [-h] [-I dir] [-e EXTENDED_ATTRIBUTES] [-s] [--abort-on-unknown-attribute] [--abort-on-name-style] [--format format] [--uuid] [--no-expand] [-o overlays] [-u unit_file]
-                   [-vt vspec_types_file] [-ot <types_output_file>] [--json-all-extended-attributes] [--json-pretty] [--yaml-all-extended-attributes] [-v version] [--all-idl-features]
-                   [--gqlfield GQLFIELD GQLFIELD] [--validate-static-uid VALIDATE_STATIC_UID] [--only-validate-no-export]
+                   [-q quantity_file] [-vt vspec_types_file] [-ot <types_output_file>] [--yaml-all-extended-attributes] [-v version] [--all-idl-features]
+                   [--validate-static-uid VALIDATE_STATIC_UID] [--only-validate-no-export] [--strict-mode]
                    <vspec_file> <output_file>
 
 Convert vspec to other formats.
@@ -27,6 +27,7 @@ IDGEN arguments:
                         Path to validation file.
   --only-validate-no-export
                         For pytests and pipelines you can skip the export of the <output_file>
+  --strict-mode         Strict mode means that the generation of static UIDs is case-sensitive.
 ```
 
 ## Example
@@ -43,6 +44,9 @@ cd path/to/your/vss-tools
 Great, you generated your first overlay that will also be used as your validation file as soon as you update your
 vehicle signal specification file.
 
+If needed you can make the static UID generation case-sensitive using the command line argument `--strict-mode`. It
+will default to false.
+
 ### Generate e.g. yaml file with static UIDs
 
 Now if you just want to generate a new e.g. yaml file including your static UIDs, please use the overlay function of
@@ -78,11 +82,28 @@ A.B.NewName:
   type: actuator
   allowed: ["YES", "NO"]
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
+```
+or
+```
+A.B.NewName:
+  datatype: string
+  type: actuator
+  allowed: ["YES", "NO"]
+  description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
+  fka: A.B.OlderName
 ```
 
-As stated if you want to rename the node `A.B.NewName` to `A.NewName` you can also write the `fka` attribute
-stating its legacy path.
+In order to add fka attribute, one can add fka directly into the vspec file or use overlay feature of vss-tools.
+
+Example mycustom-overlay-fka.vspec
+```
+A.B.NewName:
+  datatype: string
+  type: actuator
+  fka: A.B.OlderName
+```
+As stated if you want to rename the node `A.B.NewName` to `A.NewName` you can also write the Formerly Known As `fka` attribute stating its legacy path. For hashing function in previous case `A.B.OlderName` will be used.
 
 To summarize these are the `BREAKING CHANGES` that affect the hash and `NON-BREAKING CHANGES` that throw
 warnings only:
@@ -93,14 +114,14 @@ warnings only:
 | Data type             |     | Deprecation          |
 | Type (i.e. node type) |     | Deleted Attribute    |
 | Unit                  |     | Change description   |
-| Enum values (allowed) |     |                      |
+| Enum values (allowed) |     | Qualified name (fka) |
 | Minimum               |     |                      |
 | Maximum               |     |                      |
 
 Now you should know about all possible changes. To run the validation step, please do:
 
 ```bash
-./vspecID.py ../vehicle_signal_specification/spec/VehicleSignalSpecification.vspec ../output_id_v2.vspec --validate-static-uid ../output_id_v1.vspec
+./vspec2id.py ../vehicle_signal_specification/spec/VehicleSignalSpecification.vspec ../output_id_v2.vspec --validate-static-uid ../output_id_v1.vspec
 ```
 
 Depending on what you changed in the vehicle signal specification the corresponding errors will be triggered.

diff --git a/tests/vspec/test_static_uids/test_static_uids.py b/tests/vspec/test_static_uids/test_static_uids.py
@@ -11,19 +11,19 @@
 #
 
 import os
-import pytest
 import shlex
-import vspec
-import vspec.vssexporters.vss2id as vss2id
-import vspec2x
+from typing import Dict
+
+import pytest
 import yaml
 
-from typing import Dict
-from vspec.model.constants import VSSTreeType, VSSDataType, VSSUnit
+import vspec
+import vspec2x
+import vspec.vssexporters.vss2id as vss2id
+from vspec.model.constants import VSSDataType, VSSTreeType, VSSUnit
 from vspec.model.vsstree import VSSNode
 from vspec.utils.idgen_utils import get_all_keys_values
 
-
 # HELPERS
 
 
@@ -100,11 +100,51 @@ def test_generate_id(
     result_static_uid: str,
 ):
     node = get_test_node(node_name, unit, datatype, allowed, minimum, maximum)
-    result, _ = vss2id.generate_split_id(node, id_counter=0)
+    result, _ = vss2id.generate_split_id(node, id_counter=0, strict_mode=False)
 
     assert result == result_static_uid
 
 
+@pytest.mark.parametrize(
+    "node_name_case_sensitive, node_name_case_insensitive, strict_mode",
+    [
+        ("TestNode", "testnode", False),
+        ("TestNode", "testnode", True),
+    ],
+)
+def test_strict_mode(
+    node_name_case_sensitive: str, node_name_case_insensitive: str, strict_mode: bool
+):
+    node_case_sensitive = get_test_node(
+        node_name=node_name_case_sensitive,
+        unit="m",
+        datatype=VSSDataType.FLOAT,
+        allowed="",
+        minimum="",
+        maximum="",
+    )
+    result_case_sensitive, _ = vss2id.generate_split_id(
+        node_case_sensitive, id_counter=0, strict_mode=strict_mode
+    )
+
+    node_case_insensitive = get_test_node(
+        node_name=node_name_case_insensitive,
+        unit="m",
+        datatype=VSSDataType.FLOAT,
+        allowed="",
+        minimum="",
+        maximum="",
+    )
+    result_case_insensitive, _ = vss2id.generate_split_id(
+        node_case_insensitive, id_counter=0, strict_mode=strict_mode
+    )
+
+    if strict_mode:
+        assert result_case_sensitive != result_case_insensitive
+    else:
+        assert result_case_sensitive == result_case_insensitive
+
+
 @pytest.mark.parametrize(
     "test_file, validation_file",
     [("test_vspecs/test.vspec", "./validation.yaml")],
@@ -122,10 +162,10 @@ def test_export_node(
         vspec_file, include_paths=["."], tree_type=VSSTreeType.SIGNAL_TREE
     )
     yaml_dict: Dict[str, str] = {}
-    vss2id.export_node(yaml_dict, tree, id_counter=0)
+    vss2id.export_node(yaml_dict, tree, id_counter=0, strict_mode=False)
 
     result_dict: Dict[str, str]
-    with open(os.path.join(dir_path, validation_file), "r") as f:
+    with open(os.path.join(dir_path, validation_file)) as f:
         result_dict = yaml.load(f, Loader=yaml.FullLoader)
 
     assert result_dict.keys() == yaml_dict.keys()
@@ -149,23 +189,15 @@ def test_duplicate_hash(caplog: pytest.LogCaptureFixture, children_names: list):
     if children_names[0] == children_names[1]:
         # assert system exit and log
         with pytest.raises(SystemExit) as pytest_wrapped_e:
-            vss2id.export_node(
-                yaml_dict,
-                tree,
-                id_counter=0,
-            )
+            vss2id.export_node(yaml_dict, tree, id_counter=0, strict_mode=False)
         assert pytest_wrapped_e.type == SystemExit
         assert pytest_wrapped_e.value.code == -1
         assert len(caplog.records) == 1 and all(
             log.levelname == "CRITICAL" for log in caplog.records
         )
     else:
         # assert all IDs different
-        vss2id.export_node(
-            yaml_dict,
-            tree,
-            id_counter=0,
-        )
+        vss2id.export_node(yaml_dict, tree, id_counter=0, strict_mode=False)
         assigned_ids: list = []
         for key, value in get_all_keys_values(yaml_dict):
             if not isinstance(value, dict) and key == "staticUID":
@@ -186,8 +218,14 @@ def test_full_script(caplog: pytest.LogCaptureFixture):
 
 
 @pytest.mark.usefixtures("change_test_dir")
-def test_semantic(caplog: pytest.LogCaptureFixture):
-    validation_file: str = "./validation_vspecs/validation_semantic_change.vspec"
+@pytest.mark.parametrize(
+    "validation_file",
+    [
+        "./validation_vspecs/validation_semantic_change_1.vspec",
+        "./validation_vspecs/validation_semantic_change_2.vspec",
+    ],
+)
+def test_semantic(caplog: pytest.LogCaptureFixture, validation_file: str):
     clas = shlex.split(get_cla_validation(validation_file))
     vspec2x.main(["--format", "idgen"] + clas[1:])
 
@@ -203,7 +241,6 @@ def test_vss_path(caplog: pytest.LogCaptureFixture):
     test_file: str = "./test_vspecs/test_vss_path.vspec"
     clas = shlex.split(get_cla_test(test_file))
     vspec2x.main(["--format", "idgen"] + clas[1:])
-
     assert len(caplog.records) == 1 and all(
         log.levelname == "WARNING" for log in caplog.records
     )

diff --git a/tests/vspec/test_static_uids/test_vspecs/test.vspec b/tests/vspec/test_static_uids/test_vspecs/test.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_added_attribute.vspec b/tests/vspec/test_static_uids/test_vspecs/test_added_attribute.vspec
@@ -40,7 +40,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_datatype.vspec b/tests/vspec/test_static_uids/test_vspecs/test_datatype.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_deleted_attribute.vspec b/tests/vspec/test_static_uids/test_vspecs/test_deleted_attribute.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_deprecation.vspec b/tests/vspec/test_static_uids/test_vspecs/test_deprecation.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_description.vspec b/tests/vspec/test_static_uids/test_vspecs/test_description.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_name_datatype.vspec b/tests/vspec/test_static_uids/test_vspecs/test_name_datatype.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_unit.vspec b/tests/vspec/test_static_uids/test_vspecs/test_unit.vspec
@@ -41,7 +41,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/test_vspecs/test_vss_path.vspec b/tests/vspec/test_static_uids/test_vspecs/test_vss_path.vspec
@@ -42,7 +42,7 @@ A.B.NewName:
   type: sensor
   unit: mm
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
-  fka: ['A.B.OldName', 'A.B.OlderName']
+  fka: ['A.B.OlderName', 'A.B.OldName']
 A.B.IsLeaf:
   datatype: string
   type: actuator

diff --git a/tests/vspec/test_static_uids/validation.yaml b/tests/vspec/test_static_uids/validation.yaml
@@ -38,9 +38,9 @@ A.B.NewName:
   datatype: uint32
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
   fka:
-  - A.B.OldName
   - A.B.OlderName
-  staticUID: '0x126503B6'
+  - A.B.OldName
+  staticUID: '0x3FB9A236'
   type: sensor
   unit: mm
 A.Float:

diff --git a/tests/vspec/test_static_uids/validation_vspecs/validation.vspec b/tests/vspec/test_static_uids/validation_vspecs/validation.vspec
@@ -46,9 +46,9 @@ A.B.NewName:
   datatype: uint32
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
   fka:
-  - A.B.OldName
   - A.B.OlderName
-  staticUID: '0x126503B6'
+  - A.B.OldName
+  staticUID: '0x3FB9A236'
   type: sensor
   unit: mm
 A.Float:

diff --git a/...n_vspecs/validation_semantic_change.vspec → ...vspecs/validation_semantic_change_1.vspec b/...n_vspecs/validation_semantic_change.vspec → ...vspecs/validation_semantic_change_1.vspec
@@ -47,7 +47,7 @@ A.B.OldName:
   description: A.B.NewName's old name is 'OldName'. And its even older name is 'OlderName'.
   fka:
   - A.B.OlderName
-  staticUID: '0x8B7DE17D'
+  staticUID: '0x3FB9A236'
   type: sensor
   unit: mm
 A.Float: