From 0b6c83f48068fd2a427555a8ca5b1f423da46cee Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Mon, 21 Nov 2022 11:29:52 +0100 Subject: [PATCH 1/6] Struct syntax proposal Note: This PR is not supposed to be merged as is, rather as a discussion platform for #326 --- struct.md | 297 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 297 insertions(+) create mode 100644 struct.md diff --git a/struct.md b/struct.md new file mode 100644 index 000000000..ceecbfd98 --- /dev/null +++ b/struct.md @@ -0,0 +1,297 @@ +# Struct Proposal + +This document tries to define with examples what shall be (theoretically) supported in VSS after adding struct support. +I.e. what you can do still claiming that the model is correct VSS. +It only to a limited extent show implications for vss-tools, and then only for syntactic/semantic checks. +As of now it does not state how exporters are affected. + +## Proposed VSS 4.0 acceptance criteria and increments + +We need to decide how far we must go before we can release VSS 4.0, we does not need to go the whole way in one step + +### Increment 1 + +* Syntax can be used without VSS-tools complaining +* No semantic check by VSS-tools +* No documentation +* Not used in VSS standard catalog +* Struct not supported by VSS-tools exporters + + +### Increment 2 + +* Semantic check on type-references by VSS-tools +* Syntax documented +* Well defined behavior for all exporters (e.g. either support, and/or give warning if used) + +### Increment 3 + +* All "standard" exporters in vss-tools support structs +* Guidelines on when to consider using structs rather than branches documented + +### Increment 4 + +* We may start to use structs in VSS standard catalog. + + +## Simple Usage + +``` +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery + +DeliveryInfo.Address: + datatype: string + type: item + description: Destination address + +DeliveryInfo.Receiver: + datatype: string + type: item + description: Name of receiver + +DeliveryList: + datatype: DeliveryInfo + type: sensor + description: List of deliveries +``` + +For VSS 4.0 it is not necessary that vss-tools do semantic check, i.e. if someone would add an extra `f` by mistake like this: + +``` + +DeliveryList: + datatype: DeliveryInffo + type: sensor + description: List of deliveries +``` + +... then VSS-tools does not necessarily need to give an error (stretch goal to have semantic check that referred type exist). + +## Name resolution + +For now, two ways of referring to a type shall be considered correct: + +### Reference to a struct definition within same branch + +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? + +I.e. If a signal `A.B.C.D` is defined with type `X`, then the following priority order shall apply: + +* If `A.B.C.X` exists, then it will be used. +* 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 + +Reference by full path shall also be allowed. For now relative paths (e.g. `../Powertrain` shall not be supported). +But vss-tools does not need to resolve or verify that type reference is correct in VSS 4.0. + +``` + +DeliveryList: + datatype: Vehicle.Some.Branch.DeliveryInfo + type: sensor + description: List of deliveries +``` + +## Expectations on VSS implementations (e.g. VISS, KUKSA.val) + +It is expected of implementation (long-term) to support atomic read/write/subscribe of signals defined by struct. +They may support read of parts of signal, e.g. `DeliveryList.Receiver` + +## Array Support + +It shall be possible to specify that there shall be a struct of the array + + +``` +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery + +DeliveryInfo.Address: + datatype: string + type: item + description: Destination address + +DeliveryInfo.Receiver: + datatype: string + type: item + description: Name of receiver + +DeliveryList: + datatype: DeliveryInfo[] + type: sensor + description: List of deliveries +``` + + +### Expectations on VSS implementations (e.g. VISS, KUKSA.val) + +For array types (like above) VSS implementations may support several mechanisms + +* It is expected that they can support read/write/subscribe of the whole array, i.e. write all or read all in the same request +* They may optionally support additional operations like + * Writing/Reading a single instance, e.g. `DeliveryList[2]` (index mechanism is implementation dependent) + * Appending/Deleting individual instances + * Searching for instances with specific conditions. + +## Structure in Structure + +It shall be possible to refer to a structure type from within a structure + +``` +OpenHours: + type: struct + description: A struct type containing information on open hours + +OpenHours.Open: + datatype: uint8 + type: item + max: 24 + description: Time the address opens + +OpenHours.Close: + datatype: uint8 + type: item + max: 24 + description: Time the address close + +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery + +DeliveryInfo.Address: + datatype: string + type: item + description: Destination address + +DeliveryInfo.Receiver: + datatype: string + type: item + description: Name of receiver + +DeliveryInfo.Open: + datatype: OpenHours + type: item + description: When is receiver available + +DeliveryList: + datatype: DeliveryInfo + type: sensor + description: List of deliveries +``` + +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? + +``` + +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery + +DeliveryInfo.OpenHours: + type: struct + description: A struct type containing information on open hours + +DeliveryInfo.OpenHours.Open: + datatype: uint8 + type: item + max: 24 + description: Time the address opens + +DeliveryInfo.OpenHours.Close: + datatype: uint8 + type: item + max: 24 + description: Time the address close + +DeliveryInfo.Address: + datatype: string + type: item + description: Destination address + +DeliveryInfo.Receiver: + datatype: string + type: item + description: Name of receiver + +DeliveryInfo.Open: + datatype: OpenHours + type: item + description: When is receiver available + +DeliveryList: + datatype: DeliveryInfo + type: sensor + description: List of deliveries +``` + +## Inline Struct + +As an alternate approach we could consider supporting inline / anonymous structs + +``` + +DeliveryList: + datatype: struct[] + type: sensor + description: List of deliveries + +DeliveryList.Address: + datatype: string + type: item + description: Destination address + +DeliveryList.Receiver: + datatype: string + type: item + description: Name of receiver + +``` + +This could also work for struct in struct + +``` + + +DeliveryList: + datatype: struct[] + type: sensor + description: List of deliveries + +DeliveryList.Address: + datatype: string + type: item + description: Destination address + +DeliveryList.Receiver: + datatype: string + type: item + description: Name of receiver + +DeliveryInfo.Open: + datatype: struct + type: item + description: When is receiver available + +DeliveryInfo.Open.Open: + datatype: uint8 + type: item + max: 24 + description: Time the address opens + +DeliveryInfo.Open.Close: + datatype: uint8 + type: item + max: 24 + description: Time the address close + +``` + From d041c01ec3479263504365a806625e1ca298cf95 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 23 Nov 2022 10:54:02 +0100 Subject: [PATCH 2/6] Fix some review comments and add section on default values --- struct.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/struct.md b/struct.md index ceecbfd98..58b6c4495 100644 --- a/struct.md +++ b/struct.md @@ -63,6 +63,7 @@ For VSS 4.0 it is not necessary that vss-tools do semantic check, i.e. if someon DeliveryList: datatype: DeliveryInffo + comment: Note: Spelling error on line above, will only be detected if semantic check is implemented type: sensor description: List of deliveries ``` @@ -129,6 +130,17 @@ DeliveryList: description: List of deliveries ``` +By default the array has an arbitrary number of element and may be empty. +If a fixed size array is wanted the keyword `arraysize` can be used to specify size: + +``` +DeliveryList: + datatype: DeliveryInfo[] + arraysize: 5 + type: sensor + description: List of deliveries +``` + ### Expectations on VSS implementations (e.g. VISS, KUKSA.val) @@ -295,3 +307,28 @@ DeliveryInfo.Open.Close: ``` +## Default Values + +VSS supports default values for attributes, and there is a [discussion](https://github.com/COVESA/vehicle_signal_specification/issues/377) +to allow it also for sensors/actuators. For structs the following syntax shall be used + + +``` +{, < value of element 2>, ...} +``` + +Default values shall also be supported for arrays: + +``` +DeliveryList: + datatype: DeliveryInfo[] + type: attribute + default: [{'Munich','BMW'},{'Feuerbach','Bosch'}] + description: List of deliveries +``` + +TBD: How important do we see it to support default values for structs? So far we do not do any syntatic/semantic checks on default values, i.e. check that they are compatible with the used type. +I do not know if any exporter as of today do something "advanced" with the given default value. +If they just copy it as-is or ignores it then adding struct support would not be a big effort. +But translating it to something useful for the target format might be a bigger effort. + From f2a1e067fb91fbf2424959a36727986dec875067 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Fri, 25 Nov 2022 10:51:06 +0100 Subject: [PATCH 3/6] Adapting to review comments Also other updates --- struct.md | 243 ++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 174 insertions(+), 69 deletions(-) diff --git a/struct.md b/struct.md index 58b6c4495..4f7247d76 100644 --- a/struct.md +++ b/struct.md @@ -33,7 +33,6 @@ We need to decide how far we must go before we can release VSS 4.0, we does not * We may start to use structs in VSS standard catalog. - ## Simple Usage ``` @@ -51,21 +50,21 @@ DeliveryInfo.Receiver: type: item description: Name of receiver -DeliveryList: +Delivery: datatype: DeliveryInfo type: sensor - description: List of deliveries + description: Delivery ``` For VSS 4.0 it is not necessary that vss-tools do semantic check, i.e. if someone would add an extra `f` by mistake like this: ``` -DeliveryList: +Delivery: datatype: DeliveryInffo comment: Note: Spelling error on line above, will only be detected if semantic check is implemented type: sensor - description: List of deliveries + description: Delivery ``` ... then VSS-tools does not necessarily need to give an error (stretch goal to have semantic check that referred type exist). @@ -74,29 +73,89 @@ DeliveryList: For now, two ways of referring to a type shall be considered correct: -### Reference to a struct definition within same branch +* Reference by (leaf) name to a struct definition within same branch +* Reference by absolute path + +Relative paths (e.g. `../Powertrain.SomeStruct`) shall not be supported. +Structs in parent branches will not be visible, in those cases absolute path needs to be used instead + +Examples: + + +``` +A: + type: branch + description: Branch A. -As the example above. +A.DeliveryInfo: + type: struct + +A.DeliveryInfo.Address: + datatype: string + type: item + +A.DeliveryInfo.Receiver: + datatype: string + type: item + +A.Delivery1: + datatype: DeliveryInfo /* OK - As DeliveryInfo defined in same branch as Delivery1 */ + type: sensor -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? -I.e. If a signal `A.B.C.D` is defined with type `X`, then the following priority order shall apply: +A.Delivery2: + datatype: A.DeliveryInfo /* OK - Addressing using absolute path */ + type: sensor -* If `A.B.C.X` exists, then it will be used. -* Else if `A.B.X` exists, then it will be used. -* Else if `A.X` exists, then it will be used. +A.B: + type: branch -### Reference by absolute path +A.B.Delivery3: + datatype: DeliveryInfo /* ERROR - No DeliverInfo defined in branch A.B */ + type: sensor -Reference by full path shall also be allowed. For now relative paths (e.g. `../Powertrain` shall not be supported). -But vss-tools does not need to resolve or verify that type reference is correct in VSS 4.0. +A.B.Delivery4: + datatype: A.DeliveryInfo /* OK - Addressing using absolute path */ + type: sensor + +A.B.Deliver5: + datatype: ../DeliveryInfo /* ERROR - Relative paths not supported */ + type: sensor ``` -DeliveryList: - datatype: Vehicle.Some.Branch.DeliveryInfo +### Order of declaration/definition + +The struct type must be defined before it is used. + +**TBD: I think this makes it easier for our implementation, but the question is if we want this to be a requirement also in the long term** + +Example: + +``` +A: + type: branch + description: Branch A. + + +A.Delivery1: + datatype: DeliveryInfo /* ERROR - DeliveryInfo has not been defined yet! */ + type: sensor + +A.DeliveryInfo: + type: struct + +A.DeliveryInfo.Address: + datatype: string + type: item + +A.DeliveryInfo.Receiver: + datatype: string + type: item + +A.Delivery2: + datatype: DeliveryInfo /* OK - Now DeliveryInfo has been defined */ type: sensor - description: List of deliveries ``` ## Expectations on VSS implementations (e.g. VISS, KUKSA.val) @@ -192,58 +251,13 @@ DeliveryInfo.Open: type: item description: When is receiver available -DeliveryList: +Delivery: datatype: DeliveryInfo type: sensor - description: List of deliveries + description: Delivery ``` -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? - -``` - -DeliveryInfo: - type: struct - description: A struct type containing info for each delivery - -DeliveryInfo.OpenHours: - type: struct - description: A struct type containing information on open hours - -DeliveryInfo.OpenHours.Open: - datatype: uint8 - type: item - max: 24 - description: Time the address opens - -DeliveryInfo.OpenHours.Close: - datatype: uint8 - type: item - max: 24 - description: Time the address close - -DeliveryInfo.Address: - datatype: string - type: item - description: Destination address - -DeliveryInfo.Receiver: - datatype: string - type: item - description: Name of receiver - -DeliveryInfo.Open: - datatype: OpenHours - type: item - description: When is receiver available - -DeliveryList: - datatype: DeliveryInfo - type: sensor - description: List of deliveries -``` +For now it shall not be allowed to define a struct within a struct, all structs must be defined within a branch. ## Inline Struct @@ -306,18 +320,61 @@ DeliveryInfo.Open.Close: description: Time the address close ``` +**Proposal: For now inline/anonymous structs shall not be allowed! That could potentially be added later if needed** ## Default Values -VSS supports default values for attributes, and there is a [discussion](https://github.com/COVESA/vehicle_signal_specification/issues/377) -to allow it also for sensors/actuators. For structs the following syntax shall be used +VSS supports [default values for attributes](https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/attributes/), +and there is a [discussion](https://github.com/COVESA/vehicle_signal_specification/issues/377) +to allow it also for sensors/actuators. + +For structs it needs to be discussed if default values shall be given on the signal itself or on individual items. + +Example showing default values on items: + +``` +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery + +DeliveryInfo.Address: + datatype: string + type: item + default: 'Feuerbach' + description: Destination address + +DeliveryInfo.Receiver: + datatype: string + type: item + default: 'Bosch' + description: Name of receiver + +FirstDelivery: + datatype: DeliveryInfo + type: attribute + description: First delivery +``` + + + +For structs the following syntax could be used ``` {, < value of element 2>, ...} ``` +Example showing `default` on signal of struct type: -Default values shall also be supported for arrays: +``` +FirstDelivery: + datatype: DeliveryInfo + type: attribute + default: {'Munich','BMW'} + description: First delivery +``` + + +Default values could also be supported for arrays: ``` DeliveryList: @@ -332,3 +389,51 @@ I do not know if any exporter as of today do something "advanced" with the given If they just copy it as-is or ignores it then adding struct support would not be a big effort. But translating it to something useful for the target format might be a bigger effort. +**Proposal: It shall for now not be allowed to use default for signals of struct type or for items! ** + + +## Allowed Values + +VSS supports [specification of allowed values](https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/allowed/). +As of today it is theoretically supported for all datatypes, but there is an [issue](https://github.com/COVESA/vehicle_signal_specification/issues/502) +discussing if it is to be supported only for string data and possible integer-based types. + +Using `allowed` for `type: item` shall be supported (if `allowed` is supported for the used datatype). + +``` +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery + +DeliveryInfo.Address: + datatype: string + type: item + allowed: ['Munich','Feuerbach'] + description: Destination address + +DeliveryInfo.Receiver: + datatype: string + type: item + allowed: ['BMW','Bosch'] + description: Name of receiver + +DeliveryList: + datatype: DeliveryInfo[] + type: sensor + description: List of deliveries +``` + + +Theoretically `allowed` for signals of struct type could be supported if supported for all contained data types. +The example below follows the guidelines for [array types](https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/allowed/#allowed-values-for-array-types). +The usefulness could however be debated, and semantic check could be time consuming + +``` +DeliveryList: + datatype: DeliveryInfo[] + type: attribute + allowed: [{'Munich','BMW'},{'Feuerbach','Bosch'}] + description: List of deliveries +``` + +**Proposal: It shall for now not be allowed to use allowed for signals of struct type! (But allowed to use it for items)** From 87fcc3abcc5fb56b009a7a482e8b1cd00fbdace6 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Wed, 30 Nov 2022 13:49:46 +0100 Subject: [PATCH 4/6] Adapt to discussion in last vss meeting --- struct.md | 401 ++++++++++++++++++------------------------------------ 1 file changed, 136 insertions(+), 265 deletions(-) diff --git a/struct.md b/struct.md index 4f7247d76..956669a04 100644 --- a/struct.md +++ b/struct.md @@ -5,158 +5,141 @@ I.e. what you can do still claiming that the model is correct VSS. It only to a limited extent show implications for vss-tools, and then only for syntactic/semantic checks. As of now it does not state how exporters are affected. -## Proposed VSS 4.0 acceptance criteria and increments +## Rationale and Recommendations on intended usage -We need to decide how far we must go before we can release VSS 4.0, we does not need to go the whole way in one step +### Background -### Increment 1 +VSS currently supports only the following types: -* Syntax can be used without VSS-tools complaining -* No semantic check by VSS-tools -* No documentation -* Not used in VSS standard catalog -* Struct not supported by VSS-tools exporters +* Integer-based types (e.g. uint8, int32) +* Float-based types (float, double) +* String +* Boolean +In addition to this VSS supports arrays of the types given above. There are cases where this may not be sufficient. +Typical use-cases are when something cannot be described by a single value, but multiple values are needed. -### Increment 2 +A few hypothetical examples include: -* Semantic check on type-references by VSS-tools -* Syntax documented -* Well defined behavior for all exporters (e.g. either support, and/or give warning if used) +* GPS locations, where latitude and longitude must be handled together +* Obstacles - where each obstacle may contain information like, category, probability and location +* Errors/Warnings - where each item might contain information on category and priority -### Increment 3 +VSS supports a keyword `aggregate`that can be used on branches to indicate that the branch shall be read and written in atomic operations, +but that has not been considered sufficient and the semantic interpretation could be difficult if the branch contains a mix of sensors, attributes and actuators. -* All "standard" exporters in vss-tools support structs -* Guidelines on when to consider using structs rather than branches documented +### Intended usage -### Increment 4 +The proposed struct support in VSS is introduced to facilitate logical binding/grouping of data that originates from the same source. +It is intended to be used only when it is important that the data is read or written in an atomic operation. +It is not intended to be used to specify how data shall be packaged and serialized when transported. -* We may start to use structs in VSS standard catalog. +By this reason VSS-project will not introduce smaller datatypes (like `uint1`,`uint4`) to enable bit-encoding of data. +The order of elements in a struct is from a VSS perspective considered as arbitrary. +The VSS-project will by this reason not publish guidelines on how to order items in the struct to minimize size, +and no concept for introducing padding will exist. -## Simple Usage +Structs shall be used in VSS standard catalog only when considered to give a significant advantage compared to using only primitive types. -``` -DeliveryInfo: - type: struct - description: A struct type containing info for each delivery - -DeliveryInfo.Address: - datatype: string - type: item - description: Destination address +## General Idea -DeliveryInfo.Receiver: - datatype: string - type: item - description: Name of receiver +Structs shall be defined similar to VSS signals and branches. A struct must be defined within a branch and a struct contain of one or more items. -Delivery: - datatype: DeliveryInfo - type: sensor - description: Delivery -``` - -For VSS 4.0 it is not necessary that vss-tools do semantic check, i.e. if someone would add an extra `f` by mistake like this: +Structs shall be defined in a separate tree. This means that signal definitions and types cannot exist in the same files. +Tooling must thus be refactored to accept one (or more) parameters for specifying type definition(s), possibly with an argument like +`-t ./spec/vss_types.vspec` as in the example below: ``` - -Delivery: - datatype: DeliveryInffo - comment: Note: Spelling error on line above, will only be detected if semantic check is implemented - type: sensor - description: Delivery +./vss-tools/vspec2csv.py -I ./spec -t ./spec/vss_types.vspec ./spec/VehicleSignalSpecification.vspec my_output.csv ``` -... then VSS-tools does not necessarily need to give an error (stretch goal to have semantic check that referred type exist). +The top level types file (e.g. `vss_types.vspec`) can refer to other type files similar to the +[top VSS file](https://github.com/COVESA/vehicle_signal_specification/blob/master/spec/VehicleSignalSpecification.vspec). +Tooling may in the future support overlays for type declaration similar to how it is supported for signals. -## Name resolution +**TBD: What naming restriction shall apply** -For now, two ways of referring to a type shall be considered correct: +Shall it be allowed to use the same branch names in the type tree as in the signal tree, or must they be totally separated? +If we consider using standard VSS catalog where all signals resides in the `Vehicle` top branch, +is it then allowed to call the top-level in the type tree `Vehicle` as well? +That could be useful if we want to use paths like `Vehicle.Types.SomeType`for VSS standard catalog in the future. +Or shall it be required to have a totally separate tree, e.g. starting with `Types`? -* Reference by (leaf) name to a struct definition within same branch -* Reference by absolute path +Theoretically we could allow to have exactly the same branch structure in the signal files as in the type files and even reuse the same name (with full path) +as it anyway is clear from context whether we refer to a type or not. I.e. theoretically we could allow the signal `Vehicle.A.B` to refer to the struct `Vehicle.A.B` -Relative paths (e.g. `../Powertrain.SomeStruct`) shall not be supported. -Structs in parent branches will not be visible, in those cases absolute path needs to be used instead +*Alternative 1: Do not enforce any restrictions on syntactic/semantic level, i.e. tooling shall support any naming style of the types. This does not prevent us from agreeing that top level name shall be e.g. "Types" in the standard catalog* + +*Alternative 2: Require that top path for the type file must be "Types", i.e. tooling shall give error if another name is found!* -Examples: +## Simple Definition and Usage + +This could be a hypothetical content of a VSS type file ``` -A: +Types: type: branch - description: Branch A. - -A.DeliveryInfo: + +Types.DeliveryInfo: type: struct + description: A struct type containing info for each delivery -A.DeliveryInfo.Address: +Types.DeliveryInfo.Address: datatype: string type: item + description: Destination address -A.DeliveryInfo.Receiver: +Types.DeliveryInfo.Receiver: datatype: string type: item + description: Name of receiver +``` + +This struct definition could then be referenced from the VSS signal tree -A.Delivery1: - datatype: DeliveryInfo /* OK - As DeliveryInfo defined in same branch as Delivery1 */ +``` +Delivery: + datatype: Types.DeliveryInfo type: sensor +``` +For VSS 4.0 it is not necessary that vss-tools do semantic check, i.e. if someone would add an extra `f` by mistake like this: -A.Delivery2: - datatype: A.DeliveryInfo /* OK - Addressing using absolute path */ +``` +Delivery: + datatype: Types.DeliveryInffo + comment: Note: Spelling error on line above, will only be detected if semantic check is implemented type: sensor +``` -A.B: - type: branch +... then VSS-tools does not necessarily need to give an error (stretch goal to have semantic check that referred type exist). -A.B.Delivery3: - datatype: DeliveryInfo /* ERROR - No DeliverInfo defined in branch A.B */ - type: sensor +The type file may contain sub-branches and `#include`-statements just like regular VSS files -A.B.Delivery4: - datatype: A.DeliveryInfo /* OK - Addressing using absolute path */ - type: sensor +``` +Types: + type: branch -A.B.Deliver5: - datatype: ../DeliveryInfo /* ERROR - Relative paths not supported */ - type: sensor +Types.Powertrain: + type: branch + description: Powertrain types. +#include Powertrain/Powertrain.vspec Types.Powertrain ``` -### Order of declaration/definition - -The struct type must be defined before it is used. - -**TBD: I think this makes it easier for our implementation, but the question is if we want this to be a requirement also in the long term** +## Name resolution -Example: +For now, two ways of referring to a type shall be considered correct: -``` -A: - type: branch - description: Branch A. - - -A.Delivery1: - datatype: DeliveryInfo /* ERROR - DeliveryInfo has not been defined yet! */ - type: sensor +* Reference by (leaf) name to a struct definition within a branch with the same name in the type tree +* Reference by absolute path -A.DeliveryInfo: - type: struct - -A.DeliveryInfo.Address: - datatype: string - type: item +Relative paths (e.g. `../Powertrain.SomeStruct`) shall not be supported. +Structs in parent branches will not be visible, in those cases absolute path needs to be used instead. -A.DeliveryInfo.Receiver: - datatype: string - type: item +*The reference by leaf name is applicable only for structs referncing other structs, and for the case that the type branch has the same name/path as the signal branch, if allowed!* -A.Delivery2: - datatype: DeliveryInfo /* OK - Now DeliveryInfo has been defined */ - type: sensor -``` ## Expectations on VSS implementations (e.g. VISS, KUKSA.val) @@ -169,22 +152,8 @@ It shall be possible to specify that there shall be a struct of the array ``` -DeliveryInfo: - type: struct - description: A struct type containing info for each delivery - -DeliveryInfo.Address: - datatype: string - type: item - description: Destination address - -DeliveryInfo.Receiver: - datatype: string - type: item - description: Name of receiver - DeliveryList: - datatype: DeliveryInfo[] + datatype: Types.DeliveryInfo[] type: sensor description: List of deliveries ``` @@ -194,7 +163,7 @@ If a fixed size array is wanted the keyword `arraysize` can be used to specify s ``` DeliveryList: - datatype: DeliveryInfo[] + datatype: Types.DeliveryInfo[] arraysize: 5 type: sensor description: List of deliveries @@ -210,7 +179,7 @@ For array types (like above) VSS implementations may support several mechanisms * Writing/Reading a single instance, e.g. `DeliveryList[2]` (index mechanism is implementation dependent) * Appending/Deleting individual instances * Searching for instances with specific conditions. - + ## Structure in Structure It shall be possible to refer to a structure type from within a structure @@ -251,76 +220,39 @@ DeliveryInfo.Open: type: item description: When is receiver available -Delivery: - datatype: DeliveryInfo - type: sensor - description: Delivery ``` -For now it shall not be allowed to define a struct within a struct, all structs must be defined within a branch. - -## Inline Struct +### Order of declaration/definition -As an alternate approach we could consider supporting inline / anonymous structs +The order of declaration/definition shall not matter. +As signals and types are defined in different trees this is a topic only for struct definitions referring to other struct definitions. +A hypothetical example is shown below. An item in the struct `DeliveryInfo` can refer to the struct `OpenHours` even if that struct +is defined further down in the same file. ``` +DeliveryInfo: + type: struct + description: A struct type containing info for each delivery -DeliveryList: - datatype: struct[] - type: sensor - description: List of deliveries - -DeliveryList.Address: - datatype: string - type: item - description: Destination address +... -DeliveryList.Receiver: - datatype: string +DeliveryInfo.Open: + datatype: OpenHours type: item - description: Name of receiver + description: When is receiver available -``` +OpenHours: + type: struct + description: A struct type containing information on open hours -This could also work for struct in struct +... ``` -DeliveryList: - datatype: struct[] - type: sensor - description: List of deliveries - -DeliveryList.Address: - datatype: string - type: item - description: Destination address - -DeliveryList.Receiver: - datatype: string - type: item - description: Name of receiver - -DeliveryInfo.Open: - datatype: struct - type: item - description: When is receiver available +## Inline Struct -DeliveryInfo.Open.Open: - datatype: uint8 - type: item - max: 24 - description: Time the address opens - -DeliveryInfo.Open.Close: - datatype: uint8 - type: item - max: 24 - description: Time the address close - -``` -**Proposal: For now inline/anonymous structs shall not be allowed! That could potentially be added later if needed** +Inline/anonymous structs shall not be supported! ## Default Values @@ -328,112 +260,51 @@ VSS supports [default values for attributes](https://covesa.github.io/vehicle_si and there is a [discussion](https://github.com/COVESA/vehicle_signal_specification/issues/377) to allow it also for sensors/actuators. -For structs it needs to be discussed if default values shall be given on the signal itself or on individual items. - -Example showing default values on items: +It is proposed for now that default values shall not be supported for signals of struct type. +This also mean that VSS does not need to specify notation for struct values. +An exception is arrays of struct-types, where "empty array", i.e. `[]` shall be supported as default value. -``` -DeliveryInfo: - type: struct - description: A struct type containing info for each delivery - -DeliveryInfo.Address: - datatype: string - type: item - default: 'Feuerbach' - description: Destination address - -DeliveryInfo.Receiver: - datatype: string - type: item - default: 'Bosch' - description: Name of receiver - -FirstDelivery: - datatype: DeliveryInfo - type: attribute - description: First delivery -``` - - - -For structs the following syntax could be used - - -``` -{, < value of element 2>, ...} -``` -Example showing `default` on signal of struct type: - -``` -FirstDelivery: - datatype: DeliveryInfo - type: attribute - default: {'Munich','BMW'} - description: First delivery -``` +## Allowed Values -Default values could also be supported for arrays: +VSS supports [specification of allowed values](https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/allowed/). +As of today it is theoretically supported for all datatypes, but there is an [issue](https://github.com/COVESA/vehicle_signal_specification/issues/502) +discussing if it is to be supported only for string data and possible integer-based types. -``` -DeliveryList: - datatype: DeliveryInfo[] - type: attribute - default: [{'Munich','BMW'},{'Feuerbach','Bosch'}] - description: List of deliveries -``` +Using `allowed` for `type: item` shall be allowed (if `allowed` is supported for the used datatype). +Using `allowed` for signals and items of struct type or array of struct type shall not be allowed. +Theoretically `allowed` for signals of struct type could be supported if supported for all contained da -TBD: How important do we see it to support default values for structs? So far we do not do any syntatic/semantic checks on default values, i.e. check that they are compatible with the used type. -I do not know if any exporter as of today do something "advanced" with the given default value. -If they just copy it as-is or ignores it then adding struct support would not be a big effort. -But translating it to something useful for the target format might be a bigger effort. -**Proposal: It shall for now not be allowed to use default for signals of struct type or for items! ** +## Proposed VSS 4.0 acceptance criteria and increments +It is proposed that introduction of struct support shall be performed in increments -## Allowed Values +### Increment 1 -VSS supports [specification of allowed values](https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/allowed/). -As of today it is theoretically supported for all datatypes, but there is an [issue](https://github.com/COVESA/vehicle_signal_specification/issues/502) -discussing if it is to be supported only for string data and possible integer-based types. +* VSS-tools adapted to accept struct data as input. +* Syntactical check of struct definitions implemented. +* VSS-tools accepts reference to struct types. +* No semantic check by VSS-tools. +* No documentation. +* Not used in VSS standard catalog. +* Struct not supported by VSS-tools exporters. -Using `allowed` for `type: item` shall be supported (if `allowed` is supported for the used datatype). -``` -DeliveryInfo: - type: struct - description: A struct type containing info for each delivery - -DeliveryInfo.Address: - datatype: string - type: item - allowed: ['Munich','Feuerbach'] - description: Destination address +### Increment 2 -DeliveryInfo.Receiver: - datatype: string - type: item - allowed: ['BMW','Bosch'] - description: Name of receiver +* Semantic check on type-references by VSS-tools. +* Syntax documented. +* Well defined behavior for all exporters (e.g. either support, and/or give warning if used). -DeliveryList: - datatype: DeliveryInfo[] - type: sensor - description: List of deliveries -``` +### Increment 3 +* All "standard" exporters in vss-tools support structs. +* Guidelines on when to consider using structs rather than branches documented. -Theoretically `allowed` for signals of struct type could be supported if supported for all contained data types. -The example below follows the guidelines for [array types](https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/allowed/#allowed-values-for-array-types). -The usefulness could however be debated, and semantic check could be time consuming +### Increment 4 -``` -DeliveryList: - datatype: DeliveryInfo[] - type: attribute - allowed: [{'Munich','BMW'},{'Feuerbach','Bosch'}] - description: List of deliveries -``` +* Struct support released as a new VSS major release. +* Downstream projects (VISS, VSSO, KUKSA.val) can start integrating struct support. +* We may start to use structs in VSS standard catalog. -**Proposal: It shall for now not be allowed to use allowed for signals of struct type! (But allowed to use it for items)** From 2c7272c73e88b45daff116eb9cc94954aa030435 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Fri, 9 Dec 2022 13:12:15 +0100 Subject: [PATCH 5/6] Update based on comments --- struct.md | 73 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/struct.md b/struct.md index 956669a04..7e7166262 100644 --- a/struct.md +++ b/struct.md @@ -25,9 +25,6 @@ A few hypothetical examples include: * Obstacles - where each obstacle may contain information like, category, probability and location * Errors/Warnings - where each item might contain information on category and priority -VSS supports a keyword `aggregate`that can be used on branches to indicate that the branch shall be read and written in atomic operations, -but that has not been considered sufficient and the semantic interpretation could be difficult if the branch contains a mix of sensors, attributes and actuators. - ### Intended usage The proposed struct support in VSS is introduced to facilitate logical binding/grouping of data that originates from the same source. @@ -41,9 +38,34 @@ and no concept for introducing padding will exist. Structs shall be used in VSS standard catalog only when considered to give a significant advantage compared to using only primitive types. -## General Idea -Structs shall be defined similar to VSS signals and branches. A struct must be defined within a branch and a struct contain of one or more items. +### 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?* + +## General Idea and Basic Semantics + +A signal of struct type shall be defined in the same way as other VSS signals, the only difference would be that instead of using a primitive type there shall be a reference to a struct datatype. +This means that structs can be used for all types of VSS signals (o.e. sensor, attribute and actuator). +If a signal of struct type is sent or received, VSS expects all included items to have valid values, i.e. all items are mandatory +For example, if a struct contains the items A, B and C - then it is expected that the sent signal contains value for all items. +If some items are considered optional then the value range of the items must be adapted to include values indicating "not available" or "undefined", +or additional items needs to be added to indicate which items that have valid values. + +VSS makes no assumption on how structs are transferred or stored by implementations. +It is however expected that they are read and written by atomic operations. +This means that the data storage shall be "locked" while the items of the struct are read, preventing changes to happen while reading/writing the items. Structs shall be defined in a separate tree. This means that signal definitions and types cannot exist in the same files. Tooling must thus be refactored to accept one (or more) parameters for specifying type definition(s), possibly with an argument like @@ -55,23 +77,13 @@ Tooling must thus be refactored to accept one (or more) parameters for specifyin The top level types file (e.g. `vss_types.vspec`) can refer to other type files similar to the [top VSS file](https://github.com/COVESA/vehicle_signal_specification/blob/master/spec/VehicleSignalSpecification.vspec). -Tooling may in the future support overlays for type declaration similar to how it is supported for signals. - -**TBD: What naming restriction shall apply** - -Shall it be allowed to use the same branch names in the type tree as in the signal tree, or must they be totally separated? -If we consider using standard VSS catalog where all signals resides in the `Vehicle` top branch, -is it then allowed to call the top-level in the type tree `Vehicle` as well? -That could be useful if we want to use paths like `Vehicle.Types.SomeType`for VSS standard catalog in the future. -Or shall it be required to have a totally separate tree, e.g. starting with `Types`? +Tooling may in the future support overlays for type declarations similar to how it is supported for signals. -Theoretically we could allow to have exactly the same branch structure in the signal files as in the type files and even reuse the same name (with full path) -as it anyway is clear from context whether we refer to a type or not. I.e. theoretically we could allow the signal `Vehicle.A.B` to refer to the struct `Vehicle.A.B` - -*Alternative 1: Do not enforce any restrictions on syntactic/semantic level, i.e. tooling shall support any naming style of the types. This does not prevent us from agreeing that top level name shall be e.g. "Types" in the standard catalog* - -*Alternative 2: Require that top path for the type file must be "Types", i.e. tooling shall give error if another name is found!* +## Naming Restrictions +The VSS syntax and tooling shall not enforce any restrictions on naming for the type tree. It may even use the same branch structure as the signal tree. +This means that it theoretically at the same time could exist both a signal `A.B.C` and a struct `A.B.C`. +This is not a problem as it always from context is clear whether a name refers to a signal or a type. ## Simple Definition and Usage @@ -132,14 +144,26 @@ Types.Powertrain: 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 +* Reference by (leaf) name to a struct definition within a branch with the same name in the type tree. * Reference by absolute path Relative paths (e.g. `../Powertrain.SomeStruct`) shall not be supported. Structs in parent branches will not be visible, in those cases absolute path needs to be used instead. -*The reference by leaf name is applicable only for structs referncing other structs, and for the case that the type branch has the same name/path as the signal branch, if allowed!* +*The reference by leaf name is applicable only for structs referencing other structs, and for the special case that the type branch has the same name/path as the signal branch!* + +Parsers shall first look for a matching type in a branch with the same name, and if not found consider the name given to be an absolute name. +Example: + +``` +A.B.C: + datatype: Types.D + type: sensor +``` + +The parser shall first check if a type `A.B.Types.D` exist in the type tree, and if so use it. +If not found it shall search for the type `Types.D` in the type tree. ## Expectations on VSS implementations (e.g. VISS, KUKSA.val) @@ -264,6 +288,8 @@ It is proposed for now that default values shall not be supported for signals of This also mean that VSS does not need to specify notation for struct values. An exception is arrays of struct-types, where "empty array", i.e. `[]` shall be supported as default value. +It shall be possible to define default values for items (unless the item is of struct type). +If all items of a struct type have default values, then a signal (or item) using the struct type is also considered to have a default value. ## Allowed Values @@ -273,7 +299,8 @@ discussing if it is to be supported only for string data and possible integer-ba Using `allowed` for `type: item` shall be allowed (if `allowed` is supported for the used datatype). Using `allowed` for signals and items of struct type or array of struct type shall not be allowed. -Theoretically `allowed` for signals of struct type could be supported if supported for all contained da + +(Theoretically `allowed` for signals of struct type could be supported if supported for all contained items, but that would require additional effort to define syntax and for tooling to parse specified values). ## Proposed VSS 4.0 acceptance criteria and increments From a5d859a551f157dc2934b763a0d2a86488d75125 Mon Sep 17 00:00:00 2001 From: Erik Jaegervall Date: Thu, 5 Jan 2023 09:06:56 +0100 Subject: [PATCH 6/6] Give rationale for aggregate --- struct.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/struct.md b/struct.md index 7e7166262..710a4303e 100644 --- a/struct.md +++ b/struct.md @@ -38,7 +38,6 @@ and no concept for introducing padding will exist. Structs shall be used in VSS standard catalog only when considered to give a significant advantage compared to using only primitive types. - ### 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. @@ -47,12 +46,12 @@ There have been criticism that `aggregate` changes the semantic meaning of branc 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?* +Aggregate could however be useful as information on deployment level. +It gives the possibility to indicate that in this particular deployment the signals in the branch shall be treated as an aggregate. +Exact meaning of the `aggregate` keyword is then deployment specific. +With this view, aggregate shall never be used in the standard catalog, but can be used in overlays for deployment-specific purposes. ## General Idea and Basic Semantics