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

Extending the instances functionality #564

Open
jdacoello opened this issue Mar 15, 2023 · 10 comments
Open

Extending the instances functionality #564

jdacoello opened this issue Mar 15, 2023 · 10 comments

Comments

@jdacoello
Copy link
Contributor

As far as I know, the Instances: field in VSS is meant to be applied to branches only. However, this in not practical when there will be only one property of interest deriving from that branch (i.e., each branch will have only one leaf). Consider the following case:

SomeBranch.[instances of a InstanceBranch].propertyOfInterest
SomeBranch.InstanceBranch1.propertyOfInterest
SomeBranch.InstanceBranch2.propertyOfInterest
SomeBranch.InstanceBranchN.propertyOfInterest

When each of the instances of the branch will have only one propertyOfInterest, it makes more sense to instantiate the propertyOfInterest instead of the branch. The previous example would result in something like:

# Definition
SomeBranch.propertyOfInterest[instances of the property]

# Result
SomeBranch.propertyOfInterest1
SomeBranch.propertyOfInterest2
SomeBranch.propertyOfInterestN
@alanmapleburl-au
Copy link

alanmapleburl-au commented Dec 5, 2023

If I'm understanding correctly, then I would support the exact opposite idea. Namely, partitioning features of interest and reusing properties. I'll call these two approaches "Reuse FOI" and "Reuse Property." Here's a specific example articulated in both approaches:

Reuse FOI

Vehicle.Seat.Airbag.IsRow1LeftDeployed
Vehicle.Seat.Airbag.IsRow1RightDeployed

Reuse Property

Vehicle.Seat.Airbag.Row1.Left.IsDeployed
Vehicle.Seat.Airbag.Row1.Right.IsDeployed

Here are some reasons I support reusing the property:

  • The property IsDeployed is explicitly defined once, perhaps as "A boolean variable indicating that a feature has been activated"
  • Models attain resilience when they represent reality. There is, in fact, an airbag at the front left and front right.
  • Those instances may acquire new properties. For example:
Vehicle.Seat.Airbag.Row1.Left.IsPresent
Vehicle.Seat.Airbag.Row1.Left.IsEnabled

@ppb2020
Copy link
Collaborator

ppb2020 commented Dec 5, 2023

I understand airbags are used only as an example here, but I wanted to mention that there are many airbags in a vehicle as well as many different types (steering wheel, front passenger, various curtains, seat airbags, etc.). It becomes complicated to provide a standard way to reference them. Hence, we may wish not to associate them with a seat like in the example herein, but instead provide an array of different airbags present in a vehicle. For such an approach, we would need to decide how best to represent instances of airbags...

@erikbosch
Copy link
Collaborator

Concerning airbags, there have not been any discussion related to airbags during the time I have been active in COVESA, so I consider that part of VSS (as well as HVAC and some other areas) as quite immature and open for refactoring if needed. Maybe airbag is a good example where VSS shall not specify available instances like FrontLeftCurtain but just say that there may be multiple airbags in the vehicle and you access them by Vehicle.Airbag.<id>.IsDeployed where <id> is a string specified by OEM.

(Then we might need to have some "pre-defined" signals like IsPassengerSideAirbagOff, indicating whether it is safe to put a backward-facing child seat in the front passenger seat or not, but exactly which airbags that are disabled is up to the OEM)

@alanmapleburl-au
Copy link

Can we open a new discussion for airbags? I can change the example to headrests or whatever if that helps keep the conversation focused.

@ppb2020
Copy link
Collaborator

ppb2020 commented Dec 6, 2023

Absolutely. However, how airbags (or any other instance node) may be represented could perhaps have an impact on instance functionality as discussed here. In the example:

# Definition
SomeBranch.propertyOfInterest[instances of the property]

What would happen is the instances were IDs as Erik mentioned? What would happen if the instances are names of locations instead of IDs:

instances:
 - [DriverSeat, PasssengerSeat, LeftCurtain, RightCurtain]

We would end up with:

Cabin.IsDeployedDriverSeat

Would that make sense?

@jdacoello
Copy link
Contributor Author

jdacoello commented Dec 6, 2023

Please bear in mind that I started this discussion several months ago. Back then, there was not yet a proposal on the table to include the concept of FeatureOfInterest. My initial description applied to the idea of having merely:
SomeBranch.AnotherBranch.SomeProperty

Now, let me update my opinion here. If a FeatureOfInterest becomes an official part of VSS (with proper management of IDs, etc.), then it has much more meaning to (arbitrarily) instantiate a FeatureOfInterest as needed instead of the properties. I am in favor of a pattern like: Some(Instantiatable)FeatureOfInterest.SomeProperty

For example:
Door(instance).IsOpen
WindowSwitch(instance).IsEngaged
Tire(instance).Preassure

Properties themselves can be reused as long as the same datatype and description hold True for the intended use.


Regardless of the decision, I think of two possible improvements:

  1. The notation for instances have to change because the dots can be confusing.
    For instance: Instead of:
    Door.Row1Left.IsOpen
    We should have one of:
    Door(Row1Left).IsOpen
    Door{Row1Left}.IsOpen
    Door[Row1Left].IsOpen

  2. The current branching must be taken out of the regular VSS modeling workflow.
    For instance, instead of:
    Root.SomeBranch.AnotherBranch.YetAnotherBranch.Some(Instantiatable)FeatureOfInterest.SomeProperty
    We should focus on properly defining:
    Some(Instantiatable)FeatureOfInterest.SomeProperty

Then the context information can be kept in a separate way and will allow multiple ways of classifying the concepts:
Root.SomeBranch.AnotherBranch.YetAnotherBranch.SomeFeatureOfInterest

@alanmapleburl-au
Copy link

@ppb2020 SomeFeature.SomeProperty resolves that...Cabin.DriverSeat.IsDeployed

@jdacoello
Copy link
Contributor Author

@alanmapleburl-au how should I interpret your 😆 emoji on my latest comment?

I think either agreement 👍🏻 or disagreement 👎🏻 with arguments would help Us more to find a common consensus.

@alanmapleburl-au
Copy link

@jdacoello Oh, I thought you were agreeing with me :^)

@jdacoello
Copy link
Contributor Author

@jdacoello Oh, I thought you were agreeing with me :^)

I agree with this pattern Some(Instantiatable)FeatureOfInterest.Some(Reusable)Property.

However, my comment (updated opinion) included a couple more aspects. That is why I didn't know how to interpret that. So, I rather asked.

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

No branches or pull requests

4 participants