Improve naming convention to distinguish instances from branches #749
Comments
|
I see this as a little bit related to #642 and the Left/Right vs DriverSide/PassengerSide discussion. If one see the part in square brackets as an instance labels, then one could possibly allow aliases, so that the two lines below may point to the same physical instance (for a LHD vehicle) If we would go this way we need to discuss how this should affect the released artifacts produced by VSS-tool (as can be seen in https://github.com/COVESA/vehicle_signal_specification/releases/tag/v4.2) Today most expand the tree. But if we change approach we should possibly not expand as today, as for example "Row1" should not be a branch itself. |
|
MoM:
|
|
This will add complexity to the generation and parsing of paths. |
I think one need to separate different aspects, like:
Taking the VSS-tools JSON format as example, there we do not use expanded names but instead have a tree where currently Row1 is one branch and DriverSide a branch under Row1. With the proposal in this issue we could keep that design as is, possibly by adding a branch-type But in other outputs (like Yaml and CSV) we use expanded names and then just adding a metadata on the branch "line" likely does not give much information to those that look at individual signals. I do not think the work in vss-tools to change according to any of the proposal from Daniel would be that big, but we need to consider each output, as maybe not all of them supports square brackets as part of names/identifiers. Some may need or want to keep the old dot notation for instances. And it could also be a variation point. Some programmatic APIs based on VSS already today use instances differently compared to other branches, with methods like |
|
Bear in mind that the suggestion of square brackets The principal point is to make an explicit distinction between the abstract entity that can be instantiated (e.g., a I like the ideas written by @erikbosch of either:
|
|
I think that the gain/pain it applies to sets of use cases should be considered. A major use case set for VISS are the telematics use cases. For these to have it explicitly expressed in the path I believe is more of a pain than a gain (but expressed in the node metadata would be ok). |
|
MoM: Daniel, create an example. How this should be documented in vspec. |
|
Here are a couple of examples: Simple scenario
CurrentvspecVehicle.Cabin.Door:
type: branch
instances:
- Row[1,2]
- ["DriverSide","PassengerSide"]
Vehicle.Cabin.Door.Window:
type: branch
Vehicle.Cabin.Door.Window.Position:
datatype: uint8
type: actuator
min: 0
max: 100
unit: percent
description: Item position. 0 = Start position 100 = End position.
comment: Relationship between Open/Close and Start/End position is item dependent.yaml exportVehicle.Cabin.Door.Row1.PassengerSide.Window.Position:
comment: Relationship between Open/Close and Start/End position is item dependent.
datatype: uint8
description: Item position. 0 = Start position 100 = End position.
max: 100
min: 0
type: actuator
unit: percent"Door": {
"children": {
"Row1": {
"children": {
"PassengerSide": {
"children": {
"Window": {
"children": {
"Position": {
"comment": "Relationship between Open/Close and Start/End position is item dependent.",
"datatype": "uint8",
"description": "Item position. 0 = Start position 100 = End position.",
"max": 100,
"min": 0,
"type": "actuator",
"unit": "percent"
},
},
"description": "Door window status. Start position for Window is Closed.",
"type": "branch"
}
etc...ProposedvspecVehicle.Cabin.Door[i]:
type: instance_branch
instances:
- Row[1,2]
- ["DriverSide","PassengerSide"]
Vehicle.Cabin.Door[i].Window:
type: branch
Vehicle.Cabin.Door[i].Window.Position:
datatype: uint8
type: actuator
min: 0
max: 100
unit: percent
description: Item position. 0 = Start position 100 = End position.
comment: Relationship between Open/Close and Start/End position is item dependent.yaml exportVehicle.Cabin.Door[Row1.PassengerSide].Window.Position:
comment: Relationship between Open/Close and Start/End position is item dependent.
datatype: uint8
description: Item position. 0 = Start position 100 = End position.
max: 100
min: 0
type: actuator
unit: percentjson export"Door": {
"Row1.DriverSide": {
"Position": {
"comment": "Relationship between Open/Close and Start/End position is item dependent.",
"datatype": "uint8",
"description": "Item position. 0 = Start position 100 = End position.",
"max": 100,
"min": 0,
"type": "actuator",
"unit": "percent"
},
},
"description": "Door window status. Start position for Window is Closed.",
"type": "branch"
}
etc...Complex scenarioIn case of instantiations in multiple branches, we might have something like: Current (hypothetical)vspecVehicle.Cabin.Door:
type: branch
instances:
- Row[1,2]
- ["DriverSide","PassengerSide"]
Vehicle.Cabin.Door.Speaker:
type: branch
instances: ["Upper","Middle","Lower"]
Vehicle.Cabin.Door.Speaker.Volume:
type: actuator
datatype: int
min: 0
max: 100
description: The volume of a particular speaker that is embedded in the door.yaml exportVehicle.Cabin.Door.Row1.PassengerSide.Speaker.Upper.Volume:
type: actuator
datatype: int
min: 0
max: 100
description: The volume of a particular speaker that is embedded in the door.ProposedvspecVehicle.Cabin.Door[i]:
type: instance_branch
instances:
- Row[1,2]
- ["DriverSide","PassengerSide"]
Vehicle.Cabin.Door[i].Speaker[i]:
type: instance_branch
instances: ["Upper","Middle","Lower"]
Vehicle.Cabin.Door[i].Speaker[i].Volume:
type: actuator
datatype: int
min: 0
max: 100
description: The volume of a particular speaker that is embedded in the door.yaml exportVehicle.Cabin.Door[Row1.PassengerSide].Speaker[Upper].Volume:
type: actuator
datatype: int
min: 0
max: 100
description: The volume of a particular speaker that is embedded in the door.NoteThe symbols or characters to use can be discussed and agreed upon. For example, the characters |
|
MoM:
|
|
The VISS WG does not support the proposed syntax changes to the paths. |
|
It would be helpful for the discussion if some usecases that benefit from the proposal were presented. |
One example is overlay handling if you want to change data for a specific instance. Today VSS-tools if it find Similarly if you need to convert a full path to a programmatic API call. like |
|
It seems from the examples by @erikbosch that what this change provides is saving one or two rounds in an iteration loop.
If that identifier is the node type as mentioned above, then the cost to achieve the gain could be acceptable, but not if the solution is to change the path syntax. |
|
My 2 cents, and I think the main aspects here are "output" (i.e. yaml/json) and vspec changes Output
I think the square brackets are reasonable, I would also assume a branch to be marked "ibranch" (or otherwis tagged as instance branch) As downstream user I am mainly looking from KUKSA perspective which currently (in core and API) is completely "instance-unanaware", I assume this is a bit similar to VISS(R). However I feel, I could still "parse" the bracket syntax and still just not expose instances to users. It might be wise in any case to also allow the current format to be generated in exporters as well (I think it is easy to support), and the question would rather be if/when a change becomes the "default" VSPECThe given example was
BenefitsAlso since VISS brought it up: as I said "KUKSA-side" we are currently unaware of instances. However, it has always bothered me a bit that "instances" is a vspec thing, but that information is "lost" in our exports. Also, I currently do not see a big effort using/converting the "new" format in instance-unaware systems, however I did not take part in the VISS WG discussion, so I may not see all complications |
|
I thought a little about this issue during the weekend. I see (at least) two parts of the proposal. We can take the hypothetical example from Daniel above as starting point: Today, if generating for example CSV we get a lot of branches like this: We have some objection on changing the "expanded path identifiers" to something like Daniel propose to use the term But is it really needed there? Is it not more important to use it in the model after expansion to show that a branch comes from instantiation. In original *.vspec you still have the And in JSON a tree structure like This change (keywords and flattened structure) could be implemented separately from the "full path change". So far no-one has objected to the this part of the proposal. So is the way forward to give "ok" to the keyword/flattened-change, but do not change default full-path syntax. If the changed full path syntax would be needed for someone for some exporter (like CSV/Yaml) we could introduce it as an alternative output format, like |
|
About the keyword: About the resulting path naming convention: |
To inroduce the new keyword is fine. |
|
IF the flattening should also be part of this, then the dot (.) between Row1 and DriverSide should be replaced by another character, e. g. dash (-), to become Row1-DriverSide. Dash should then only be allowed in node names when used for this separation. A parser could then treat it like a single node name without any further analysis. |
Well, if you are to generate full path from the tree structure and you are to keep the current dot-pattern you would need to translate it back to |
|
You previously stated
The path Vehicle.Cabin.Door.Row1.DriverSide is then in the tree represented by three branch nodes: Vehicle, Cabin, Door, and one instance_branch node: Row1.DriverSide Not doing that leads to a more complex parsing of path names. It also opens for unclarities, e.g. what does the path Vehicle.Cabin.Door.Row1 point to? |
I think the expanded version here is a bit confusing to me, I would have expected So this would in my opinion be less confusing when in the (flattened/expanded?) version somehting lie
or
would be supported. That seems more clear to me. (And I think there is no "harm" in still supporting the "old" flavor" of
as well, maybe even without "marking" different branches at all. |
|
Based on the previous comments, let me summarize the proposal as follows: Change 1 - Introduce the keyword
|
|
I think the information about instantiation should be represented as metadata in the nodes, and not being visible in the path expressions. With this available in the tree there is no need to introduce either the instance_branch node type or modifying the paths. |
|
MoM:
|
|
Just created the linked PR for the information keeping of what branches have been created from the Whether a branch has created from instances or explicitly written by the user should not be reflected in the type of the node since they will not differ. |
|
I think the second part of this issue should be solved by e.g. an option in exporters. |
|
I think COVESA/vss-tools#399 solves the entire issue, which with the referenced PR merged can be closed. |
The current property naming convention can cause confusion when several branches exist, as it uses the same dotted notation for both branches and instances.
Simple scenario
For example, the property name
Vehicle.Cabin.Door.Row1.DriverSide.Window.Positiondoes not clearly differentiate between theRow1.DriverSidebeing an instance of theDoorfeature of interest, versus a branch in the property hierarchy.To address this issue and improve the clarity of the property naming convention, the following two alternatives are proposed:
Vehicle.Cabin.Door[Row1.DriverSide].Window.Position. In this approach, a set of distinct symbols, such as square brackets[ ], are used to enclose the instance information, making it visually distinct from the branch names. The specific symbols used (e.g.,[ ],{ },- -etc.) can be determined based on team preference and overall consistency.Vehicle.Cabin.Door.Instance[Row1.DriverSide].Window.PositionIn this approach, a separate identifier, such as the keywordInstance, is used to explicitly indicate that the following information represents the instance of the feature of interestDoor. The instance information is then enclosed in a set of distinct symbols, similar to the first solution.The goal of these proposals is to maintain the simplicity of the dot notation for branch names, while providing a clear visual distinction for the instance information. This will improve the overall readability and understanding of the property naming convention used in the codebase.
Complex scenario
What about cases when multiple branches are instantiated to specify the property of a feature of interest? For example, a vehicle can have multiple instances of a
Door(e.g., Rows and sides), and each one can also have multiple instances ofSpeaker(e.g., upper, middle, low, etc.). If we follow the current approach, we would end up with something like:Vehicle.Cabin.Door.Row1.DriverSide.Speaker.Upper.Volume. With the proposed approaches, we could have something nicer and easier to handle like:Vehicle.Cabin.Door[Row1.DriverSide]Speaker[Upper].VolumeVehicle.Cabin.Door.Instance[Row1.DriverSide].Speaker.Instance[Upper].VolumeOther examples can be, multiple buttons in a particular component, multiple lights in a particular position inside the cabin, multiple USB ports or button embedded in a seat, etc.
Any opinion or additional suggestion?
The text was updated successfully, but these errors were encountered: