COVESA / vehicle_signal_specification

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

Top nodes in VehicleSignalSpecification.vspec #84

Closed UlfBj closed 3 years ago

UlfBj commented 5 years ago

Under the root node Vehicle there is now, besides the Media node and the Private node, also also all the nodes like DriveTrain, Cabin, Body, ADAS, Chassis, and OBD. I think these should be pushed one level down, under a branch node that may have the name Car (from VIWI), or something else. The nodes mentioned above belong to the same logical domain, and should reside under a domain name node for these. It will be more comparable to the Media domain node, and further new domain nodes that might be added later.

danielwilms commented 5 years ago

In VSSo @klotzbenjamin uses the Vehicle branch as a root branch instead of .car, which IMO adds overhead, which is not really necessary. I really liked the design of Benjamin. As well Media compares well with the other branches as a solely "virtual" branch. So you have a VIWI example, where it would be needed? @klotzbenjamin: Any opinion on that?

UlfBj commented 5 years ago

I think Vehicle is a good name for the root.

I think Chassis Body, Drivetrain, ADAS, etc should be placed together under one branch node, as I see them logically to belong to the same domain. But as it is hard to find a good name for that domain, maybe that indicates it is not a coherent logical group. But this is more a matter of taste, I will not insist on my view.

What I would like to insist on, however, is that the inclusion shown below is changed.

include Vehicle/Vehicle.vspec Vehicle

This clutters the level directly under the root with nodes like IgnitionOnTime, etc. I think this should be gathered under a branch node, and not be directly under the root. The Vehicle.vspec file name is not so well chosen either, being the same as the root name.

BR Ulf

danielwilms commented 5 years ago

I think Chassis Body, Drivetrain, ADAS, etc should be placed together under one branch node, as I see them logically to belong to the same domain. But as it is hard to find a good name for that domain, maybe that indicates it is not a coherent logical group. But this is more a matter of taste, I will not insist on my view.

If we find a domain within vehicle to group them, we can of course discuss it. I just would really like to avoid duplications in the path like, vehicle -> car -> ... I second your comment, that I don't see the logical connection right now either.

What I would like to insist on, however, is that the inclusion shown below is changed.

include Vehicle/Vehicle.vspec Vehicle

This clutters the level directly under the root with nodes like IgnitionOnTime, etc. I think this should be gathered under a branch node, and not be directly under the root. The Vehicle.vspec file name is not so well chosen either, being the same as the root name.

Do I understand right, that you want to have on the first level branches only? It would be a good compromise IMO. I'll think of a naming proposal. If you're faster, go ahead :)

UlfBj commented 5 years ago

Do I understand right, that you want to have on the first level branches only?

Yes.

It would be a good compromise IMO. I'll think of a naming proposal. If you're faster, go ahead :)

Right now I do not have a proposal, but I will also think of it.

UlfBj commented 5 years ago

Here is my proposal:

In Vehicle.vspec the vehicle identification data is already under the branch VehicleIdentification.

The following set, under Vehicle signals, I propose that all, except AmbientTemperature and IsMoving, is put under a branch called Statistics.

The following sets Spacial acceleration and Spacial rotation, I propose is put under a branch called Inertia, where also isMoving goes.

The attributes at the end goes under the VehicleIdentification branch. The accelerationTime is questionable to me.

Where AmbientTemperature goes I do not know.

rtroncy commented 5 years ago

Some comments:

Here is my proposal: In Vehicle.vspec the vehicle identification data is already under the branch VehicleIdentification. The following set, under Vehicle signals, I propose that all, except AmbientTemperature and IsMoving, is put under a branch called Statistics.

Statistics? This seems a odd name for grouping vehicle signals. What did you mean exactly?

The following sets Spacial acceleration and Spacial rotation, I propose is put under a branch called Inertia, where also isMoving goes.

+1

UlfBj commented 5 years ago

Statistics? This seems a odd name for grouping vehicle signals. What did you mean exactly? Most of them are accumulated values over time, IdleTime, DriveTime, etc.

I see now that one breaking that pattern is Speed, so it should be put elsewhere. Statistics is maybe not the best name, ‘Accumulated time’ is probably better.

danielwilms commented 5 years ago

Statistics? This seems a odd name for grouping vehicle signals. What did you mean exactly? Most of them are accumulated values over time, IdleTime, DriveTime, etc.

I see now that one breaking that pattern is Speed, so it should be put elsewhere. Statistics is maybe not the best name, ‘Accumulated time’ is probably better.

How about Vehicle.TripRecorder? @rtroncy, @klotzbenjamin

UlfBj commented 5 years ago

How about Vehicle.TripRecorder? Better than Accumulated time.

UlfBj commented 5 years ago

Speed could go under Inertia (if we go with that).

rtroncy commented 5 years ago

How about Vehicle.TripRecorder? @rtroncy, @klotzbenjamin

You add a notion of Trip that does not really exist. When are those values being reset? When you switch on/off the engine? When the engine car is going in sleeping mode? Recorder is good as the underlying signals will indeed capture a phenomena that is accumulated over a period of time from my understanding. Again, what is the exhaustive list of signals that will be grouped under this branch?

UlfBj commented 5 years ago

They are found in the Vehicle.vspec file. https://github.com/GENIVI/vehicle_signal_specification/blob/master/spec/Vehicle/Vehicle.vspec

danielwilms commented 5 years ago

How about Vehicle.TripRecorder? @rtroncy, @klotzbenjamin

You add a notion of Trip that does not really exist. When are those values being reset? When you switch on/off the engine? When the engine car is going in sleeping mode? Recorder is good as the underlying signals will indeed capture a phenomena that is accumulated over a period of time from my understanding. Again, what is the exhaustive list of signals that will be grouped under this branch?

I like the proposal to use simply Vehicle.Recorder. Then I would put values like speed and ambientTemperature under it as well. First I thought about Vehicle.Status but that's maybe a bit too generic.

UlfBj commented 5 years ago

I like the proposal to use simply Vehicle.Recorder. +1

Then I would put values like speed and ambientTemperature under it as well. Speed I think is more suitable in Inertia.

klotzbenjamin commented 5 years ago

I like the proposal to use simply Vehicle.Recorder.

+1

danielwilms commented 5 years ago

Then I would put values like speed and ambientTemperature under it as well. Speed I think is more suitable in Inertia.

@UlfBj: is there any other signal you'd see there, besides speed?

UlfBj commented 5 years ago

Not in Vehicle.vspec. I have not checked other files.

Skickat från min Samsung Galaxy-smartphone.

-------- Originalmeddelande -------- Från: danielwilms notifications@github.com Datum: 2018-12-10 19:15 (GMT+01:00) Till: GENIVI/vehicle_signal_specification vehicle_signal_specification@noreply.github.com Kopia: "Björkengren, Ulf" ulf.bjorkengren@volvocars.com, Mention mention@noreply.github.com Rubrik: Re: [GENIVI/vehicle_signal_specification] Top nodes in VehicleSignalSpecification.vspec (#84)

Then I would put values like speed and ambientTemperature under it as well. Speed I think is more suitable in Inertia.

@UlfBjhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FUlfBj&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C7f09e742e68e4baf86bd08d65ecb751b%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636800625263651241&sdata=9yjJWi0%2B%2BuQov8s7jui74L2fpmPNa71IRkO%2BWHOTF8w%3D&reserved=0: is there any other signal you'd see there, besides speed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FGENIVI%2Fvehicle_signal_specification%2Fissues%2F84%23issuecomment-445916166&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C7f09e742e68e4baf86bd08d65ecb751b%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636800625263661254&sdata=Xim49wYRO9PRl%2Fx09iVxYuoVvoDmnSLfauO6OUhagY4%3D&reserved=0, or mute the threadhttps://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAkkYQRZguysghPILP53gyhYiZ2LVOoR2ks5u3qS8gaJpZM4ZIeKt&data=02%7C01%7Culf.bjorkengren%40volvocars.com%7C7f09e742e68e4baf86bd08d65ecb751b%7C81fa766ea34948678bf4ab35e250a08f%7C0%7C0%7C636800625263661254&sdata=qXKfIKH5%2Bv6V%2BQ17OKhRpz8Vfih8H9rwTzcQr6ustOw%3D&reserved=0.

UlfBj commented 5 years ago

@rtroncy :+1 for grouping Chassis Body, Drivetrain, ADAS and some more (what is the exhaustive list?) under a common branch name

After discussing internally about a possible common branch name, the best we could come up with was "VehicleData".

Our view was that even if it is not a clean homerun, it is an improvement compared to having all these branches directly under the Vehicle root node.

Comments are welcome:).

danielwilms commented 3 years ago

done.