Skip to content

feat(units): Add QUDT unit and quantity kind correspondances#900

Open
jdacoello wants to merge 2 commits into
COVESA:masterfrom
jdacoello:feat/qudt-units
Open

feat(units): Add QUDT unit and quantity kind correspondances#900
jdacoello wants to merge 2 commits into
COVESA:masterfrom
jdacoello:feat/qudt-units

Conversation

@jdacoello
Copy link
Copy Markdown
Contributor

Adds external reference to unit model QUDT as explained in #828

For example:

the unit kilometer km points to its correspondence https://qudt.org/vocab/unit/KiloM which has lots of extra metadata like translated labels, and conversion formulas.

Comment thread spec/units.yaml Outdated
quantity: length
allowed-datatypes: ['numeric']
qudt-unit: http://qudt.org/vocab/unit/MilliM
qudt-quantity-kind: https://qudt.org/vocab/quantitykind/Length
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like it

From a tooling perspective it seems to be a bit superfluous to have both qudt-quantity-kind and quantity here. Maybe make quantityoptional if there is a qudt reference?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@sschleemilch @jdacoello - adding the qudt references can be done without changing vss-tools. But what do you think of changing validation logic so that only one of quantity and qudt-quantity-kind are required. I.e. quantity needed only if qudt-quantity-kind is not defined. That way we could limit our file https://github.com/COVESA/vehicle_signal_specification/blob/master/spec/quantities.yaml to only include "custom" quantities.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Fine for me. I can have a look.

Btw I would make it a bit more nested:

qudt:
  unit: ...
  kind: ...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added indentation as suggested

@erikbosch
Copy link
Copy Markdown
Collaborator

MoM:

  • D: Same unit can be used for multiple quantities
  • D: Makes sense to list at least one, most important to have unit reference
  • E: Please review/discussion

@erikbosch
Copy link
Copy Markdown
Collaborator

@jdacoello - could you update the markdown at https://github.com/COVESA/vehicle_signal_specification/blob/master/docs-gen/content/rule_set/data_entry/data_units.md#unit-file-syntax so that it mentions the two new fields as optional fields. Possibly also give an example.

@erikbosch
Copy link
Copy Markdown
Collaborator

MoM:

Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com>
Signed-off-by: JD Alvarez <8550265+jdacoello@users.noreply.github.com>
@erikbosch
Copy link
Copy Markdown
Collaborator

MoM:

  • Erik to a new look
  • D: Do you want to add checks to vSS-tools?
  • E: I can create a ticket and see if @sschleemilch have time to add checks
  • Ok to merge after maintainer approval

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

Successfully merging this pull request may close these issues.

3 participants