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 documentation #716

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Update documentation #716

merged 1 commit into from
Feb 20, 2024

Conversation

erikbosch
Copy link
Collaborator

@erikbosch erikbosch commented Feb 8, 2024

  • Clarifies string/float/double.
  • Reformat recommendations to state that they also are rules for VSS catalog
  • Remove enum recommendation

Proposed follow-up actions if PR accepted.

  • Remove enum name check from vss-tools (So that for example no warning is given for Vehicle.OBD.PidsA)
  • Add a "case insensitive uniqeness check" for VSS name so it give error in strict mode if you have both Vehicle.Cabin.Abc and Vehicle.Cabin.ABC
  • When done adapt Makefile used in CI so that we enforce recommendations for the standard catalog.

Fixes #705
Fixes #706
Partially #700 (but more needed to complete it)
Fixes #662
Fixes #280

@erikbosch erikbosch marked this pull request as ready for review February 9, 2024 09:20
@erikbosch erikbosch added Scope:Minor A change that is not major and not trivial. Status:New An issue/PR that not yet have been discussed/announced at a VSS meeting Status:Meeting Intended to be discussed at next VSS-project meeting labels Feb 13, 2024
Vehicle.Cabin.Door.Row1.Left.IsLocked
SomeOtherTree.AnotherBranch.MySignalName
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure for what is this an example, doesn't it state above every branch need to start with "Vehicle"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the VSS standard catalog the root name is Vehicle, but you may have a different tree using VSS syntax and then you can select whatever root name you want. Like if you use HIM to specify that multiple trees shall be used. I tried to make it more visible by renaming it from SomeBranch to SomeOtherTree. But that is sort of a special case so I am considering just removing this line.

@@ -57,6 +62,8 @@ The time and date format shall be as shown below, where the sub-second data and
YYYY-MM-DDTHH:MM:SS.ssssssZ
```
VSS also supports the unit `unix-time` that can be used together with datatype `uint32`/`uint64`/`int64`to indicate that the signal value correspond to number of non-leap seconds which have passed since 00:00:00 UTC on Thursday, 1 January 1970.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this relevant here, "unix-time" being a unit (not a datatype), and we are also not explicitly talking about all the other units in the standard catalogue unit file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But with that argument we could remove the whole Timestamps section from data_types.md as we do not have any specific "timestamp" datatype. I think it makes sense to somewhere describe the two methods we support for reporting timestamps, but maybe it fits better in the unit section of documentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will refactor #717 a bit and then we can remove it here

@erikbosch
Copy link
Collaborator Author

MoM: Please review

* Boolean signals must start with `Is`.
* Full path of node names must be unique also when using case insensitive comparison.
It is for example not allowed to have both `Vehicle.Abc` and `Vehicle.ABC` in the standard catalog
even if that is valid VSS as VSS is case sensitive.
Copy link
Contributor

Choose a reason for hiding this comment

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

Lines 121 to 123 are confusing. Can we shorten it to something like:
Node names are case insensitive for comparison operations, therefore the full path of node names must be unique e.g. Vehicle.Abc and Vehicle.ABC are the same in comparison and therefore the presence of both is prohibited.

Clarifies string/float/double.
Reformat recommendatations to state that they also are rules for VSS catalog
Remove enum recommendation

Fixes COVESA#705
Fixes COVESA#706
Partially solves COVESA#700
Fixes COVESA#662

Signed-off-by: Erik Jaegervall <[email protected]>
@erikbosch
Copy link
Collaborator Author

PR updated to address comments

@erikbosch erikbosch removed the Status:New An issue/PR that not yet have been discussed/announced at a VSS meeting label Feb 20, 2024
@erikbosch
Copy link
Collaborator Author

MoM: Merge

@erikbosch erikbosch merged commit 93550c3 into COVESA:master Feb 20, 2024
4 checks passed
@erikbosch erikbosch deleted the erik_doc branch February 20, 2024 16:13
erikbosch added a commit to boschglobal/vss-tools that referenced this pull request Feb 21, 2024
Aligning messages to what is stated in documentation after
COVESA/vehicle_signal_specification#716

Signed-off-by: Erik Jaegervall <[email protected]>
erikbosch added a commit to boschglobal/vss-tools that referenced this pull request Feb 21, 2024
Aligning messages to what is stated in documentation after
COVESA/vehicle_signal_specification#716

Signed-off-by: Erik Jaegervall <[email protected]>
erikbosch added a commit to COVESA/vss-tools that referenced this pull request Mar 11, 2024
Aligning messages to what is stated in documentation after
COVESA/vehicle_signal_specification#716

Signed-off-by: Erik Jaegervall <[email protected]>
nwesem pushed a commit to nwesem/vss-tools that referenced this pull request Apr 4, 2024
Aligning messages to what is stated in documentation after
COVESA/vehicle_signal_specification#716

Signed-off-by: Erik Jaegervall <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope:Minor A change that is not major and not trivial. Status:Meeting Intended to be discussed at next VSS-project meeting
Projects
None yet
3 participants