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

Rust Vss-Parser (Lesson learn) #595

Open
fulup-bzh opened this issue May 7, 2023 · 5 comments
Open

Rust Vss-Parser (Lesson learn) #595

fulup-bzh opened this issue May 7, 2023 · 5 comments

Comments

@fulup-bzh
Copy link

fulup-bzh commented May 7, 2023

I just push my initial Rust vss-parser https://github.com/redpesk-labs/vss-covesa-rs . My goal it to build something equivalent to what I did for dbc/sockcan https://github.com/redpesk-labs/canbus-rs . Note that this code should remain under "work in progress" for an other couple of weeks.

Here after few lessons learn from my initial experience with vss specifications. Current vss documentation, is quite good. Nevertheless few points could be improved to reduce the initial cost of adaption.

  • Documentation few broken links, an automatic markdown crawler should fix this. (ex: data-type, struct link) within https://covesa.github.io/vehicle_signal_specification/rule_set/data_entry/sensor_actuator .
  • Signal Unit: it is not clear to me. Is 'units' is part of vss spec or not? I have some difficulty to understand how it could be possible to implement an interoperable data-model without normalizing units. I found units.yaml (is this the vss defacto spec ?)
  • Some hard to justify grammar:
    • datatype:uint16+arraysize:5 in place of uint16[5]. The problem is that arraysize and datatype are defined in two different lines (with no order). As VSS already support datatype:uint16[] why not systematically use the same syntax with unit16[xx] ?
    • unit: m/s^2 in place of unit: m/s[2]
    • some string use ["] others ['] when default/allowed type is inherited from signal datatype.
    • I find 'depreciated' (ex: FuelSystem.vspec:90) what are we support to do with this ?
  • Structure: I'm not sure to fully understand them. I found some embedded structure samples, but no clear explanation to get sure about how to smartly use them.

Missing feature for my project: Expect if vss-structure solve this, I did not find how to implement virtual signal. I have 'hardware structured' signals coming from DBC or NMEA2000 that I would like to map them on VSS. I also have signal units that depend on countries (ex: wind-speed in knots for France and in m/s for Finland). I was expecting to find something like:

dbc.canid.xxx (aggregated branch)
 - signal1
 - signal2(speed)
 - signal3(longitude)
 - signal4(latitude)
 - ...
 - signaln

Virtual signal

-- vpath: Vehicle.Vehicle.Speed  (Vehicle.vspec:186)
   type: sensor
   description: Vehicle speed.
   datatype: (dbc.canid.xxx.signal2)
-- vpath: Vehicle.Vehicle.CurrentLocation.Latitude  (Vehicle.vspec:425)
   type: sensor
   description: Current latitude of vehicle in WGS 84 geodetic coordinates, as measured at the position of GNSS receiver antenna.
   datatype: (dbc.canid.xxx.signal4)
   min: -90
-- vpath: Vehicle.Vehicle.CurrentLocation.Longitude  (Vehicle.vspec:433)
   type: sensor
   description: Current longitude of vehicle in WGS 84 geodetic coordinates, as measured at the position of GNSS receiver antenna.
   datatype: (dbc.canid.xxx.signal3)
   min: -180
-- vpath: Vehicle.Vehicle.CurrentLocation.Heading  (Vehicle.vspec:441)
   type: sensor
   description: Current heading relative to geographic north. 0 = North, 90 = East, 180 = South, 270 = West.
   datatype: double
   min: 0
@fulup-bzh
Copy link
Author

Does VSS expect enum to be processed as string ?

Following actuator is typed as "string" when in fact it is an enum, and that with no doubt the CAN message is going to be coded on 3 bits. Should the mapping be done in the overlay ? Sending and processing a string is far more expensive than processing an enum, it also leave more error opportunity in application code.

-- vpath: Vehicle.Powertrain.Transmission.Vehicle.Powertrain.Transmission.PerformanceMode  (Transmission.vspec:94)
   type: actuator
   description: Current gearbox performance mode.
   datatype: string
   allowed:
     NORMAL,
     SPORT,
     ECONOMY,
     SNOW,
     RAIN,

@fulup-bzh
Copy link
Author

fulup-bzh commented May 8, 2023

What about a pretty name for sensors ?

For client application, it would be nice to have a pretty name to display. Ideally in multiple local languages with the option to complete locale from overlay. I have no special opinion on syntax until it is simple to parse, but at least an English version of sensor name to display would be nice to have at main VSS spec level.

-- vpath: Lights.Brake.Lights.Brake.IsActive  (BrakeLights.vspec:6)
   pretty_name:  'stop light'
     - fr: 'feu de stop'
     - de: '....'
   type: actuator
   description: Indicates if break-light is active. ...
   datatype: string
   allowed:
     INACTIVE,
     ACTIVE,
     ADAPTIVE,

@erikbosch
Copy link
Collaborator

Some answers:

Detection of broken links in generated documentation

Would indeed be a good improvement. A pull request checking it as part of CI would be gladly accepted. Added #600 to follow up.

Deprecation

Described here: https://covesa.github.io/vehicle_signal_specification/rule_set/basics/#deprecation-since-version-21

The COVESA intention is to only do backward incompatible changes in major releases. So if we by any reason wants to rename a signal Vehicle.ABC to Vehicle.CDE and the next planned release is 4.1, then we can for v4.1 add Vehicle.CDE and add a deprecation note to Vehicle.ABC. Then as part of release preparations for v5.0 the signal Vehicle.ABC can be removed. COVESA does not specify what downstream projects shall do with the information. They could possibly give a warning if a deprecated signal is used.

Units

The file https://github.com/COVESA/vehicle_signal_specification/blob/master/spec/units.yaml contains the units used by the tree starting at https://github.com/COVESA/vehicle_signal_specification/blob/master/spec/VehicleSignalSpecification.vspec. To be able to use the tools https://github.com/COVESA/vss-tools you must specify both a *.vspec-file and a unit file. Examples used in CI can be seen in https://github.com/COVESA/vehicle_signal_specification/blob/master/Makefile.

And, yes we use style like m/s^2 and cm^3. Unfortunately this convention is not documented. We should preferably refer to some standard (or other authority) on how to write units, but I could not easily found one by a quick search. On the other hand I could not find any standard advocating m/s[2] as preferred representation for acceleration either. Added #598 to follow up.

Single or double quotes

We have a recommendation that single quotes shall be used for string literals. But it is not consistently used. In most cases it has no practical consequence - the *.vspec files are based on Yaml and there can be a slight difference in meaning between single and double quotes, but should for most of our usage not matter. Added #599 for follow up.

VSS Enum Handling

There have been long discussions in COVESA on enums. The official answer is: allowed is not the same as an enum. (But that does not prevent your implementation from treating/storing/transmitting them in some other format, like an enum or integer. Either by some form of implicit mapping like first item is 0, second 1 and so on, or some tool-specific overlay that specifies exact mapping)

Pretty names for signals

Here I could see two possible approaches for COVESA. One is to just define some keyword (with localization support) to be able to give a user friendly name for signals, the alternative is to actually also define user friendly names. There are ongoing discussions if VSS also need "short names" for signals, as the names of today for some platforms are too long, so it could possibly be related. We see similar requests to add more keywords now and then and we need to come up with a good way to handle coordination of them. It is being discussed as part of COVESA Data Expert Group ( see COVESA/data-expert-group#11 (restricted access))

@fulup-bzh
Copy link
Author

Thank you for your quick response, outside of "virtual signal", you address all my concerns. For COVESA expert group page, either I do not have access or the link is wrong, but I cannot access COVESA/data-expert-group#11

@erikbosch
Copy link
Collaborator

erikbosch commented May 9, 2023

Meeting notes:

  • Fulup: If interested please feel to test the parse, requests and pull requests welcome
  • Achim: unit conversion discussed also in AUTOSAR

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

2 participants