[Accepted with Revisions] SDL 0255 - Enhance BodyInformation vehicle data #847

theresalech · 2019-10-16T15:18:15Z

Hello SDL community,

The review of "SDL 0255 - Enhance BodyInformation vehicle data" begins now and runs through October 22, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0255-Enhance-BodyInformation-vehicle-data.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

#847

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

Is the problem being addressed significant enough to warrant a change to SDL?
Does this proposal fit well with the feel and direction of SDL?
If you have used competitors with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
Please state explicitly whether you believe that the proposal should be accepted into SDL.

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you,
Theresa Lech

Program Manager - Livio
theresa@livio.io

joeygrover · 2019-10-21T20:28:39Z

1. I don't think References signal "____" is a very helpful description to have in the spec itself when it looks to deal with OEM specific signal names. I would recommend changing it to be descriptive of the parameter not the proprietary signal.

2. The original struct uses Driver|Passenger for front modules while using location for rear ones. The author's addition use only the locations. This seems like it will be a strong point of confusion for developers. Will Driver|Passenger by dynamic per vehicle to correlated with their actual respective locations for that version of car or just static of left or right as defined in the spec?

3. The author uses the terminology of (Right\Left)(Front|Rear) when describing door locations. I do not agree with this approach since we have created the grid system for identifying module positions in the car. The solution added here will only be applicable to 1-4 door vehicles. I'm not sure if we want to mix the functionality from RC into Vehicle data though and will welcome more conversation on the topic.

4. Speaking more to point 2, would it ever be useable to allow these modules to be controlled via RC? locking/unlocking doors seems standard, even opening the doors is possible on some vehicle makes.

5. More clarification is necessary for a few of the params. Is a hatchback hatch considered a trunk? Is the truck bed tailgate considered the trunk?

atiwari9 · 2019-10-22T12:45:04Z

@joeygrover

Agreed, i just did not want to change original param descriptions to avoid confusion. But since it is out here and we are going to update BodyInformation, these should be updated as well. #ToUpdate
No matter which approach is used between right/left or driver/passenger, app would need to know if vehicle is right/left hand drive to correctly understand the physical location of the doors. I kept the new signals as left/right for mapping purposes to signals. But i am OK with either approach. Additionally we should also include a Left/Right hand drive param so that app can logically relate driverr/passenger side doors with physical locations.
RC uses grid for climate zones, while doors on a vehicle are limited to front/rear so far. Generally we should future proof the API but i could not think of a production vehicle with more than 5 doors(4+1 rear liftgate). Even buses/vans do not have more than 4 doors. It makes sense to implement grid for seats but for doors a grid would be an overkill IMO. Looking forward to counter points on this.
Yes it is possible that there might be a need to control door related functions via RC. But RC and vehicle data can provide redundant information since not all apps would be RC apps and they still might need to access the door related vehicle data.
Usually hatchback hatch is considered a liftgate, but ofcourse these are terminologies and we do not need to have separate param for each term. Ideally a more generic name would be better to represent Liftgate/hatch/trunk/boot etc.

joeljfischer · 2019-10-22T15:28:22Z

I agree that we should not mix terminology here, it could turn out to be very confusing for a developer. I tried to look for Vehicle Data (VD) related information on the driver location, but it seems to only be available via Remote Control (RC) at the moment. That's probably a deficiency that we should remedy. We should either deprecate and replace the old ones to be front-left / front-right, or we should stick to driver / passenger.
This is where the problem manifests of not having seat related information available via VD. If the driver door is ajar / locked, a VD developer has no idea how to display a graphic of what is happening unless he grabs the user's location and guesses.

I think that ideally we would go with grids again here, but that would be a lot of work / change. I don't have a recommendation here, but I think that using driver / passenger without a way to specify their locations is a significant deficiency.

I also believe that the current setup is unsustainable. We need a more extensible API for doors / front / rear liftgates.

atiwari9 · 2019-10-22T16:27:49Z

@joeljfischer

We can stick to driver/passenger terminology, in addition we should also include a new param for right/left hand driving to that app can logically compute the physical position of the door.
This would be solved if we add a new param for right/left hand drive. Using that, app can determine physical positions for info graphic.

In general i agree that grids implementation would make this tricky. But take a look at proposal for SeatOccupancy which is NOT in review yet but would definitely have similar discussions. I believe it makes sense to discuss both of these together and collectively decide on grid implementation for Vehicle Data.

theresalech · 2019-10-23T15:12:33Z

As the 2019-10-22 Steering Committee meeting did not reach a quorum, a vote could not take place on this proposal. The proposal will remain in review until the next meeting on 2019-10-29.

theresalech · 2019-10-30T14:35:33Z

There was discussion about keeping the proposal for SeatOccupancy in mind when reviewing this proposal, with regards to implementing a grid system for doors. The Project Maintainer agreed that whichever approach (grid or otherwise) is determined best for Enhance BodyInformation should be applied to SeatOccupancy and other vehicle data proposals as well. The Steering Committee voted to keep this proposal in review to allow members and the Project Maintainer additional time to discuss.

Jack-Byrne · 2019-11-05T14:33:27Z

If we wanted to implement this proposal as a grid, here is an initial suggestion for what the mobile api could look like.

<enum name="DoorStatus" since="x.x">
    <element name="CLOSED"/>
    <element name="LOCKED"/>
    <element name="AJAR"/>
</enum>

<enum name="DoorType" since="x.x">
    <element name="DRIVER"/>
    <element name="PASSENGER"/>
    <element name="REAR_GATE"/>
    <element name="FRONT_GATE"/>
</enum>

<struct name="Door" since="x.x">
    <param name="grid" type="Grid"  mandatory="false">
        <description>Grid location of a door. Value corresponds to a seat/module location</description>
    </param>
    <param name="status" type="DoorStatus" mandatory="false"/>
    <param name="doorType" type="DoorType" mandatory="false"/>
</struct>


<struct name="BodyInformation" since="x.x">
    <param name="doors" type="Door" array="true"/>
</struct>

I am unsure if DoorType is necessary, but I thought it would help make the door struct more descriptive.

I am open to suggestions for this type of implementation.

atiwari9 · 2019-11-05T14:42:41Z

I was writing this as well, here is what i came up with:

Below are the potential changes for grid implementation for BodyInformation, similar approach can be used for SeatOccupancy as well.

New Struct gridItemStatus is needed:

<struct name="gridItemStatus">
    <description>Describes the Status of a parameter of grid item.</description>
    <param name="itemLocation" type="Grid"  mandatory="true">
	<param name="conditionActive" type="Boolean"  mandatory="true">
    </param>
</struct>

Then BodyInformation Struct would be updated as:

<struct name="BodyInformation" since="2.0">
	<param name="parkBrakeActive" type="Boolean" mandatory="true">
		<description>References signal "PrkBrkActv_B_Actl".</description>
	</param>
	<param name="ignitionStableStatus" type="IgnitionStableStatus" mandatory="true">
		<description>References signal "Ignition_Switch_Stable". See IgnitionStableStatus.</description>
	</param>
	<param name="ignitionStatus" type="IgnitionStatus" mandatory="true">
		<description>References signal "Ignition_status". See IgnitionStatus.</description>
	</param>
	<param name="driverDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatDrv_B_Actl".</description>
	</param>
	<param name="passengerDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatPsngr_B_Actl".</description>
	</param>
	<param name="rearLeftDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatRl_B_Actl".</description>
	</param>
	<param name="rearRightDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatRr_B_Actl".</description>
	</param>
+	<param name="doorAjar" type="gridItemStatus" array="true" minsize=0 maxsize=100 mandatory="false" since="X.x">
+		<description>Provides status for doors if Ajar or not. true if locked</description>
+	</param>	
+	<param name="trunkAjar" type="Boolean" mandatory="false" since="X.x">
+		<description>true if vehicle trunk is ajar, else false</description>
+	</param>
+	<param name="hoodAjar" type="Boolean" mandatory="false" since="X.x">
+		<description>true if vehicle hood is ajar, else false</description>
+	</param>
+	<param name="doorLocked" type="gridItemStatus" array="true" minsize=0 maxsize=100 mandatory="false" since="X.x">
+		<description>Provides status for doors if Locked or not. true if locked</description>
+	</param>
+	<param name="trunkLocked" type="Boolean" mandatory="false" since="X.x">
+		<description>true if trunk is locked, else false</description>
+	</param>
</struct>

atiwari9 · 2019-11-05T14:59:15Z

@JackLivio -

I am unsure if DoorType is necessary, but I thought it would help make the door struct more descriptive.

DoorType struct would limit what HMI can name a door as. If app has grid information and item reference for a grid, it can itself determine what type of door the information is about. The names might change with OEM's implementation and future car designs. It'd just be one extra item for us to maintain.

I do like that you have consolidated Door Lock/Ajar status in to one and it does make sense as if Door is Ajar then it can't be locked. But if i incorporate that then i can't reuse gridItemStatus struct for SeatOccupancy. Sample implementation for Doors with single param for Locked/Ajar would be:

New Struct DoorsStatus is needed:

<struct name="DoorsStatus">
    <description>Describes the Status of a parameter of grid item.</description>
    <param name="itemLocation" type="Grid"  mandatory="true">
	<param name="doorStatus" type="DoorStatus"  mandatory="true">
    </param>
</struct>

New enum DoorStatus is needed:

<enum name="DoorStatus" since="x.x">
    <element name="CLOSED"/>
    <element name="LOCKED"/>
    <element name="AJAR"/>
</enum>

Then BodyInformation Struct would be updated as:

<struct name="BodyInformation" since="2.0">
	<param name="parkBrakeActive" type="Boolean" mandatory="true">
		<description>References signal "PrkBrkActv_B_Actl".</description>
	</param>
	<param name="ignitionStableStatus" type="IgnitionStableStatus" mandatory="true">
		<description>References signal "Ignition_Switch_Stable". See IgnitionStableStatus.</description>
	</param>
	<param name="ignitionStatus" type="IgnitionStatus" mandatory="true">
		<description>References signal "Ignition_status". See IgnitionStatus.</description>
	</param>
	<param name="driverDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatDrv_B_Actl".</description>
	</param>
	<param name="passengerDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatPsngr_B_Actl".</description>
	</param>
	<param name="rearLeftDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatRl_B_Actl".</description>
	</param>
	<param name="rearRightDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatRr_B_Actl".</description>
	</param>
+	<param name="doorsStatus" type="doorsStatus" array="true" minsize=0 maxsize=100 mandatory="false" since="X.x">
+		<description>Provides status for doors if Ajar or not. true if locked</description>
+	</param>	
+	<param name="trunkAjar" type="Boolean" mandatory="false" since="X.x">
+		<description>true if vehicle trunk is ajar, else false</description>
+	</param>
+	<param name="hoodAjar" type="Boolean" mandatory="false" since="X.x">
+		<description>true if vehicle hood is ajar, else false</description>
+	</param>
+	<param name="trunkLocked" type="Boolean" mandatory="false" since="X.x">
+		<description>true if trunk is locked, else false</description>
+	</param>
</struct>

joeljfischer · 2019-11-05T16:40:52Z

I think the last proposal is the closest, but I still don't like the trunkAjar / hoodAjar. It seems to assume that those are the only options. What if the vehicle has a trunk and a frunk (front trunk)? Would we have to reuse hood? That's not as semantically correct. Why not just assume that the trunk / hood are doors? Or an alternate similar array to the doorsStatus array.

atiwari9 · 2019-11-05T19:33:34Z

@joeljfischer For app's perspective, It does not matter if it is a hood or a trunk, Though at some places semantics do matter but most of People still call EV's front trunk a hood. And there are some new body types which have a side trunk as well(Rivian). We can consolidate these however to a single param as:

New Struct DoorsStatus is needed:

<struct name="DoorsStatus">
	<description>Describes the Status of a parameter of door.</description>
	<param name="doorLocation" type="Grid"  mandatory="true"/>
	<param name="doorStatus" type="DoorStatus"  mandatory="true"/>
</struct>

New enum DoorStatus is needed:

<enum name="DoorStatus" since="x.x">
	<element name="CLOSED"/>
	<element name="LOCKED"/>
	<element name="AJAR"/>
</enum>

New Struct GatesStatus is needed:

<struct name="GatesStatus">
	<description>Describes the Status of a parameter of trunk/hood/etc.</description>
	<param name="gateType" type="GateType"  mandatory="true"/>
	<param name="gateStatus" type="DoorStatus"  mandatory="true"/>
</struct>

New enum GateType is needed:

<enum name="GateType" since="x.x">
	<element name="FRONT"/>
	<element name="BACK"/>
	<element name="RIGHT"/>
	<element name="LEFT"/>
</enum>

Then BodyInformation Struct would be updated as:

<struct name="BodyInformation" since="2.0">
	<param name="parkBrakeActive" type="Boolean" mandatory="true">
		<description>References signal "PrkBrkActv_B_Actl".</description>
	</param>
	<param name="ignitionStableStatus" type="IgnitionStableStatus" mandatory="true">
		<description>References signal "Ignition_Switch_Stable". See IgnitionStableStatus.</description>
	</param>
	<param name="ignitionStatus" type="IgnitionStatus" mandatory="true">
		<description>References signal "Ignition_status". See IgnitionStatus.</description>
	</param>
	<param name="driverDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatDrv_B_Actl".</description>
	</param>
	<param name="passengerDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatPsngr_B_Actl".</description>
	</param>
	<param name="rearLeftDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatRl_B_Actl".</description>
	</param>
	<param name="rearRightDoorAjar" type="Boolean" mandatory="false" deprecated="true" since="X.x">
		<description>References signal "DrStatRr_B_Actl".</description>
	</param>
+	<param name="doorsStatus" type="doorsStatus" array="true" minsize=0 maxsize=100 mandatory="false" since="X.x">
+		<description>Provides status for doors if Ajar/Closed/Locked</description>
+	</param>	
+	<param name="gatesStatus" type="GatesStatus" array="true" minsize=0 maxsize=100 mandatory="false" since="X.x">
+		<description>Provides status for trunk/hood/etc if Ajar/Closed/Locked</description>
+	</param>
</struct>

joeljfischer · 2019-11-05T19:41:26Z

That's better. The only scaling niggle that I could have with it is GateType since it doesn't use the Grid. What if there are more than one gate per side? I know that's unlikely, but I thought I'd mention it anyway.

Jack-Byrne · 2019-11-05T21:41:26Z

My suggestion would be to rename structs "DoorsStatus" to"Door" or "Doors", and "GatesStatus" to "Gate" or "Gates". Having two structs named so closely such as DoorStatus and DoorsStatus is not great for readability in my opinion.

atiwari9 · 2019-11-05T22:07:05Z

I felt the same, so my suggestion:

rename enum DoorStatus -> DoorStatusType

Just saying Doors is also a little vague IMO.

Jack-Byrne · 2019-11-05T22:15:26Z

Would DoorsStatus be renamed DoorStatus? The struct should represent a single instance of an item. On the param definition line where the struct is listed as an array, the param name could be called doorsStatus.

	<param name="doorsStatus" type="DoorStatus" array="true" minsize=0 maxsize=100 mandatory="false" since="X.x">
		<description>Provides status for doors if Ajar/Closed/Locked</description>
	</param>

theresalech · 2019-11-06T15:49:33Z

The Steering Committee voted to return this proposal for revisions. The revisions will include implementing the proposal as a grid, and rename DoorStatus to DoorStatusType and DoorsStatus to DoorStatus.

theresalech · 2019-11-13T16:52:22Z

The author has made Steering Committee-requested changes, and the revised proposal will now be under review until our next meeting on 2019-11-19.

joeljfischer · 2019-11-14T19:34:21Z

I have a few notes on the updated proposal:

1. I don't believe that the parameter names on the DoorStatus and GateStatus structs need to repeat "door" and "gate".

2. It's odd that DoorStatus.doorLocation uses Grid while GateStatus.gateType uses a limited enum. Why not make GateStatus.gateLocation and use Grid?

3. BodyInformation.doorsStatus should be doorStatuses and BodyInformation.gatesStatus should be gateStatuses. It's not a single status for multiple doors / gates, it's multiple statuses for multiple doors / gates.

atiwari9 · 2019-11-14T20:34:34Z

@joeljfischer

Ok #ToUpdate
You did mention this before, but given that it is unlikely that there are multiple hoods etc. , it should be OK to just use GateType.

Even though unlikely, if we still want to consider that some vehicle might have multiple side storages(say a bus), i'd recommend to extend the enum with such possibilities. If we still want to use grid for this, that'd be something like:

<struct name="GateStatus" since="X.x">
	<description>Describes the status of a parameter of trunk/hood/etc.</description>
	<param name="gateType" type="Common.GateType" mandatory="true"/>
        <param name="gateLocation" type="Common.Grid" mandatory="true"/>
	<param name="status" type="Common.DoorStatusType" mandatory="true"/>
</struct>

Ok #ToUpdate

theresalech · 2019-11-20T01:49:06Z

The Steering Committee voted to accept this proposal with the revisions suggested in this comment from the Project Maintainer. It was also noted that a future proposal could be submitted to add a Vehicle Data Manager to assist developers with retrieving vehicle data.

theresalech · 2019-11-20T01:49:53Z

@atiwari9 Please advise when a new PR has been entered to update the proposal to reflect the agreed upon revisions. I'll then merge the PR so the proposal is up to date, and enter issues in the respective repositories for implementation. Thanks!

jordynmackool · 2019-11-22T22:04:32Z

Revisions made on 2019-11-22 and implementation issues entered:

core
iOS
java suite
rpc
generic hmi
sdl_hmi

theresalech · 2020-10-21T19:45:22Z

JavaScript Suite Issue: smartdevicelink/sdl_javascript_suite#338

theresalech added core hmi in review ios java suite rpc labels Oct 16, 2019

theresalech changed the title ~~[In Review] SDL 0255 - Enhance BodyInformation vehicle data~~ [Returned for Revisions] SDL 0255 - Enhance BodyInformation vehicle data Nov 6, 2019

theresalech added returned for revisions and removed in review labels Nov 6, 2019

smartdevicelink locked and limited conversation to collaborators Nov 6, 2019

theresalech changed the title ~~[Returned for Revisions] SDL 0255 - Enhance BodyInformation vehicle data~~ [In Review] SDL 0255 - Enhance BodyInformation vehicle data Nov 13, 2019

theresalech added in review and removed returned for revisions labels Nov 13, 2019

smartdevicelink unlocked this conversation Nov 13, 2019

theresalech changed the title ~~[In Review] SDL 0255 - Enhance BodyInformation vehicle data~~ [Accepted with Revisions] SDL 0255 - Enhance BodyInformation vehicle data Nov 20, 2019

theresalech added accepted with revisions and removed in review labels Nov 20, 2019

theresalech closed this as completed Nov 20, 2019

smartdevicelink locked and limited conversation to collaborators Nov 20, 2019

theresalech added the javascript label Oct 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Accepted with Revisions] SDL 0255 - Enhance BodyInformation vehicle data #847

[Accepted with Revisions] SDL 0255 - Enhance BodyInformation vehicle data #847

theresalech commented Oct 16, 2019 •

edited

Loading

joeygrover commented Oct 21, 2019

atiwari9 commented Oct 22, 2019

joeljfischer commented Oct 22, 2019

atiwari9 commented Oct 22, 2019 •

edited

Loading

theresalech commented Oct 23, 2019

theresalech commented Oct 30, 2019

Jack-Byrne commented Nov 5, 2019 •

edited

Loading

atiwari9 commented Nov 5, 2019 •

edited

Loading

atiwari9 commented Nov 5, 2019

joeljfischer commented Nov 5, 2019

atiwari9 commented Nov 5, 2019

joeljfischer commented Nov 5, 2019

Jack-Byrne commented Nov 5, 2019 •

edited

Loading

atiwari9 commented Nov 5, 2019

Jack-Byrne commented Nov 5, 2019

theresalech commented Nov 6, 2019

theresalech commented Nov 13, 2019

joeljfischer commented Nov 14, 2019

atiwari9 commented Nov 14, 2019

theresalech commented Nov 20, 2019

theresalech commented Nov 20, 2019

jordynmackool commented Nov 22, 2019 •

edited

Loading

theresalech commented Oct 21, 2020

[Accepted with Revisions] SDL 0255 - Enhance BodyInformation vehicle data #847

[Accepted with Revisions] SDL 0255 - Enhance BodyInformation vehicle data #847

Comments

theresalech commented Oct 16, 2019 • edited Loading

joeygrover commented Oct 21, 2019

atiwari9 commented Oct 22, 2019

joeljfischer commented Oct 22, 2019

atiwari9 commented Oct 22, 2019 • edited Loading

theresalech commented Oct 23, 2019

theresalech commented Oct 30, 2019

Jack-Byrne commented Nov 5, 2019 • edited Loading

atiwari9 commented Nov 5, 2019 • edited Loading

atiwari9 commented Nov 5, 2019

joeljfischer commented Nov 5, 2019

atiwari9 commented Nov 5, 2019

joeljfischer commented Nov 5, 2019

Jack-Byrne commented Nov 5, 2019 • edited Loading

atiwari9 commented Nov 5, 2019

Jack-Byrne commented Nov 5, 2019

theresalech commented Nov 6, 2019

theresalech commented Nov 13, 2019

joeljfischer commented Nov 14, 2019

atiwari9 commented Nov 14, 2019

theresalech commented Nov 20, 2019

theresalech commented Nov 20, 2019

jordynmackool commented Nov 22, 2019 • edited Loading

theresalech commented Oct 21, 2020

theresalech commented Oct 16, 2019 •

edited

Loading

atiwari9 commented Oct 22, 2019 •

edited

Loading

Jack-Byrne commented Nov 5, 2019 •

edited

Loading

atiwari9 commented Nov 5, 2019 •

edited

Loading

Jack-Byrne commented Nov 5, 2019 •

edited

Loading

jordynmackool commented Nov 22, 2019 •

edited

Loading