feat: instances relation

[sschleemilch]

About

Proposed solution for COVESA/vehicle_signal_specification#919.

Implements the possibility to replace the node where the expanded instances will be attached to instead of attaching them as children.

Added a new literal keyword instances_relation that takes either sibling or child. Default is child and is the current behavior.

Example

Vehicle:
  description: d
  type: branch

Vehicle.Door:
  description: d
  type: branch

Vehicle.Door.Row:
  description: d
  type: branch
  instances:
    - Row[1,3]
  instances_relation: sibling

Vehicle.Door.Row.Count:
  description: d
  type: attribute
  datatype: uint8
  instantiate: false

Vehicle.Door.Row.Side:
  description: d
  type: branch
  instances:
    - [Driver,Passenger]
  instances_relation: sibling

Vehicle.Door.Row.Side.Count:
  description: d
  type: attribute
  datatype: uint8
  instantiate: false

Which will produce:

Vehicle
└── Door
    ├── Row
    │   └── Count
    ├── Row1
    │   ├── Side
    │   │   └── Count
    │   ├── Driver
    │   └── Passenger
    ├── Row2
    │   ├── Side
    │   │   └── Count
    │   ├── Driver
    │   └── Passenger
    └── Row3
        ├── Side
        │   └── Count
        ├── Driver
        └── Passenger

Note that this enables to attach elements to all RowX instances (here Count) but still have the Driver and Passenger as direct children of those instances.

This was not possible before since you would need a node in between to do it:

Vehicle:
  description: d
  type: branch

Vehicle.Door:
  description: d
  type: branch
  instances:
    - Row[1,3]

Vehicle.Door.Row:
  type: branch
  description: d
  instantiate: false

Vehicle.Door.Row.Count:
  description: d
  type: attribute
  datatype: uint8

Vehicle.Door.Side:
  description: d
  type: branch
  instances:
    - [Driver, Passenger]

Vehicle.Door.Side.Count:
  description: d
  type: attribute
  datatype: uint8
  instantiate: false

Which results in:

Vehicle
└── Door
    ├── Row
    │   └── Count
    ├── Row1
    │   └── Side
    │       ├── Count
    │       ├── Driver
    │       └── Passenger
    ├── Row2
    │   └── Side
    │       ├── Count
    │       ├── Driver
    │       └── Passenger
    └── Row3
        └── Side
            ├── Count
            ├── Driver
            └── Passenger

See that the sides can only be attached to the Side child of all RowX instances.

[wiedmama]

@sschleemilch
I would call it rather relation instead of instance_mode and sibling instead of remove as there is nothing removed and it fits better to the (default) option childern.

[sschleemilch]

@sschleemilch I would call it rather relation instead of instance_mode and sibling instead of remove as there is nothing removed and it fits better to the (default) option childern.

Renamed option to instances_relation with values child and sibling

[wiedmama]

With the new approach it should be possible (and supported) to create trees like

A
A.Count
A.B.Count
A.B1.SomeValue
A.B2.SomeValue
A.C.Count
A.C1.AnotherValue
A.C2.AnotherValue

Where A holds two sibling instances B and C on the same level.

This was not possible in the past.

@sschleemilch : Can you please extend your tests to cover this also?

[sschleemilch]

@wiedmama done

[UlfBj]

This is a clever solution, we can both eat the apple and keep it.

[wiedmama]

@wiedmama done

Thank you for adding this but after a closer look to my example I have a question:
Can you explain to me the purpose of A.Count? I can't 😳.

So I think it should be removed to avoid confusion.

[sschleemilch]

@wiedmama done

Thank you for adding this but after a closer look to my example I have a question: Can you explain to me the purpose of A.Count? I can't 😳.

So I think it should be removed to avoid confusion.

Removed. Took your expected tree 1:1

[erikbosch]

Great progress during my vacation, I like it!

Things to discuss in the next VSS meeting:

• Any objections to this proposal
• Do we want (at some time in the future) to change the default for the standard catalog, or for all?
• Are there long term requests to keep the old syntax
  • Short/Mid-term I understand removing the old style would give backward incompatibility problems

[sschleemilch]

Do we want (at some time in the future) to change the default for the standard catalog

What default? The standard catalog is fine unless we think that some instances should actually be a sibling instead of child

[wiedmama]

I could imagine doing it in multiple steps:

1. Introduce new instances_relation tag with default child (what we are now about to do).
2. Change the standard VSS catalogue to use instances_relation: sibling in every place (should be possible without changing the expanded tree but further discussions needed here).
3. If/When the new VSS standard catalogue does not longer use instances_relation: child: Change the default for instances_relation from child to sibling.

[erikbosch]

Do we want (at some time in the future) to change the default for the standard catalog

What default? The standard catalog is fine unless we think that some instances should actually be a sibling instead of child

Yes, that is what I think we should discuss. Do we want to keep current tree as is or not. If we get new PRs with new instances - do we prefer old or new syntax.

[erikbosch]

MoM:

• Schildt: How does this relate to VDM, improve here or there?
• Schildt: So this is just syntactic sugar?
• E: Could possibly help downstream implementations, like discoverability in APIs
• JD: Downstream tolls may not support this
• E: Do we want to use the new construct in std catalog
• JD: The vspec is the core catalog, this just affects exporters
• E: But we include artifacts as part of releases
• Schildt: Like yaml export
• Schildt: We could add "child" explicitly
• JD: Change should preferably be non-intrusive to limit needed work.
• JD: As long as you can get the same output it is fine
• JD: Many use concatenated path as id, with an identifier that would not be a problem
• Please review

[sschleemilch]

I could imagine doing it in multiple steps:

1. Introduce new instances_relation tag with default child (what we are now about to do).
2. Change the standard VSS catalogue to use instances_relation: sibling in every place (should be possible without changing the expanded tree but further discussions needed here).
3. If/When the new VSS standard catalogue does not longer use instances_relation: child: Change the default for instances_relation from child to sibling.

Why should everything be switched to sibling? Do we think that every instance we modelled in vss should actually be a sibling and we just didn't do it because it was technically not possible? If yes, then okay but then its a major breaking change.

[wiedmama]

Why should everything be switched to sibling? Do we think that every instance we modelled in vss should actually be a sibling and we just didn't do it because it was technically not possible? If yes, then okay but then its a major breaking change.

I had current implementation of doors in mind:

Door:
  type: branch
  instances:
    - Row[1,2]
    - ["DriverSide","PassengerSide"]

where it is not possible to put something to the branch
Vehicle.Cabin.Door.Row1 as you cannot get in between e.g. Row1 and DriverSide. As far as I understood you can only put something at Vehicle.Cabin.Door or then below e.g. Vehicle.Cabin.Door.Row1.DriverSide. That's a general problem with these multi dimensional instances.

On the plus side, re-write them with instances_relation: sibling the expanded tree would be (almost, see next point) the same and you will gain the possibility to put something on Row level.

On the minus side it will add maybe unused branches for e.g. Vehicle.Cabin.Door.Row and Vehicle.Cabin.Door.Row1.Side. At least for the Row branch and Count attribute will be useful but for the Side branch something like Count won't be enough to "know" which generated siblings are there.

[sschleemilch]

Understood. This is of course fine as a follow up task.
I was also thinking about unused branches and whether we should add an option to strip empty branches so that exporters do not even see them

[sschleemilch]

So basically targeting all instances with two or more dimensions to give downstream a chance to attach to them