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

Feature rule set extensions #685

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

alanmapleburl-au
Copy link

@alanmapleburl-au alanmapleburl-au commented Nov 8, 2023

These extensions to the VSS allow consumers to understand each signal well enough to determine its relation to signals emitted by vehicle components and to downstream client and consumer needs. Because they are optional extensions permitted by VSS Overlays, they do not break any current code or processes.

The properties.md file describes the promotion of dataProperty and objectProperty to a first-class types, allowing signals to reference the property they report.

The **elementType ** key provides each entry's classification within the vehicle signal domain, such as SignalDefinition, FeatureOfInterest, Property, etc.

The **identifier ** key provides a unique and immutable key. This allows restructuring of the tree and the files/folders without any effect on the key. In my examples I've used the entire path of the entry, but it could be any unique value.

The **definition ** key provides the necessary and sufficient conditions of the entry which can translate to formal axioms.

The featureOfInterest and property keys in the attributes, sensors, and actuators explicitly refer to the object being observed or manipulated, and the property being reported. This is needed not only to aid integration, but to allow analysis, discovery, automation, and inferencing through the graph.

Add k/v pairs

Supports inter-entry references, domain types, and rigorous semantics
Add the Property entry

Allows signals to explicitly refer to the property they report
Add k/v pairs

elementType classifies attributes as signal defniitions
featureOfInterest and property, along with definition, explicitly define the attribute
Add k/v pairs

identifier
featureOfInterest and property
elementType and definition
opposed to a signal entry). This is the default, in case ```type``` is omitted.
The value ```branch``` specifies that this is a branch entry. This is the default, in case ```type``` is omitted.

**```identifier```** *[optional]*
Copy link
Collaborator

@SebastianSchildt SebastianSchildt Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not quite sure isn't this redundant if it "is the Path? And if it just "could be" the path isn't it the same as arbitrary static-id?

Do you have a PR to vss-tools, to see how it would be used/influence the outputs?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it could be practical in some representations, like when we generate JSON so that you can get the full name directly without the need of checking the structure. But if so it is something that typically cannot and should not be defined in source *.vspec files, but rather be generated by tooling

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SebastianSchildt The direct answer is, "no." I won't personally be using vss-tools. The long answer is that as a data modeler/ontologist I work to make sure that schemas, models, and ontologies are designed in such a way to be generally useful. At Ford, we would like to use our signal schema for many things beyond those imagined in vss-tools, and this has already proved helpful. There are also some rather subtle conceptual principles in play, such as the where to draw the line between a feature of interest and a property. Having the entire path visible I believe pushes people to a more informed decision.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I've updated the initial comment to try to be more explicit

A set of characters that uniquely identifies the branch, usually the full path

**```elementType```** *[optional]*
A type that categorizes the branch's role in the vehicle signal domain, or as a Node if the branch exists purely for tree traversal and has no vehicle signal domain signifcance on its own.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some rule, how this Applies to current VSS, i.e. how to decide what is what? Are "Features of Interest" only things that are currently instantiated and everything else is a "Node", i.e waht would Vehicle.Body.Lights be?

Also what is the difference between FeatureOfInterestand Node

Copy link
Author

@alanmapleburl-au alanmapleburl-au Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A feature of interest is any physical object whose properties can be observed or reported. Vehicle.Body.Lights has a property (LightSwitch) that can be reported, so it's a feature of interest. (I'm not saying it's an ideal one.) There are branches in VSS that simply act as parent nodes to a group of signals. I just looked and they've mostly been cleaned up :^) This one remains in vehicle.vspec:

Acceleration:
type: branch
description: Spatial acceleration. Axis definitions according to ISO 8855.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a featureOfInterest does not necessarily have to be a physical object. It can also refer to a specific aspect or characteristic of something, such as the charging function of an electrical vehicle.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thank you. In fact, in our internal model we have a feature of interest called "chargingSession," which is a process. I'm not quite sold on functions, and in fact we modeled all charging signals without a thing called "charging." But I think about functions in the philosophical sense as something that a feature of interest is intended to achieve. That BFO book I recommended explains all that.

@SebastianSchildt
Copy link
Collaborator

I made some small comments, but generally, to get a better understanding, can you somewhere upload how the standard catalogue would look like implementing these suggestions? I.e. modified vspecs or something you generated. I guess like all things, what specifically to use as content for your suggested properties might be discussed on a case by case basis, but seeing some "this is how I meant it" example of the VSS as you have modified might help the discussion.

**```elementType```** *[optional]*
A type that categorizes the branch's role in the vehicle signal domain, or as a Node if the branch exists purely for tree traversal and has no vehicle signal domain signifcance on its own.
- ```FeatureOfInterest```: A physical object whose properties can be observed and possibly manipulated.
- ```Node```: A physical object whose properties can be observed and possibly manipulated.
Copy link
Collaborator

@erikbosch erikbosch Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They should not have the same description, or?

Do we have a need to consider one of them as "default", i.e. if elementType is not given is it then a "node" or is it undefined.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, 99% are Feature of Interest so that could be the default. At some point I will probably create a PR that adds one more branch element type, but let's not get ahead of ourselves.

Copy link
Author

@alanmapleburl-au alanmapleburl-au Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've committed a change to describe node as 'A branch in the tree that does not correspond to a domain object. These are generally used to group related nodes.'

---
title: "Property Entry"
date: 2023-11-07
weight: 1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for ordering when we generate HTML with Hugo. If we want it last we should have at least 8

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, got it. Thanks. Please feel free to change, btw.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erikbosch I think it should probably be after Branch Entry and at the same level.

The identifier of the physical object whose properties can be observed and possibly manipulated by signals of this type

**```property```** *[optional]*
The identifier of the property being reported by signals of this type
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are to introduce "optional" support for property while keeping the current mechanism of explicitly specifying datatype+unit for each signal we need to give some guidelines on how they shall be used. Topics like:

  • Are they allowed/recommended for the standard catalog (i.e. the *.vspec files in this repo)
  • If allowed/recommended in the standard catalog, is it then mandatory that the referenced property has the same datatype and unit as explicitly stated for the signal (to avoid confusion)
  • If used for customization or extension, and datatype/unit differs between property and what is explicitly stated for signal, then what has priority?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add those, thanks.

date: 2023-11-07
weight: 1
---

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The term property is already used to define "members" of a struct, see https://github.com/COVESA/vehicle_signal_specification/blob/master/docs-gen/content/rule_set/data_entry/data_types_struct.md

We may theoretically live with that if define two types of properties in different context (struct property in type file, data property somewhere else) but there is a risk for confusion so changing name on one of them likely makes sense. An easy approach would be "dataproperty" and "structproperty", but there is possibly better names. As part of this we need to discuss how/where properties should be defined. In any of the existing trees (signal tree and type tree) or in a new tree? We should also state some guidelines/decisions on how the VSS-project intends to work with (data)properties. Do we intend to add data properties to the standard catalog (short term or long term), or is it totally up to user?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name: We can get rid of Property altogether and just promote DataProperty and ObjectProperty to the type. (Unfortunately, "Attribute" was also used.)

Supposing this PR gets merged, it would be rapidly followed by a series of PR's that contain properties and add a reference to them in the signals. They are definitely meant to be part of the catalog and not up to the user. I would not add them to the existing tree as they are meant to be reused.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've committed another change that replaces property with dataProperty and objectProperty.

@erikbosch
Copy link
Collaborator

erikbosch commented Nov 9, 2023

As general info to reviewers - this PR is related to the presentation "Integrating Vehicle Signals with VSS and Metadata" which is available in the COVESA wiki. Please review the presentation to get more background information.

Some areas that I think we should discuss:

  • There have been similar discussions in the VSSo group, are we ready to "promote" the proposed extension as official (but optional) extensions, or should we better for now look a how this can be managed as a custom VSS profile, and how VSS and vss-tools can manage custom profiles.
  • If accepted as an "offical extension", how do we want the VSS standard catalog (the *.vspec files) to use the new additions. Not at all, or do we gradually want to introduce some of them, ans possibly long term replace some of them with the new concept? Maybe we need a table or similar to state what we expect or accept in the standard catalog for new signals.
  • How can we define semantic rules, like if a signal has specified datatype "uint8" and property "MyProperty" which in turns specify "uint16", what does that mean? Is that a semantic error or does someone of them have precedence? Or is it up to the user?
  • How do we want vss-tools to handle the new items. An obvious question is how/where properties should be defined. But apart from that - how shall output for existing tools be affected? Vss-tools may for example generate identifier, and it is easy to include in yaml/json output, but not obvious how to include it in other formats.

@erikbosch erikbosch marked this pull request as draft November 13, 2023 09:05
@erikbosch
Copy link
Collaborator

erikbosch commented Nov 14, 2023

Meeting notes:

  • Erik: Adnan&Erik to have meeting with Alan to discuss way forward
  • Peter: This makes VSS difficult to understand, would be difficult to convince people to use it.
  • Daniel: Important to specify what requirements this solution fulfills. We need a framework to define and discuss requirements on VSS.
  • Erik: But we do not have any formal requirements on VSS as of today, could possibly be useful if managed well
  • Daniel: There have been some discussions within VSSo, but Ford presentation cover many ideas not yet socialized in VSSo. I see values in some features, not all needed
  • Daniel: we need COVESA guideline on how to document projects/initiatives, a simple form for each project. We need to make PRs more objective, the list of requirements must be shared/agreed. I have offered Paul to assist in establish guidelines.

@alanmapleburl-au
Copy link
Author

alanmapleburl-au commented Nov 15, 2023

@SebastianSchildt Please see Erik's general info to reviewers. He refers to a presentation that will satisfy your desire to see it in action.
@erikbosch I would follow up the acceptance of this PR with a series of PR's that populate the additions
@erikbosch I think the initial rule on signals is that the property's data type, unit, min, and max are reviewed for any overrides. That doesn't break anything. Eventually the rule could be that those k/v pairs default to the specified property if omitted.
@jdacoello I tried to specify the requirements. If you comment the ones that seem unnecessary I can elaborate.

I'm interested in the notion of a custom VSS profile but couldn't find any info on that.

@jdacoello
Copy link
Contributor

jdacoello commented Nov 16, 2023

  • Daniel: ...I have offered Paul to assist in establish guidelines.

Here is the first draft for such a methodology. I explain there the fundamental components that must be described by each of the COVESA projects and its associated artefacts using a unified structure.

https://wiki.covesa.global/pages/viewpage.action?pageId=85196928

@erikbosch
Copy link
Collaborator

I spent some time thinking how we possibly could "transform" this PR into a "VSS Profile". Not straightforward as we do not really have anything called "VSS profile" as of today, but I created a draft PR to show my ideas. My idea is to create a section in the documentation where we document more or less official VSS extensions. In my draft I created something very short for the Eclipse KUKSA DBC mechanism and what I consider mostly as a placeholder for the "Ford Data Properties" (or whatever we would like to call it). This could possibly be a way forward to capture the good ideas from Alan but at the same time postponing the decision on whether this shall be an official/recommended profile or even integrated into the official syntax.

image

@erikbosch erikbosch added Scope:Strategic a change that significantly would change scope/meaning/technology of project. Status:Review Please review/discuss contents labels Nov 21, 2023
@SebastianSchildt
Copy link
Collaborator

I like the general direction, as a word could we maybe rather call this "VSS Extensions" or "VSS Addons"? maybe somebody has a better term, but for me "Profile" either sounds like "Two wheeler profile" "Truck profile" pr potetnially something like "VSS base profile", i.e. in my head I connect it more with the "catalogue" part, whereas the things suggested here are bit more about adding new "features"/functionality to the model

@erikbosch
Copy link
Collaborator

Meeting notes:

  • Daniel: Risk if we allow addition of extensions that we split efforts, that it will not kept in synched with other efforts

  • Daniel: Would recommend a clear scope of what the VSS Yaml supports.

    1. Level of agreements - terminology/vocabulary, schemas
    1. Tooling
  • What can be done - right now targeting vocabulary

  • If we want schema we may target a more expressive/modern language to express it.

  • Erik: Is conclusion that proposal shall be seen as input to VSSo project, and not included into VSS documentation as of today, neither as std or as "extension".

  • Daniel: We should extend scope of ontology. VSS project should focus on vocabulary of VSS signals. We should better use GraphQl tooling so that we do not need spend so much time on tools. Better than expanding VSS syntax.

@erikbosch erikbosch added Status:DoNotMerge PR shall not be merged now, maybe later, maybe after rework and removed Status:Review Please review/discuss contents labels Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope:Strategic a change that significantly would change scope/meaning/technology of project. Status:DoNotMerge PR shall not be merged now, maybe later, maybe after rework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants