Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update static UID fka functionality. #326

Merged
merged 11 commits into from
Jan 31, 2024
20 changes: 12 additions & 8 deletions docs/vspec2id.md
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -9,8 +9,9 @@ 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>] [--json-all-extended-attributes] [--json-pretty] [--yaml-all-extended-attributes] [-v version] [--all-idl-features]
[--gqlfield GQLFIELD GQLFIELD] [--jsonschema-all-extended-attributes] [--jsonschema-disallow-additional-properties] [--jsonschema-require-all-properties] [--jsonschema-pretty]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could potentially remove some fields that have no impact (and will not even be allowed after my vss-tools restructuring PR has been merged) like [--yaml-all-extended-attributes] and [--gqlfield GQLFIELD GQLFIELD]

[--validate-static-uid VALIDATE_STATIC_UID] [--only-validate-no-export] [--strict-mode]
<vspec_file> <output_file>

Convert vspec to other formats.
Expand All @@ -27,6 +28,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
Expand All @@ -43,6 +45,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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does case sensitive mean here? That two signals A.B and A.b get different UID so that renaming A.b to A.B is considered a name change?


### 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
Expand Down Expand Up @@ -78,11 +83,10 @@ 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']
Copy link
Collaborator

@erikbosch erikbosch Jan 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the order have any significance?

```

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.
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. 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:
Expand All @@ -93,14 +97,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) |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a stupid question but what does "fka" means

| 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.
Expand Down
83 changes: 60 additions & 23 deletions tests/vspec/test_static_uids/test_static_uids.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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")],
Expand All @@ -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()
Expand All @@ -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":
Expand All @@ -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:])

Expand All @@ -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
)
Expand Down
2 changes: 1 addition & 1 deletion tests/vspec/test_static_uids/test_vspecs/test.vspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/vspec/test_static_uids/test_vspecs/test_unit.vspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/vspec/test_static_uids/validation.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading
Loading