Decoupling instance handling from model - #919
Conversation
Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
|
We will seriously risk interoperability if we leave it as a recommendation. The existing instances should remain, the documentation should describe how the cardinality of instances can be updated using overlays. |
|
Maybe the issue here ist just wording? Technically (and usability-wise) I would love to finally split out instances (as it would make it more clear you are not "breaking" VSS law per se if you need to add another row of seats or remove one. At the same time it should not be be/feel like "oh look you could do instances here is an example but we do not care" (like the motorbike overlay currently, that neither is nor do we feel that it is in any way normative). However I my point of view a good way forward is
That way I think it is more clean technically without diminishing the current normative (soft :))power of VSS |
|
As Ulf states, if we introduce this it would be a major change as backward compatibility for users may be broken. But how much users are affected would depend on how they use VSS
If they use a fork of VSS, already has overlays or similar things where they for example change instances things might get a bit complicated, as order of overlays then would matter, and there might be corner cases I have not thought of yet. We may add a mandory instance file argument. But if so we may need to accept that it is empty as there might be models were no instances are used. The documentation in https://covesa.github.io/vehicle_signal_specification/extensions/overlay/index.html mention instances, so it is already possible to change instance handling with overlays. This PR focus more on whether we actually should define number of wheels, number of seats and so on in std catalog or not. Some of the older issues propose introduction of "instance pattern". That could be an option if we from a model perspective wants to dictate how for example seat instances shall be named, but not exactly which shall exist. |
|
MoM:
|
|
Some partially unrelated topics if/when we are discussing "VSS instances v2.0"
|
|
MoM:
|
|
I think there are some limitations in the "instances" concept that might be overcome when we allow arrays of branches. A new attribute Example: Vehicle.Chassis.Axle:
type: branch
description: Axle Signals
elements: Axle[1,2]
Vehicle.Chassis.Axle.Wheel:
type: branch
description: Signals of a distinct axle
elements: ["Left","Right"]which is expanded to There are two main differences compared to instances:
Putting the new elements on the same level offers the possibility to describe trees like which is currently not possible with instances. With instances it is also not possible to put items (attribute/sensor/actuator/branch) on every level. Using instances like done in Expressing the same tree with Vehicle.Cabin.Door.Row:
type: branch
description: High-level cabin door row data.
elements: Row[1,2]
Vehicle.Cabin.Door.RowCount:
type: attribute
instantiate: false
description: Number of doors
datatype: uint8
Vehicle.Cabin.Door.Row.Side:
type: branch
description: High-level cabin door row data.
elements: ["DriverSide","PassengerSide"]
Vehicle.Cabin.Door.Row.Side.SideCount:
type: attribute
instantiate: false
description: Number of sides the door can be on
datatype: uint8It needs further discussions whether the elements should be limited to numeric, like I think with this approach we can express the "numeric instances" in a more elegant way (might be useful for Jasper), have the flexibility to add items anywhere in the path and allow shorter paths in the end. There should be no impact regarding tooling as long as the expanded model is used. I already wrote a prototype of the vss-tools to support When we introduce rules how the |
|
The proposal by @wiedmama solves the issues with the current instances concept. I support this proposal with the comments: |
|
@wiedmama We had a very similar discussion on this topic some time ago, and I am trying to identify the issue / pr. It was very long chat, myb @erikbosch or @SebastianSchildt can help. In any case, if you want to proceed with your proposed solution, you still need to define the "array" first and then work with its key elements or instances. This is precisely the purpose of using instances they provide a structured way to access and manage those elements. With instances you can do quite some of these things, you can generate similar setup. Then if i am implementing API to grab all Axles i would be able to do it with Vehicle.Chassis.Axles.* here is docu if you want to do what you are describing, go into actual upper level branch with direct tags, then i would suggest include. Because what you are referring to, it is not an array. also if you want to attach additional attributes, e.g. Axle 1 has different attributes e.g. some extra wheels on Axle2, or simply attribute is needed in the top branch, you can do that as well via overlay Cheers, |
@wiedmama Vehicle:
type: branch
description: V
instances:
- Implement[1,2]
Vehicle.Foo:
type: branch
description: Foo
instantiate: false
Vehicle.Bar:
type: branch
description: Foo
Vehicle.Implement2.Boom1:
type: branch
description: Boom1
Vehicle.Implement1.Tank1:
type: branch
description: Tank1
Vehicle.Implement2.Boom1.Section3:
type: branch
description: Section3
|
|
@sschleemilch : was a little bit too compact. I tried to outline a tree where you can have e.g. 1..3 implements, each having 1..2 booms and 1..2 tanks on implement level and e.g. 1..32 sections on boom level. Is this possible with instances? I failed trying to do so. Writing this I also had in mind a mechanism where the actual number of implements, booms, tanks, etc. might be overwritten or set by another mechanism like discussed already above. So writing each element manually is not an option. |
Vehicle:
type: branch
description: d
Vehicle.Door:
type: branch
description: d
instances:
- Row[1,2]
- [DriverSide, PassengerSide]If I understood correctly, you would like to attach something to all Row* instances without naming all of them explicitly? Vehicle:
type: branch
description: d
Vehicle.Door:
type: branch
description: d
Vehicle.Door.Row:
type: branch
description: d
instances:
- [1,2]
Vehicle.Door.Row.Count:
type: attribute
description: d
datatype: uint8
instantiate: false
Vehicle.Door.Row.Side:
type: branch
description: d
instances:
- [Driver,Passenger]
Vehicle.Door.Row.Side.Count:
type: attribute
description: d
datatype: uint8
instantiate: falseI fear that having elements and instances at the same time might be a bit much to grasp the differences. Is that the core of the limitation you see? |
|
The elements concept main difference to the instances concept is as I see it that it does not add new levels of nodes to the branch, it modifies the node level where it is declared. This I see as an improvement that makes it worth replacing the instances concept with it. Even if both concepts can achieve the same results, it is expressed in a more understandable syntax with the elements concept. |
|
Ulf is right: the difference is where the new items are placed. With @sschleemilch : Here the implements have both a tank and a boom array as children. This is not possible with instances. Using instances you always need a non-instance level in between which will end up in And I still think the actual numbers for A, B, C should be described in a way that makes it easy to describe deviations like already discussed in this PR. I am sure there is a way to describe typical variation points which will be necessary when VSS is used beyond passenger cars. Not directly related but triggering an idea: In your (@sschleemilch) example you created instances that will end up expanded in branches Using |
Yeah, I get the idea and yes, you cannot express it with instances. So I am in general fine to change it but probably we only should have either That |
|
I agree that the name instances is more intuitive than elements. So I am for keeping the name but change the behavior. |
|
@wiedmama |
One example (as already written above) are nested branches
Here you have up to 3 levels to describe the machine structure e.g. of a fertilizer. Moreover there are things like an Regarding the name Besides this with which will not work with Introducing a new tag also allows a migration scenario where instances and elements can be used in the same tree. After a migration period, the instances might be marked as outdated/legacy an can be finally removed. |
|
@wiedmama , your comments on naming are valid. |
Quite often we need those nodes. Otherwise you just start messing around with naming of concept and instances and then whole model is gone. Also, where do you add attributes that are related to all instances which you have? If we remove top node in many cases you will just have to extend attribute name and find a patch. |
You are right: We need a place to put everything that is related to all instances. But the "top node" is not removed, it is just side-by-side with the generated elements/instances: Will generate This approach allows to keep the expanded tree the same and so should not affect all tools which work on expanded trees. The As already explained above I would also be fine if it generates On the upside, with this approach it is clear that pure number branches are instances/elements of its parent branch. It is also clearly visible what is common for all instances (everything else). On the downside, I am not sure whether all tools can handle pure number branches and it would be a breaking change as the resulting expanded tree would be different. |
|
@wiedmama I think your examples breaks the elements model. To generate Vehicle.Foo.FooInstances: Or have I misunderstood the new model? |
In short: No you were right. The declaration should be It should be elements: Foo[1,3] not elements: [1,3]. This was my original suggestion. The other/new approach came in my mind when I read the remarks of @adobekan which includes paths like can be generated by which is quite the same But as already explained, this will be a breaking change as the generated models will be not the same and it is only possible if pure number branches are allowed and supported by tools. |
|
New name proposal: array |
|
@UlfBj From your example I assume that you want to avoid pure number branches. I guess we will run intro trouble with pure number branches: When e.g. an SDK is generated with access objects, the class names will bee based on the branch name but class names in many programming languages must not be numbers. To allow an assignment of the generated elements to the owning parent I would like to suggest that the generated branches names always start with name of the base branch: will expand to |
|
@wiedmama , I believe that e. g. JASPAR wants pure number nodes in their "VSS tree". So although I agree with you that it might lead to trouble in some scenarios I think we should leave that judgement to the implementers of the trees. The same goes for the relationship between parent/child node names. I do not think we should restrict it as you propose above, but leave also this to the judgement of the implementer. |
|
In the CVIS project we are defining a trailer tree that shall include a path structure as shown below, This was attempted to be achieved with the vspec below but VSS-tools rejects it as a Critical model error. Axle: #include Axle.vspec Axle Axle.Liftable: #include AxleFeature/Liftable.vspec Axle.Liftable I cannot see how to get around this deficiency of the instances concept (I guess this is what @wiedmama has pointed to already). If that is the case I would say the decision to replace it with the array concept should be straight forward as I believe it can generate the wanted path structure. |
|
I don't see why not: Vehicle:
description: d
type: branch
Vehicle.Axle:
instances:
- Row[1,2]
type: branch
description: d
Vehicle.Axle.Wheel:
instances:
- Pos[1,2]
description: d
type: branch
Vehicle.Axle.Liftable:
type: branch
instances:
- Pos[1,2]
instantiate: false
description: d
Vehicle.Axle.Liftable.Position:
type: sensor
datatype: uint8
description: d
Vehicle.Axle.Liftable.AxleConfig:
type: branch
description: d
instantiate: falseThe only valid deficiency of instances is that you will always need a branch node in between instance definitions if you want to attach something on all of those instances, which is not the case here. |
|
@sschleemilch you are right. This was what I believed I was doing, but after looking closer I found i did not. Thanks for triggering me to look closer :). |
|
See COVESA/vss-tools#545 for a possible solution by introducing a new keyword |

This is a PR for discussion. Now and then we have discussed how to handle instances as they typically varies. This ticket concerns an idea of decoupling instance definition from the actual tree. In shot - anyone can use what instances they find useful for their vehicle. But for reference VSS can publish a default instance set to get same signals as today.
vspec export yaml -u ./spec/units.yaml --strict -s ./spec/VehicleSignalSpecification.vspec -o vss.yaml -l spec/default_instances.vspecThe latter could possibly be handled by both documenting recommended instantiation models ("use RowX for axles") or alternatively by adding syntax support to define "allowed" instance models for signals, like
instance_type: ROW_POS. We could also just give recommendations in thedefault_instances.vspecIf we go this way we need to agree on what our standard release assets shall contain - with/without default instances.
Related to #898 #810 #642 #564