search

COVESA / vehicle_signal_specification

Vehicle Signal Specification - standardized way to describe automotive data
Mozilla Public License 2.0
319 stars 164 forks source link

UUIDs instead of id database #102

Closed danielwilms closed 5 years ago

danielwilms commented 5 years ago

As the process of generating the id database is not so transparent and fail proof, I would suggest to replace them with uuids, to have them truly unique and having a fixed length. To do so and not having an arbitrary string length as input the namespace system of uuid v5 could be used:

That can be done recursively to the leafs. I think it offers some advantages over the old system as mentioned above. Any opinion on this?

magnusfeuer commented 5 years ago

Just so I get it:

  1. Root UUID would be something like 491eb4c4-6772-11e9-9709-679fd5b85dd5

  2. Engine UUIID would be generated through: $ uuid -v5 491eb4c4-6772-11e9-9709-679fd5b85dd5 Engine 68a75eb6-8be0-56c9-86f0-e567beb37fc1 [Result is used as input in step 3.]

  3. Engine.RPM would be generated through: $ uuid -v5 68a75eb6-8be0-56c9-86f0-e567beb37fc1 Engine.RPM df1faf92-7d3f-590f-a206-778de3d6eb81

I really like the idea of a root UUID, which may even be different for each release, thus allowing a program to verify that they have loaded the correct version of a VSS spec. Taking this a step further, a vendor can create a new root UUID for a vendor-internal release that consists of the GENIVI spec + internal and proprietary extensions.

That said, I have two hesitations about UUIDs:

  1. We already have a way to use a string to uniquely identify a signal, which is the full path name.
  2. UUIDs are a lot heavier to hash on in a program when mapping UUID to an internal struct or object.

That said (again), I would be happy to update the tooling to support this, and retire the original int-based ID file.

My $0.02.

danielwilms commented 5 years ago

We already have a way to use a string to uniquely identify a signal, which is the full path name.

Yes, absolutely agreed. I got sometimes the complaint, that strings with varying length might not be so easy to handle in certain environments and that a fixed size unique identifier would help. I didn't want to replace the pathname as a primary identifier but just add another option.

magnusfeuer commented 5 years ago

Sounds good to me. Do you want me to open a branch and implement it?

danielwilms commented 5 years ago

@magnusfeuer, sorry I've overseen your comment. That would be fantastic!

magnusfeuer commented 5 years ago

Will do. Give me a week or so. Do you want me to retire the integer Signal IDs?

magnusfeuer commented 5 years ago

@danielwilms Starting on this now. I will remove Signal IDs since they can be implicitly created on both pub and sub side by:

  1. Sort all signals by their name or UUID.
  2. Use the position of the signal in the sorted list as a signal index.
magnusfeuer commented 5 years ago

@danielwilms https://github.com/GENIVI/vehicle_signal_specification/tree/magnusfeuer-issue-102

The generated CSV, fidl, and JSON files looks ok at a cursory glance, with UUID values having replaced integer IDs everywhere.

I cannot speak for the cnative format since I haven't read up on that particular subsystem. @UlfBj - Can you have a look at cnative on this branch to make sure it does what it is supposed to?

I will also add a vspec2hdr script that generates a C header file with all the signal specs.

I may also add a vspec2py that generates python code with the signal spec as wll.

UlfBj commented 5 years ago

@UlfBjhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FUlfBj&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C2b676261a5014e292a4a08d6d5ab6f69%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636931329616995992&sdata=ZYfnOnP%2FgRwBq3lOOaB96VKNp50Gw3wv7CN6ZH6QNCk%3D&reserved=0 - Can you have a look at cnative on this branch to make sure it does what it is supposed to? I will do so. Due to W3C f2f meeting and other assignments it will take some time, but I will do it.

BR Ulf

Ulf Björkengren Ph. D. Connectivity Strategist

M +4553562142 ulf.bjorkengren@volvocars.commailto:ulf.bjorkengren@volvocars.com

VOLVO CAR CORPORATION 94014 Lund R&D Tech Center Frederikskaj 10A Copenhagen, Denmark volvocars.com

From: Magnus Feuer [mailto:notifications@github.com] Sent: den 11 maj 2019 02:56 To: GENIVI/vehicle_signal_specification vehicle_signal_specification@noreply.github.com Cc: Björkengren, Ulf ulf.bjorkengren@volvocars.com; Mention mention@noreply.github.com Subject: Re: [GENIVI/vehicle_signal_specification] UUIDs instead of id database (#102)

@danielwilmshttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdanielwilms&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C2b676261a5014e292a4a08d6d5ab6f69%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636931329616975974&sdata=YgxZ%2FvZSyBsOsbqWWZks13JgIvB0M3TaPg9MyCDVJ6s%3D&reserved=0 https://github.com/GENIVI/vehicle_signal_specification/tree/magnusfeuer-issue-102https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FGENIVI%2Fvehicle_signal_specification%2Ftree%2Fmagnusfeuer-issue-102&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C2b676261a5014e292a4a08d6d5ab6f69%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636931329616985987&sdata=OZu86K5CJ1crfkR%2F7eCioE5ZZNuRYRJJaAI1SmWeZcE%3D&reserved=0

The generated CSV, fidl, and JSON files looks ok at a cursory glance, with UUID values having replaced integer IDs everywhere.

I cannot speak for the cnative format since I haven't read up on that particular subsystem. @UlfBjhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FUlfBj&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C2b676261a5014e292a4a08d6d5ab6f69%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636931329616995992&sdata=ZYfnOnP%2FgRwBq3lOOaB96VKNp50Gw3wv7CN6ZH6QNCk%3D&reserved=0 - Can you have a look at cnative on this branch to make sure it does what it is supposed to?

I will also add a vspec2hdr script that generates a C header file with all the signal specs.

I may also add a vspec2py that generates python code with the signal spec as wll.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FGENIVI%2Fvehicle_signal_specification%2Fissues%2F102%23issuecomment-491466097&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C2b676261a5014e292a4a08d6d5ab6f69%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636931329616995992&sdata=wQCwEgYJFRG%2B1rmoBXP1IzDVfQsEBVeTbVkApZdXY7c%3D&reserved=0, or mute the threadhttps://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAJERQQPL5M3NCOZ3FIQHAW3PUYKR7ANCNFSM4HHZDJNA&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C2b676261a5014e292a4a08d6d5ab6f69%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636931329617006000&sdata=RHQeQGgFJZjeVWn5Ikf061T%2BidF55ICtjJ3pXRjC41g%3D&reserved=0.

magnusfeuer commented 5 years ago

Added vspec2c.py that generates hdr and C file that encodes the entire signal spec and provides access functions.

Demo code that exercises the auto-generated code: https://github.com/GENIVI/vehicle_signal_specification/blob/magnusfeuer-issue-102/tools/vspec2c_demo/demo.c

At this point I am ready to merge into master @UlfBj @danielwilms - Are you ok with a Merge request? We will lose ID and get UUID.

UlfBj commented 5 years ago

Are you ok with a Merge request? We will lose ID and get UUID. It is fine with me.

magnusfeuer commented 5 years ago

Merged to master. I've tested the generated C/Hdr code extensively through an internal project, and it seems solid. Please ping me if you have issues and I will fix.

UlfBj commented 5 years ago

I cannot speak for the cnative format since I haven't read up on that particular subsystem. @UlfBj - Can you have a look at cnative on this branch to make sure it does what it is supposed to?

This is now fixed in the PR#108 "C-native tool updated with UUID support. "

magnusfeuer commented 5 years ago

Thank you, @UlfBj .

Merged.