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

Instances in vehicle_signal_specification/spec/Vehicle/MotionManagement /Steering.vspec for Axle are probably wrong #804

Open
Herrmann-Alexander opened this issue Jan 28, 2025 · 7 comments · May be fixed by #809

Comments

@Herrmann-Alexander
Copy link
Contributor

Image
While looking trough the VSS signals I saw that the instances for Axle in vehicle_signal_specification/spec/Vehicle/MotionManagement /Steering.vspec were Row[1,1] while they should probably be Row[1,2]

@erikbosch
Copy link
Collaborator

When proposing the model we opted for having Row[1,1] as default number of instances, as relatively few vehicles have four wheel steering, i.e. better to have just one instance as default, so that users can change to two instances in an overlay (or in a fork) if they need support for four wheel steering.

Do you have a need for four wheel steering in your VSS usage?

@Herrmann-Alexander
Copy link
Contributor Author

No, I don't have a need for four wheel steering. I just thought it might be unconsciously and wanted to point it out if it's a typo or something. But since it's intended I can close

@erikbosch
Copy link
Collaborator

But we should at least add a comment/rationale, I can create a PR for that and bring up the discussion

@Herrmann-Alexander
Copy link
Contributor Author

Sounds good

@erikbosch
Copy link
Collaborator

MoM:

  • Issue presented at meeting
  • Erik to prepare PR with rationale, will be discussed when pr available

erikbosch added a commit to boschglobal/vehicle_signal_specification that referenced this issue Feb 6, 2025
Two parts

- Explain why we currently have Row[1,1] for steerable axle
- Add some general info on the design decisions that we (implicitly) have made over time for instances

Fixes COVESA#804
Signed-off-by: Erik Jaegervall <[email protected]>
erikbosch added a commit to boschglobal/vehicle_signal_specification that referenced this issue Feb 6, 2025
Two parts

- Explain why we currently have Row[1,1] for steerable axle
- Add some general info on the design decisions that we (implicitly) have made over time for instances

Fixes COVESA#804
Signed-off-by: Erik Jaegervall <[email protected]>
@erikbosch erikbosch linked a pull request Feb 6, 2025 that will close this issue
@jdacoello
Copy link
Contributor

Isn't it the same as:

...
  instances: [Row1]
...

I think the confusion is caused by the use of the notation PREFIX[start,end] which is intended to expand all the possible combinations.
Here, if the range will expand into only 1 label, then using simple Row1 as instance label would do the job.

@erikbosch
Copy link
Collaborator

erikbosch commented Feb 12, 2025

Yes, but then we would use a different pattern compared to other Row instantiations. But i have no strong opinion if we rather would like to use/recommend instances: ["Row1"] if there is only one row and instances: Row[1,n] if there are more. You can discuss in next meeting.

For current usage it does not really matter. Would maybe matter more if we would have something like in #642 where instances are proposed to be defined with type. Would make absolutely no difference for key/value-pair solutions but could make difference if you have a model based implementation where you want to call things like

my_axle = getAxle(row=1) or my_axle = getAxle("Row1")

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.

3 participants