Struct syntax proposal #511

erikbosch · 2022-11-21T10:33:30Z

We have in #326 had a long discussion on struct support, this is a syntax/semantic proposal trying to include some corner cases.
If we are to announce/release struct support before next AMM we should better get going, as the work needs to be performed in multiple increments.

Note: This PR is not supposed to be merged as is, rather as a discussion platform for #326. Selected parts shall later be integrated into documentation.

Note: This PR is not supposed to be merged as is, rather as a discussion platform for COVESA#326

struct.md

kkoppolu1 · 2022-11-22T21:15:54Z

struct.md

+* Else if `A.B.X` exists, then it will be used.
+* Else if `A.X` exists, then it will be used.
+
+### Reference by absolute path


+1 to this.
Referencing by qualified name/path is useful.
It allows for namespacing type names with the same identifier (like C++ namespaces).
We could think of the node path of the struct node as the enclosing namespace for the type name.

kkoppolu1 · 2022-11-22T21:17:44Z

struct.md

+
+As the example above.
+
+TBD: Or do we want a more flexible approach, i.e. if you specify "ABC" as datatype, that a tool shall search upwards in all parent branches?


"Reference by absolute path" more explicitly conveys a user's intent than a graph search for the type name. Looking at the vspec, the reader wouldn't know where exactly the type is picked up from.

I have no problem limiting ourselves (for now) to either referring to a name in the same branch or using absolute path. We can always add graph search later if there would come up a reasonable use-case

struct.md

kkoppolu1 · 2022-11-22T21:21:53Z

struct.md

+
+TBD: Where shall the inner struct be defined? Shall it be allowed to define it within a struct as well, or does it need to be defined within a branch like above? If allowed to be defined within a struct, how do we want name resolution to work, only support exact (current) scope and absolute path, or a more flexible setup searching upwards.
+
+I.e. shall the following alternative style (where the struct `OpenHours` is defined within `DeliveryInfo`) be allowed or even preferred?


This is a much more advanced modeling use case (like inner classes in programming languages which is not that often used). We could extend the specification once the basic modeling of structs is launched and we have feedback on gaps/use cases that need to be covered by future extensions.

I have no problem with that limitation. Better to start small and extend as needed. If no-one has any other objection I will remove the "inner struct" example and replace it with a statement similar to:

"It shall not be allowed to define a struct within a struct"

erikbosch · 2022-11-23T09:55:54Z

struct.md

+DeliveryList:
+  datatype: DeliveryInfo[]
+  type: attribute
+  default: [{'Munich','BMW'},{'Feuerbach','Bosch'}]


Added a proposal for default values

Also other updates

erikbosch · 2022-11-25T09:59:29Z

Did some updates:

Now stating that structs in parent branches must be addressed by absolute path
Now stating that structs cannot be defined within another struct
Proposing that struct must be defined before usage
Proposing that inline/anonymous structs shall not be allowed (could be supported later if wanted)
Proposing that allowed shall not be allowed for struct type signals (but for items)
Proposing that default shall not be allowed.

UlfBj · 2022-11-25T13:02:51Z

Proposing that struct must be defined before usage

What is "before" in a tree structure?

erikbosch · 2022-11-25T14:55:57Z

Proposing that struct must be defined before usage

What is "before" in a tree structure?

Can you not have an ordered list of children in a tree? Supporting arbitrary order might complicate implementation of struct support in vss-tools, and does at least need to be considered during implementation and testing. If we have a majority that thinks that order should not matter I have no problems with it.

UlfBj · 2022-11-27T10:44:35Z

Can you not have an ordered list of children in a tree?

I do not think that is what most libs that can be used for this sort of tasks support.
And it would not make much difference anyway, since a parser must first access the node that contains the instantiation of the struct before the parser knows what struct definition to search for. So it will require two passes.
I think we are on a wrong track here, if one sees "simplicitity" as a valued feature of the VSS model, then this is not the way to go.

UlfBj · 2022-11-28T13:23:52Z

struct.md

+
+### Order of declaration/definition
+
+The struct type must be defined before it is used.


This requirement makes it impossible to reuse the struct definition in parts of the tree that lies before the definition.

If we consider it a too big limitation we shall remove it, lets discuss on the meeting tomorrow

Meeting discussion:

Ulf - should we have a separate branch on "top" like "Vehicle.DataDefinition.*" and require all struct definitions to be there? Makes tree cleaner, could obstruct the view.

Krishna - having two passes is easy, order shall not matter in tree structure. Having dedicated branch can give problem, current proposal allows to reuse name. Having a separate top branch then we might need to replicate tree structure.

Erik: Do you want the use of a common top data folder to be a VSS style guide or something specified in syntax?

Ulf: In syntax, reserved word

Sebastian/Ulf/Krishna: use separate file for structs

Erik: That means inline structs cannot be supported.

Erik: What prefix to use for types

Adnan: We need to discuss limitations so that people does not miss-use structs

Proposed way forward:

Erik to adapt struct.md based on decision that structs shall be defined in a separate file and that tooling shall get two params (vspec struct file and vspec signal file)

Erik (also) to propose/prepare "soft" limitations/recommendation, i.e. recommendations on when to use struct and when we from VSS will accept structs being added to VSS

struct.md

kkoppolu1 · 2022-12-07T00:24:25Z

struct.md

+
+For now, two ways of referring to a type shall be considered correct:
+
+* Reference by (leaf) name to a struct definition within a branch with the same name in the type tree


This is no longer valid with the approach of hosting types in a separate tree right?

There could be two scenarios where this could be relevant:

Internal references within type tree

This is valid and makes sense, that item B.B below can refer to type A within the same file without the need to the full path. I.e. supporting just using leaf name if referring to a type in the same branch in the same tree.

Example - Parser finds that B.B refers to type A. It will then check the branch containing struct B and find struct A and use it.

Struct A Item A.A Item A.B Struct B Item B.A Item B.B: Datatype: A

Same branch names in signal and type tree

This is more of a corner case. If you use the same branch structure in signal tree and type tree, and you from signal tree refer to a type in a branch with the same name in the type tree - shall that be allowed?

Example - if signal A.B.C refers to type D, shall the parser then check if there exist a type A.B.D in the type tree?

Or shall we restrict leaf-name lookup to only be within the same tree, i.e. practically allowing it only for the type tree?

Signal Tree:

Branch A Branch B Signal C, Datatype = D;

Type tree:

Branch A Branch B Struct D;

Hmm, this corner case makes me think that we need a dedicated root node name for types just like we have one for signals, namely, Vehicle

I agree on the need for a dedicated root node name for types.

struct.md

erikbosch · 2022-12-13T14:56:51Z

struct.md

+
+**TBD**:
+
+*Shall we keep `aggregate`, or remove it? If it is to be kept, does it need to be better defined to be able to argue why it is still needed? Do we know of anyone or any implementation actually using it?*


I have some difficulties fining a good argument why aggregateshall be kept, and if kept what it actually should mean.

Aggregate: To be used when a collection of sensors need to be treated as a unit (Existing definition)
Struct: To be used when a single sensor output is non-primitive/struct/higher-order/complex.

In essence, aggregate is used for grouping sensors just as today while struct is used for defining a single sensor.

If you want to deprecated aggregate, that means its existing utility is also not justifiable?

If someone sees a value in it I have no problems keeping it, but then I think the semantic meaning needs to be clarified. Or at least we need to discuss if it needs to be refined. What shall a tool/API/server do when it find a branch marked as "aggregate". Shall it be considered just as an advice to developers, or shall the tool actively refuse/decline reading or writing of individual signals in the branch. Do we see this as an area where VSS shall have an opinion, or do we leave it to the tool/server/API-implementation, i.e. one tool may decline individual signal requests for that branch, another tool may accept it.

Copying the comment from @UlfBj here:

If it is to be kept then I think a credible example shall be presented.

We have previously said that Vehicle.CurrentLocation could be a branch were aggregate=true could make sense. But one could then also argue that it is the same sensor that sets/publish lat/long/altitude, so that they would qualify for the criteria for being in a struct as well.

UlfBj · 2022-12-14T11:30:17Z

If it is to be kept then I think a credible example shall be presented.

…

On Wed, Dec 14, 2022, 12:27 Erik Jaegervall ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In struct.md <#511 (comment)> : > + + +### Structs vs. Aggregate + +VSS supports a keyword `aggregate` that can be used on branches to indicate that the branch preferably shall be read and written in atomic operations. +The keyword is however currently not used in the standard catalog, and it is not known if any implementation exists that actually consider it. +There have been criticism that `aggregate` changes the semantic meaning of branches and signals, i.e. that a signal is no longer handed as an independent object. +The exact meaning of `aggregate` is furthermore not well defined by VSS. Shall for example a write request (or update of sensor values) be rejected by an implementation +if not all signals in the branch are updated in the same operation. +Semantic interpretation is also ambiguous if the branch contains a mix of sensors, attributes and actuators. + +Using structs as datatype is better aligned with the view that VSS signals are independent objects, and the semantic ambiguities related to `aggregate` are not present for structs. + +**TBD**: + +*Shall we keep `aggregate`, or remove it? If it is to be kept, does it need to be better defined to be able to argue why it is still needed? Do we know of anyone or any implementation actually using it?* If someone sees a value in it I have no problems keeping it, but then I think the semantic meaning needs to be clarified. Or at least we need to discuss if it needs to be refined. What shall a tool/API/server do when it find a branch marked as "aggregate". Shall it be considered just as an advice to developers, or shall the tool actively refuse/decline reading or writing of individual signals in the branch. Do we see this as an area where VSS shall have an opinion, or do we leave it to the tool/server/API-implementation, i.e. one tool may decline individual signal requests for that branch, another tool may accept it. — Reply to this email directly, view it on GitHub <#511 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJERQQLXKRAXASDYTOFBLHTWNGVJXANCNFSM6AAAAAASGPSG54> . You are receiving this because you were mentioned.Message ID: ***@***.*** com>

erikbosch · 2023-01-10T15:29:36Z

Meeting notes: Krishna now unlocked. Will start looking at work packages, will discuss with Erik

erikbosch · 2023-01-30T11:28:06Z

Change this one to draft to indicate that it just represents working material, not intended for merge

erikbosch · 2023-02-24T10:01:14Z

Closing this one now.
Most info has been added to documentation.
Issues have been created in vss-tools for remaining tasks

Struct syntax proposal

0b6c83f

Note: This PR is not supposed to be merged as is, rather as a discussion platform for COVESA#326

slawr reviewed Nov 22, 2022

View reviewed changes

struct.md Outdated Show resolved Hide resolved

slawr reviewed Nov 22, 2022

View reviewed changes

struct.md Show resolved Hide resolved

erikbosch mentioned this pull request Nov 22, 2022

Potential support of structs in VSS w3c/automotive#461

Closed

kkoppolu1 reviewed Nov 22, 2022

View reviewed changes

Fix some review comments and add section on default values

d041c01

erikbosch commented Nov 23, 2022

View reviewed changes

Adapting to review comments

f2a1e06

Also other updates

erikbosch force-pushed the erikbosch/erik_struct branch from 6ed2166 to f2a1e06 Compare November 25, 2022 09:54

UlfBj reviewed Nov 28, 2022

View reviewed changes

SebastianSchildt mentioned this pull request Nov 29, 2022

Complex datatypes/structs in KUKSA.val eclipse/kuksa.val#422

Closed

Adapt to discussion in last vss meeting

87fcc3a

kkoppolu1 reviewed Dec 7, 2022

View reviewed changes

Update based on comments

2c7272c

erikbosch commented Dec 13, 2022

View reviewed changes

Give rationale for aggregate

a5d859a

erikbosch marked this pull request as draft January 30, 2023 11:28

This was referenced Feb 3, 2023

Topic: CommandLine COVESA/vss-tools#220

Merged

Topic: StructConstants COVESA/vss-tools#221

Merged

erikbosch mentioned this pull request Feb 22, 2023

Struct support - vspec2ttl COVESA/vss-tools#241

Open

erikbosch closed this Feb 24, 2023

erikbosch deleted the erikbosch/erik_struct branch September 29, 2023 09:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Struct syntax proposal #511

Struct syntax proposal #511

erikbosch commented Nov 21, 2022 •

edited

Loading

kkoppolu1 Nov 22, 2022

kkoppolu1 Nov 22, 2022

erikbosch Nov 23, 2022

kkoppolu1 Nov 22, 2022

erikbosch Nov 23, 2022

erikbosch Nov 23, 2022

erikbosch commented Nov 25, 2022

UlfBj commented Nov 25, 2022 •

edited

Loading

erikbosch commented Nov 25, 2022

UlfBj commented Nov 27, 2022

UlfBj Nov 28, 2022

erikbosch Nov 28, 2022

erikbosch Nov 29, 2022 •

edited

Loading

kkoppolu1 Dec 7, 2022

erikbosch Dec 7, 2022

kkoppolu1 Dec 13, 2022

UlfBj Jan 24, 2023

erikbosch Dec 13, 2022

kkoppolu1 Dec 14, 2022

erikbosch Dec 14, 2022

erikbosch Dec 14, 2022

erikbosch Dec 14, 2022

UlfBj commented Dec 14, 2022 via email

erikbosch commented Jan 10, 2023

erikbosch commented Jan 30, 2023

erikbosch commented Feb 24, 2023


		As the example above.

		TBD: Or do we want a more flexible approach, i.e. if you specify "ABC" as datatype, that a tool shall search upwards in all parent branches?


		TBD: Where shall the inner struct be defined? Shall it be allowed to define it within a struct as well, or does it need to be defined within a branch like above? If allowed to be defined within a struct, how do we want name resolution to work, only support exact (current) scope and absolute path, or a more flexible setup searching upwards.

		I.e. shall the following alternative style (where the struct `OpenHours` is defined within `DeliveryInfo`) be allowed or even preferred?


		### Order of declaration/definition

		The struct type must be defined before it is used.


		For now, two ways of referring to a type shall be considered correct:

		* Reference by (leaf) name to a struct definition within a branch with the same name in the type tree


		TBD:

		Shall we keep `aggregate`, or remove it? If it is to be kept, does it need to be better defined to be able to argue why it is still needed? Do we know of anyone or any implementation actually using it?

Struct syntax proposal #511

Struct syntax proposal #511

Conversation

erikbosch commented Nov 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikbosch commented Nov 25, 2022

UlfBj commented Nov 25, 2022 • edited Loading

erikbosch commented Nov 25, 2022

UlfBj commented Nov 27, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikbosch Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UlfBj commented Dec 14, 2022 via email

erikbosch commented Jan 10, 2023

erikbosch commented Jan 30, 2023

erikbosch commented Feb 24, 2023

erikbosch commented Nov 21, 2022 •

edited

Loading

UlfBj commented Nov 25, 2022 •

edited

Loading

erikbosch Nov 29, 2022 •

edited

Loading