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

Instance expansion buggy when using a mix of list unrolling and "normal" #390

Closed
sschleemilch opened this issue Jul 29, 2024 · 2 comments · Fixed by #382
Closed

Instance expansion buggy when using a mix of list unrolling and "normal" #390

sschleemilch opened this issue Jul 29, 2024 · 2 comments · Fixed by #382

Comments

@sschleemilch
Copy link
Collaborator

Found while doing #382. Not sure if we should try to fix it though if we want to go with #382.

Test vspec:

Vehicle:
  type: branch
  description: Vehicle

Vehicle.Door:
  description: Door
  type: branch
  instances:
  - Row[1,2]
  - RowId
  - Column[1,2]
  - ColumnId

Expected:

Vehicle:
  description: Vehicle
  type: branch

Vehicle.Door:
  description: Door
  type: branch

Vehicle.Door.Row1:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row1.RowId:
  description: Door
  type: branch

Vehicle.Door.Row2:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row2.RowId:
  description: Door
  type: branch

Actual:

Vehicle:
  description: Vehicle
  type: branch

Vehicle.Door:
  description: Door
  type: branch

Vehicle.Door.Row1:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1.Column1:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1.Column1.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1.Column2:
  description: Door
  type: branch

Vehicle.Door.Row1.Column1.Column2.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2.Column1:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2.Column1.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2.Column2:
  description: Door
  type: branch

Vehicle.Door.Row1.Column2.Column2.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row1.RowId:
  description: Door
  type: branch

Vehicle.Door.Row2:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1.Column1:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1.Column1.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1.Column2:
  description: Door
  type: branch

Vehicle.Door.Row2.Column1.Column2.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2.Column1:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2.Column1.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2.Column2:
  description: Door
  type: branch

Vehicle.Door.Row2.Column2.Column2.ColumnId:
  description: Door
  type: branch

Vehicle.Door.Row2.RowId:
  description: Door
  type: branch
@erikbosch
Copy link
Collaborator

I think the instance handling code as of today has some limitations. Looking at https://covesa.github.io/vehicle_signal_specification/rule_set/instances/ one can see that two "types" of instance declarations:

  • a list of strings, where each element defines a single instance, e.g. ['DriverSide','PassengerSide'] results into two instances of every following data entry in the path, named DriverSide and PassengerSide
  • a string, followed by a range defined through [n,m], with n,m as integer and n <= m, which defines the number of instances. Position[1,4] results into 4 instances of every following data entry in the path, named Position1, Position2, Position3 and Position4. It is in VSS recommended to use 1 as start index for the first row/axle/position/…

If you use something else you can unfortunately get unexpected results. Like in your case when - RowId is neither a "prefix+range" or "list of strings". Do you get what you expect if you define it like below?

Vehicle.Door:
  description: Door
  type: branch
  instances:
  - Row[1,2]
  - [RowId]
  - Column[1,2]
  - [ColumnId]

As long as we only intend to support <string>[<min>,<max>] and [<literal1>, <literal2>,...] then an example like your should preferably give an error. I have by the way thought of having a more elaborate instance syntax, see COVESA/vehicle_signal_specification#642

@sschleemilch
Copy link
Collaborator Author

I fixed it in the referenced PR that supports that case, so should be fine in the long run if we wanna go with it.
Will try to join tomorrow to present it a bit. Currently fixing tests. All instance tests are green already

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants