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

SDL 0173 - Read Generic Network Signal data #502

Closed
theresalech opened this issue May 30, 2018 · 43 comments
Closed

SDL 0173 - Read Generic Network Signal data #502

theresalech opened this issue May 30, 2018 · 43 comments

Comments

@theresalech
Copy link
Contributor

theresalech commented May 30, 2018

Hello SDL community,

The review of "SDL 0173 - Read Generic Network Signal data" begins now and runs through February 19, 2019. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0173-Read-Generic-Network-Signal-data.md

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

#502

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

@theresalech theresalech changed the title SDL 0173 - Read Generic Network Signal data [In Review] SDL 0173 - Read Generic Network Signal data May 30, 2018
@joeljfischer
Copy link
Contributor

I have a few questions & notes on this.

  1. The introduction says To enable OEM proprietary mobile apps to read raw CAN/Network data by specifying uniquely identifiable network data attributes. However, I see nothing restricting developers from figuring out the network signal ids and using them. In other words, this seems very insecure.

  2. If this is not just for OEM applications, then I don't think the split between this fragmented, proprietary approach and the open approach of VehicleData is appropriate for SDL. This proposal provides no concrete examples of data and situations in which this approach is more appropriate than GetVehicleData outside of a generic "we could update without updating SDL," and "more data."

I believe this proposal has adequately described a solution, but not a problem significant enough to warrant acceptance at this time. If this is just for OEM applications, it needs built-in safeguards to make sure that we're not soft-deprecating the open GetVehicleData in favor of a proprietary version.

@atiwari9
Copy link
Contributor

atiwari9 commented Jun 1, 2018

If this is not just for OEM applications, then I don't think the split between this fragmented, proprietary approach and the open approach of VehicleData is appropriate for SDL. This proposal provides no concrete examples of data and situations in which this approach is more appropriate than GetVehicleData outside of a generic "we could update without updating SDL," and "more data."

This solution is only for OEM proprietary apps. We want to enable OEM apps with access to wider set of vehicle data which might not necessarily be useful for 3rd party apps. Also the motivation behind this is to provide quick implementation for OEM apps for any new feature set it wants to add. If a particular vehicle data item could be utilized for 3rd party apps as well, then that would be implemented in GetVehicleData API itself. OEM apps should NOT use this API to retrieve data set which is available via GetVehicleData.

The introduction says To enable OEM proprietary mobile apps to read raw CAN/Network data by specifying uniquely identifiable network data attributes. However, I see nothing restricting developers from figuring out the network signal ids and using them. In other words, this seems very insecure.

Policies would prevent unauthorized access to this API, does this answer the question?

@Jack-Byrne
Copy link
Contributor

@atiwari9 The proposal does not mention rate limiting at all, do you have any information on how often you would expect the OEM app to receive CAN network updates? If the CAN data updates frequently and the OEM app is subscribed to a lot of different types of CAN network data I can see this clogging up Core's transports and affecting the performance of other SDL Apps.

@atiwari9
Copy link
Contributor

atiwari9 commented Jun 5, 2018

@JackLivio - I have kept the update frequency out of the proposal on purpose. It should be left to OEM to decide what is best suited for their app and what the HW/SW can handle, There is a provision to send String params in networkDataAttribute which can be used to define additional attributes like BUS, Resolution, offset, update frequency among others.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2018-06-12, to allow additional time for discussion on the issue. There was discussion during the call regarding the value of OEM-specific vehicle data and the flexibility available to OEMs with the new RPC suggested in the proposal versus the distinct branching of the vehicle data feature, the inability to expose the new method on a data set level, but only an all or nothing policy table entry, the burden placed on the app developer with this new RPC if exposed, and dis-incentivization of OEMs to continue to expose vehicle data through previous methods to developers limiting the expansion of the data set to developers.

It was agreed that before our next meeting, Steering Committee Representatives, especially OEM Members, should advise on vehicle data parameters they would like to see in SDL that are not currently exposed through the vehicle data RPC. The Steering Committee will then use that information to determine if a new API is needed for OEM-specific vehicle data, if it makes sense to add commonly requested vehicle data to the current vehicle data RPC, or both. Additionally, discussion should take place around possible alternatives or modifications to the new RPC proposed here, which could give policies more granular control on data sets to enable third party developer access and prevent closed door development changes to Core.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting to allow members more time to advise on vehicle data parameters they would like to see in SDL that are not currently exposed through the vehicle data RPC.

@atiwari9
Copy link
Contributor

Below is some more information on OEM specific use cases which potentially are not useful for 3rd party apps. I have also tried to address some of the concerns pointed out in last meeting. I invite everyone to put forward their points to carry on the discussion on this.

Why do we (& every OEM should) need this:

  • There are literally 1000’s of Vehicle Data Signals which an OEM would want to utilize for analysis/app features and services purposes
  • Some of the examples which could be related to every OEM:
    • Battery Performance
      • Battery Cell voltage of each cell, there are hundreds of such signals
      • Battery temperature of each probe
    • Charge related granular signals
      • There are 30 or so signals to setup vehicle charging profiles
    • Regenerative energy and vehicle efficiency signals
      • Brake Coach
      • Energy returned
      • Acceleration/Energy Consumed
    • Vehicle Diagnostic signals
      • Cluster Warnings etc
    • Vehicle Performance signals
      • Air/Fuel Ratio
        • O2 sensors for each head
      • Lateral acceleration
      • Gyroscope etc
  • To quickly provide access to vehicle data w/o waiting for change process to approve and add new vehicle data
  • To enable OEM app access to vehicle data items which
    • Are not so much useful to 3rd Party apps
      • E.g. we intend to deprecate below as these are NOT used by any of 3rd party apps and are too specific to Ford:
        • auxECallNotificationStatus
        • PowerMode
        • PowerModeQF
        • PowerMode_UB
        • CarMode
  • OEM may not want to share the specific vehicle data outside

Concerns:

  • Burden on the OEM side to process the signal data
    • Since the vehicle data in question in only usable by OEM, it is better to let OEM apps decode it. It saves unnecessary burden on SDL and OEM HMI implementation to maintain the logic to process large volume of vehicle data signals
    • Any data which can be utilized by 3rd party app would be moved to GetVehicleData API through proper channels
  • De-Incentivize OEM from maintaining existing GetVehicleData
    • OEMs would still support 3rd party apps which need access to feature rich vehicle data set as access to 3rd party apps can only be provided via existing GetVehicleData API. I believe every OEM understands the value GetVehicleData provides for OEM and 3rd party apps alike and hence would add commonly used vehicle data items in existing GetVehicleData API. We rather want to bring in new vehicle data items for GetVehicleData e.g.:
      • EV battery %
      • 12v Battery voltage/%
    • Additionally, OEMs could utilize other existing channels as well e.g. SystemRequest if the intention was to solely work on OEM specific data items.
    • So this does not De-Incentivize the OEM from maintaining GetVehicleData.
  • To expose policy table control at data set level
    • This is only makes sense for finite set of data items which can be utilized by broader set of apps. Motivation behind this proposal is to provide access to all the vehicle data items as and when need by OEM. To maintain granular control over all data items would need common mapping table to maintain 1000’s of data items. That is unnecessary burden on policy servers and would just clutter the API.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2018-06-26, to allow representatives additional time to review the author's comment and also to advise on vehicle data parameters they would like to see in SDL that are not currently exposed through the vehicle data RPC. It was requested that if representatives do not have any additional vehicle data parameters they would like to see, they should comment on this issue with that information, so that next steps on the proposal can be made informatively, with all members' needs taken into account.

@takaharuueno
Copy link

takaharuueno commented Jun 21, 2018

@atiwari9 Thank you for the information "why do we (& every OEM should) need this:". I understood proposed solution and additional information. We could use it for our proprietary mobile app. Currently we do not have the plan to access to additional vehicle data parameters for 3rd party apps. I hope my comment will be helpful.

@joeygrover
Copy link
Member

I've been trying to come up with how to handle this to be extensible and I don't believe I have all the details, but here is what I'm thinking. If we standardize a list of vehicle data types then each OEM could create a map of their custom signals to those data types. If the proxy libraries are passed this map it would be possible to create a more developer friendlier API based on the standardized data set, but be able to be customized based on the connected module as well as be updated over time. The map would only include what vehicle data items that app should have access.

There are two different methods to achieve this. Either the module obtains the map and passes it to the app, or the app obtains the list based on a URL supplied by the module.

For both we must first add a new system capability, EXTENDED_VEHICLE_DATA.

<struct name="SystemCapability">

...
	<param name="extendedVehicleDataCapability" type="ExtendedVehicleDataCapability" mandatory="false">
	    <description>Describes extended capabilities for vehicle data.</description>
    </param>
</strucct>

The policy table could have a new endpoint, extended_vehicle_data_url added that would inform it where the map was located.

"endpoints": {
    "0x07": {
        "default": ["http://x.x.x.x:3000/api/1/policies/proprietary"]
     },
     "0x04": {
         "default": ["http://x.x.x.x:3000/api/1/softwareUpdate"]
     },
     "queryAppsUrl": {
         "default": ["http://sdl.shaid.server"]
     },
     "lock_screen_icon_url": {
         "default": ["http://i.imgur.com/TgkvOIZ.png"]
     },
     "extended_vehicle_data_url": {
         "default": ["http://x.x.x.x:3000/api/1/vehicledata/"]
     }
}

App obtains list

<struct name="ExtendedVehicleDataCapability">
	<param name="url" type="String">
	    <description> URL to obtain vehicle data map. Downloaded document will contain map to form parse and construct requests for extended vehicle data</description>
	</param>
</struct>

The URL returned would be stored in the policy table to ensure it can be updated. It can also include a token if desired to allow the app to connect to the hosting server.

Module obtains list and passes to device

During a policy table update the module would retrieve the new vehicle data map. The process would mimic a policy table update in terms of module to proxy flow.

<struct name="ExtendedVehicleDataCapability">
	<param name="vehicleDataMap" type="JSONObject">
	    <description> An unverified JSON object that the client can parse to create a map</description>
	</param>
</struct>

The app would then receive the map directly from the core module.

Open item

The part that still needs to be flushed out is how to enforce policies over such a generic request as the one given in the original proposal; that is, beyond the all allowed or all denied method of the singular RPC. I believe it could still be done at a granular level, but functional groups would have to include a near custom module by module description of it's capabilities.

@joeygrover
Copy link
Member

Has the author had time to review my previous comment?

@mrapitis
Copy link
Contributor

@joeygrover we would like to follow the outcome from our previous steering committee meeting (as seen below). We will reach out to @theresalech to setup a technical meeting / discussion.

SDL 0173 - Read Generic Network Signal data
(#502)
Vote: Deferred (Keep in Review)
The author and project maintainer will plan to work through the details before
the next Steering Committee meeting. If other SDLC Members are interested in
joining the discussion ...

@theresalech
Copy link
Contributor Author

theresalech commented Jul 10, 2018

It looks like the Steering Committee's decision from the 2018-06-26 meeting did not get recorded. Please find below:

The author and project maintainer agreed that this is an important part of the platform that should be discussed in more detail before moving forward. The last comment from the project maintainer describes extending the feature further as a better vehicle data acquisition system. This system would allow dynamically uploading mapping from a policy server/backend system and deploying to the module itself (to be cached and pushed to client apps with access) or enabling the app through a URL to connect directly to the backend server to obtain that mapping. Ultimately, the SDLC is looking to achieve two things with the implementation of this feature: preserve the intention to expose as many vehicle data items to third parties and make sure those get integrated, and also provide a broader set of vehicle data items to OEMs that can be extended in an easier fashion to vehicles without necessarily predefining explicit pieces of data.

@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal, keeping it in review until our next meeting on 2018-07-17, so that the author and project maintainer can work through the details together offline.

@theresalech
Copy link
Contributor Author

Author and Project Maintainer discussed next steps offline. Author will be providing example CAN data for Project Maintainer to investigate ability to standardize into vehicle data sheets. Proposal will likely be returned for revisions once investigation is complete.

@theresalech theresalech changed the title [In Review] SDL 0173 - Read Generic Network Signal data [Deferred] SDL 0173 - Read Generic Network Signal data Jul 18, 2018
@theresalech
Copy link
Contributor Author

The Steering Committee voted to defer this proposal until the author and project maintainer are able to complete their investigation and suggest revisions.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Jul 18, 2018
@jordynmackool jordynmackool changed the title [Deferred] SDL 0173 - Read Generic Network Signal data SDL 0173 - Read Generic Network Signal data Dec 20, 2018
@smartdevicelink smartdevicelink unlocked this conversation Dec 20, 2018
@jordynmackool jordynmackool changed the title [Returned for Revisions] SDL 0173 - Read Generic Network Signal data SDL 0173 - Read Generic Network Signal data Feb 6, 2019
@jordynmackool
Copy link
Contributor

The requested revisions have been made. This review begins now and runs through February 12, 2019.

@smartdevicelink smartdevicelink unlocked this conversation Feb 6, 2019
@joeygrover
Copy link
Member

Before jumping into specifics, one thing I think this proposal is still missing is a bridge between older Core implementations and newer proxies to take advantage of the newly available OEM vehicle data. For example, if we define a new data type in the RPC spec that gets built into the libraries, it would be great to be able to allow that data retrieval from older head-units via this updated schema for the Core implementation.

A base JSON schema file for SDL core and proxy will be kept locally and an additional one in the OEM cloud.

These are two different schemas that should be kept seperate? Or should Core be able to update the schema it uses all together through the SystemRequest process and only store a single list?

How does the proxy library download this updated schema? Or will it only be available to OEM apps so it can be hardcoded with them?

reference

I believe a better name for this would be key or name. Reference seems like an odd choice here.

engineState would ideally be an enum if it was a Standardized data type. But since this it OEM example of vehicle data on the fly, we can use String data type.

What happens if an OEM data item later becomes a standardized data type? Would proxies have to ensure backwards compatibility? I would imagine no as this should only be exposed to OEM apps and they would be required to maintain that ability.

Access to data items would still be controlled using Policies the same way. OEM would need to add new vehicle data items to policies.

How? Where in Core could it be validated against this new schema? It seems like a completely new piece of the validation code in Core. It also seems like the policy table additions would have to be flushed out so the SDL Policy Server could facilitate this type of validation. @nickschwab @JackLivio any thoughts on this?

Once core has downloaded and processed the new vehicle data params, it'd send an onPermissionsChange notification to the connected app with new vehicle data params.

Is a permission change the most logical place to put this update? Couldn't we use the system capabilities? Per the app services proposal there will be a new feature to allow for OnSystemCapability notifications that an app subscribes. Also, the standard get method is always available.

Add getter and setter for generic vehicle data in GetVehicleData/SubscribeVehicleData/UnsubscribeVehicleData:

public void setGenericNetworkData(String vehicleDataName, Boolean vehicleDataState){
	setParameters(vehicleDataName, vehicleDataState);
}

public Boolean getGenericNetworkData(String vehicleDataName){
	return getBoolean(vehicleDataName);
}

Why are these methods passing in and returning Booleans? Shouldn't it be a generic Object?

Add getter and setter for generic vehicle data in SubscribeVehicleDataResponse/UnsubscribeVehicleDataResponse:

public void setGenericNetworkData(String vehicleDataName, VehicleDataResult vehicleDataState){
	setParameters(vehicleDataName, vehicleDataState);
}

public VehicleDataResult getGenericNetworkData(String vehicleDataName){
	return (VehicleDataResult) getObject(VehicleDataResult.class, vehicleDataName);
}

Is VehicleDataResult the right struct to use? If we don't have all the vehicle data types defined in the VehicleDataType there's no way this could provide meaningful data?

@atiwari9
Copy link
Contributor

@joeygrover

Before jumping into specifics, one thing I think this proposal is still missing is a bridge between older Core implementations and newer proxies to take advantage of the newly available OEM vehicle data. For example, if we define a new data type in the RPC spec that gets built into the libraries, it would be great to be able to allow that data retrieval from older head-units via this updated schema for the Core implementation.

Did you mean to be able to read newer vehicle data from older SDL core version? I do not see how that'd be possible without making changes to SDL core itself. core needs to support dynamically download and build the new vehicle data items to add support for new vehicle data on the go. Currently core only accepts GetVehicleData requests for vehicle data items defined in API.

These are two different schemas that should be kept seperate? Or should Core be able to update the schema it uses all together through the SystemRequest process and only store a single list?
How does the proxy library download this updated schema? Or will it only be available to OEM apps so it can be hardcoded with them?

Core and proxy would have a static schema stored locally which would have definitions for approved vehicle data items. Both the files are essentially the same and act as reference for supported vehicle data items before any downloaded content is applied. This vehicle data item would be available to all apps since it would be part of approved vehicle data items through SDL evol process.

I believe a better name for this would be key or name. Reference seems like an odd choice here.

Though it is just semantics, it is important to be clear about what a particular field means. Between key and name, i'd not prefere name as it is too generic. And key also might cause some ambiguity with hashmap references. But still better than name. To me reference is better still since this points to a definition in OEM side mapping table. But i am open to changing it to key as well.

What happens if an OEM data item later becomes a standardized data type? Would proxies have to ensure backwards compatibility? I would imagine no as this should only be exposed to OEM apps and they would be required to maintain that ability.

In case OEM specific data becomes standardized, proxy would implement specific getter/setter fields for that data. OEM app should still be able to read the data through specific or generic getter/setter methods since it'd still have the vehicle data definitions. In this case, it'd be in OEM app's interest to switch to specific getter/setter methods to save on data processing. But older OEM app versions won't break as proxy would still have generic getter/setters. So proxy will have both specific and generic implementations. (Note that Generic implementation at proxy is one time change)

How? Where in Core could it be validated against this new schema? It seems like a completely new piece of the validation code in Core. It also seems like the policy table additions would have to be flushed out so the SDL Policy Server could facilitate this type of validation. @nickschwab @JackLivio any thoughts on this?

After core has processed new vehicle data items after dynamically downloading the schema file, it'd know what new vehicle data items it supports. let's say engine_state. Now, PTU would also have this new vehicle data as part of app policies just like older vehicle data. Core's mechanism to check for permissions would be reused in this case. Only change at core side would be to process the new vehicle data as per downloaded schema during PTU process.

Is a permission change the most logical place to put this update? Couldn't we use the system capabilities? Per the app services proposal there will be a new feature to allow for OnSystemCapability notifications that an app subscribes. Also, the standard get method is always available.

Correct me if i am wrong, but vehicle data is NOT a SystemCapability item today.

<enum name="AppServiceType">
	 <element name = "MEDIA"/>
	 <element name = "WEATHER"/>
	 <element name = "NAVIGATION"/>
	 <element name = "VOICE_ASSISTANT"/>
	 <element name = "GENERIC"/>
	 <element name = "COMMUNICATION_VOIP"/> <!-- Currently no specific definitions -->
	 <element name = "MESSAGING"/>			<!-- Currently no specific definitions -->
 	 <element name = "TTS"/> 				<!-- Currently no specific definitions -->

</enum>

I believe onPermissionsChange is logical and would need nearly no changes to support this. Even today after PTU, onPermissionsChange is sent to the app with new RPCs/vehicleDataItems as per what is sent by the server. GetSystemCapability is designed to communicate what all services an app supports. In vehicle data use case, core already supports vehicle data and is just adding more vehicle data items. For and app, onPermissionsChange is more logical way to get these updates as even today apps check onPermissionsChange to find out if a particular rpc/param is allowed or not. I believe going the OnSytemCapability route would need more changes and work than necessary for this.

Why are these methods passing in and returning Booleans? Shouldn't it be a generic Object?

Get/Subscribe/UnSubscribe are Boolean as these are just requesting vehicle data. Refer to GetVehicleDataResponse/OnVehicleData methods for actual data return type.

Is VehicleDataResult the right struct to use? If we don't have all the vehicle data types defined in the VehicleDataType there's no way this could provide meaningful data?

Yes it is correct return type as it is used only for SubscribeVehicleDataResponse/UnsubscribeVehicleDataResponse which just tells us whether the Subscription/Unsubscription was successful or not. In case core does not understand request data, it would return Invalid_Data as per current implementation.

@joeygrover
Copy link
Member

Did you mean to be able to read newer vehicle data from older SDL core version? I do not see how that'd be possible without making changes to SDL core itself. core needs to support dynamically download and build the new vehicle data items to add support for new vehicle data on the go. Currently core only accepts GetVehicleData requests for vehicle data items defined in API.

I meant for Core versions that would implement this feature to be able to handle future vehicle data types that it didn't originally have defined. This is especially powerful considering the proxy libraries can be built out to support standard APIs for the data and developers get access to newly agreed upon vehicle data much quicker than the current cycle.

Core and proxy would have a static schema stored locally which would have definitions for approved vehicle data items. Both the files are essentially the same and act as reference for supported vehicle data items before any downloaded content is applied. This vehicle data item would be available to all apps since it would be part of approved vehicle data items through SDL evol process.

I think you missed my question here. Does core store both a static schema and a downloaded one? Or do they get merged? Or maybe just replaced? Does the proxy download the updated schema or is it just required to be hardcoded into the app? Should the schema be stored separately, merged together, or replaced if one is downloaded?

Though it is just semantics, it is important to be clear about what a particular field means. Between key and name, i'd not prefere name as it is too generic. And key also might cause some ambiguity with hashmap references. But still better than name. To me reference is better still since this points to a definition in OEM side mapping table. But i am open to changing it to key as well.

While some semantics are just that, it is important to use standard. industry accepted terms when possible. This helps keep the learning curve lower, as we do not force them to create a new mapping in their mind between our terms and the industry's. The standard convention to a Map entry is a key-value pair. There are other associated terms like name–value pair, field–value pair or attribute–value pair, but I have never come across it as a reference-value pair in terms of a map entry (I could be mistaken). I believe key is the correct term in this instance as it applies to a key value pair in Core.

In case OEM specific data becomes standardized, proxy would implement specific getter/setter fields for that data. OEM app should still be able to read the data through specific or generic getter/setter methods since it'd still have the vehicle data definitions. In this case, it'd be in OEM app's interest to switch to specific getter/setter methods to save on data processing. But older OEM app versions won't break as proxy would still have generic getter/setters. So proxy will have both specific and generic implementations. (Note that Generic implementation at proxy is one time change)

So Core would have to support both the standardized vehicle data mappings and the custom ones for the same data type in the scenario where the SDLC defines one of the types they use into a standard data set. However, this is still an open item based on my first question. Can Core using this feature be updated to use newly SDLC defined data types and how would that work in Core?

After core has processed new vehicle data items after dynamically downloading the schema file, it'd know what new vehicle data items it supports. let's say engine_state. Now, PTU would also have this new vehicle data as part of app policies just like older vehicle data. Core's mechanism to check for permissions would be reused in this case. Only change at core side would be to process the new vehicle data as per downloaded schema during PTU process.

There are a lot of leaps here that I feel need to be addressed.

Now, PTU would also have this new vehicle data as part of app policies How is the Policy table structured to allow this? What changes will the Sdl Policy Server project need to make to support it? (And Ford doesn't use this project is not an acceptable answer here).

[..] new vehicle data as part of app policies just like older vehicle data. Those data sets were predefined so we can easily add them per the RPC spec. How does the policy server get updated to support custom data?

Core's mechanism to check for permissions would be reused in this case. Again this assumes the policy table can be built out just like the older vehicle data RPCs but these new RPCs don't use standard data sets so validation becomes difficult.

Correct me if i am wrong, but vehicle data is NOT a SystemCapability item today.
There is a misunderstanding of what SystemCapability means. It is not specific to app services. The only piece that came from app services was the new ability to subscribe to a system capability to receive updates through a new RPC OnSystemCapbilityUpdated. Please read through this proposal to ensure you have the correct understanding of the purpose behind System Capabilities. I still believe this is a much better spot than OnPermissionChange as the updated vehicle datas are a direct change on it's capabilities, where is permission changes should happen based on the current state of the module's capabilities.

Get/Subscribe/UnSubscribe are Boolean as these are just requesting vehicle data. Refer to GetVehicleDataResponse/OnVehicleData methods for actual data return type.

I don't believe this is the correct way to handle this. Why wouldn't you just take an array of Strings into the method instead? Adding each key individually with a boolean seems redundant. The array should imply which data items should be retrieved by their existence in the array.

Yes it is correct return type as it is used only for SubscribeVehicleDataResponse/UnsubscribeVehicleDataResponse which just tells us whether the Subscription/Unsubscription was successful or not. In case core does not understand request data, it would return Invalid_Data as per current implementation.

VehicleDataResult has two parameters with types VehicleDataType and VehicleDataResultCode. VehicleDataType is an enum with predefined values; the enum does not include custom data types. How can VehicleDataResult be the correct type to use if that enum set is not expanded to accept all possible vehicle data types?

@atiwari9
Copy link
Contributor

@joeygrover

I meant for Core versions that would implement this feature to be able to handle future vehicle data types that it didn't originally have defined. This is especially powerful considering the proxy libraries can be built out to support standard APIs for the data and developers get access to newly agreed upon vehicle data much quicker than the current cycle.

Well yes, that is possible. Schema downloaded from cloud would be based on current schema version in open SDL. That ensures that any addition to vehicle data items in open SDL in future would be available to first SDL version with this feature. Though 3rd party apps would need to update the proxy version to use newer vehicle data items.

I think you missed my question here. Does core store both a static schema and a downloaded one? Or do they get merged? Or maybe just replaced? Does the proxy download the updated schema or is it just required to be hardcoded into the app? Should the schema be stored separately, merged together, or replaced if one is downloaded?

As per current proposal, core has a local copy of schema as approved by open SDL evol. Every schema file needs to be baselined on current version in open SDL. core can download and replace the schema file from OEM cloud and replace local file with that. This would add OEM specific data items.

Proxy on the other hand would only have local copy of schema. It can not download/update schema on the fly. To access OEM specific items, proxy would provide generic methods as defined in proposal.

While some semantics are just that, it is important to use standard. industry accepted terms when possible. This helps keep the learning curve lower, as we do not force them to create a new mapping in their mind between our terms and the industry's. The standard convention to a Map entry is a key-value pair. There are other associated terms like name–value pair, field–value pair or attribute–value pair, but I have never come across it as a reference-value pair in terms of a map entry (I could be mistaken). I believe key is the correct term in this instance as it applies to a key value pair in Core.

I am open to changing reference to key. Though reference still makes more sense to me as value for this reference would change based on vehicle type/architecture/EV/CAN_Bus etc. key in itself implies that it'll have unique value, but reference could vary based on environment being used.

So Core would have to support both the standardized vehicle data mappings and the custom ones for the same data type in the scenario where the SDLC defines one of the types they use into a standard data set. However, this is still an open item based on my first question. Can Core using this feature be updated to use newly SDLC defined data types and how would that work in Core?

That is correct, even if there are NO OEM specific items, proxy would have two ways to access a vehicle data item. Specific one and generic one. But point to note there is that Generic one is common for all vehicle data items. But only OEM apps would be able to truely use generic method given lack of structure definitions. For your 2nd question, i answered above and to reiterate, Core can update to new schema which would be baselined on schema version in open SDL. 3rd party apps would need to update to new proxy version to access newer vehicle data items. OEM apps can use generic method or specific method depending on proxy version it has.

Now, PTU would also have this new vehicle data as part of app policies How is the Policy table structured to allow this? What changes will the Sdl Policy Server project need to make to support it? (And Ford doesn't use this project is not an acceptable answer here).
[..] new vehicle data as part of app policies just like older vehicle data. Those data sets were predefined so we can easily add them per the RPC spec. How does the policy server get updated to support custom data?

Policy server should have all the new vehicle data items updated anyways to open SDL version of schema. When core downloads and processes that schema in future, it'd have definitons for new vehicle data items. Legacy core versions which do not support this method would simply ignore new vehicle data items pushed by policy server. Refer: smartdevicelink/sdl_core#1514

When it comes down to vehicle data items OEM adds, i believe that needs to be added to PT server implementation OEM is using. Now i am not specifically aware of the process for SDL Policy server implementation, is there separate policy per OEM? If you have any suggestions on this, please share. Also this would only be needed if OEM does not have custom policy server implementation.

Core's mechanism to check for permissions would be reused in this case. Again this assumes the policy table can be built out just like the older vehicle data RPCs but these new RPCs don't use standard data sets so validation becomes difficult.

Assuming you are talking about mismatch between what schema has and what PTU serves. If schema has a vehicle data item but PTU does not, then core would build it in policies DB and DISALLOW such request if app requests it. If PTU has a vehicle data item but schema does not, then core ignores such vehicle data item and does not build it in policies DB. Policy DB gets refreshed at next PTU.

There is a misunderstanding of what SystemCapability means. It is not specific to app services. The only piece that came from app services was the new ability to subscribe to a system capability to receive updates through a new RPC OnSystemCapbilityUpdated. Please read through this proposal to ensure you have the correct understanding of the purpose behind System Capabilities. I still believe this is a much better spot than OnPermissionChange as the updated vehicle datas are a direct change on it's capabilities, where is permission changes should happen based on the current state of the module's capabilities.

So that'd need new capability type for vehicle data correct? Would you need to define enumerations for OEM specific vehcile data items as well in advance? How will the structure look like, can you please specify?

onPermissionsChange would anyways still be sent to the app which has access to new vehicle data items when PTU update is applied(this assumes schema download and processing has already taken place). For OEM specific items, we should not send unnessasary updates to non-OEM apps or apps which do not have access to read those vehicle data items. While onPermissionsChange ensures that, OnSystemCapabilityUpdated would send these new vehicle data item list to ALL the apps which have subscribed to that.

I don't believe this is the correct way to handle this. Why wouldn't you just take an array of Strings into the method instead? Adding each key individually with a boolean seems redundant. The array should imply which data items should be retrieved by their existence in the array.

Though it goes in line with how other vehicle data items are requested, but i do not see any reason why we can't do a string array for multiple data items. Then app may request one or multiple at same time. I'd update the proposal for this.

VehicleDataResult has two parameters with types VehicleDataType and VehicleDataResultCode. VehicleDataType is an enum with predefined values; the enum does not include custom data types. How can VehicleDataResult be the correct type to use if that enum set is not expanded to accept all possible vehicle data types?

Thanks for pointing this out. I guess then we should use the parent RPCStruct instead or add a new result class GenericVehicleDataResult with:

public void setDataType(String dataType) {
        setValue(KEY_DATA_TYPE, dataType);
}

public String getDataType() {
        return (String) getObject(VehicleDataType.class, KEY_DATA_TYPE);
}

@joeygrover
Copy link
Member

I am open to changing reference to key. Though reference still makes more sense to me as value for this reference would change based on vehicle type/architecture/EV/CAN_Bus etc. key in itself implies that it'll have unique value, but reference could vary based on environment being used.

Key implies that it itself is static but the value it is associated to can change. So the value can be unique per platform.

Policy server should have all the new vehicle data items updated anyways to open SDL version of schema. When core downloads and processes that schema in future, it'd have definitons for new vehicle data items. Legacy core versions which do not support this method would simply ignore new vehicle data items pushed by policy server. Refer: smartdevicelink/sdl_core#1514

When it comes down to vehicle data items OEM adds, i believe that needs to be added to PT server implementation OEM is using. Now i am not specifically aware of the process for SDL Policy server implementation, is there separate policy per OEM? If you have any suggestions on this, please share. Also this would only be needed if OEM does not have custom policy server implementation.

I am looking for how the PT would be structured with this data. Can you please provide?
See the policy server project: https://github.com/smartdevicelink/sdl_server. OEMs take this, clone it and deploy it. To my knowledge, no one outside of Ford is using a custom, unrelated policy server implementation as it is extremely cumbersome to create one. Therefore, how the server gets updated to handle this is important.

Assuming you are talking about mismatch between what schema has and what PTU serves. If schema has a vehicle data item but PTU does not, then core would build it in policies DB and DISALLOW such request if app requests it. If PTU has a vehicle data item but schema does not, then core ignores such vehicle data item and does not build it in policies DB. Policy DB gets refreshed at next PTU.

Can you point me to the code in Core where you believe PT checks are happening?

**Can you please provide a sample policy table entry that can be used to check against the custom vehicle data schema? **

So that'd need new capability type for vehicle data correct? Would you need to define enumerations for OEM specific vehcile data items as well in advance? How will the structure look like, can you please specify?

No, you can create a new capability for VehicleDataCapability that would hold the information about vehicle data. I would assume one param would be something like an array of they keys as strings that hold the custom vehicle data names.

I believe it still works and can be an important part of how apps can retrieve what vehicle data is supported by the module they are connected with. Right now it's a try and fail method for each data type; it would be much better as a queryable data set.

Though it goes in line with how other vehicle data items are requested, but i do not see any reason why we can't do a string array for multiple data items. Then app may request one or multiple at same time. I'd update the proposal for this.

I actually rescind my comment here, you were correct. The RPC itself is flawed, but your solution is in alignment with it so I am fine with it. While it's very inefficient it is likely the best way to structure the message sent to Core.

Thanks for pointing this out. I guess then we should use the parent RPCStruct instead or add a new result class GenericVehicleDataResult with:

Maybe we could just modify VehicleDataResult. Add a new enum element to the VehicleDataType of OEM_SPECIFIC, GENERIC, or CUSTOM. Then add a new optional parameter to VehicleDataResult that holds the specific type as a string. Call it subDataType or similar.

@atiwari9
Copy link
Contributor

I am looking for how the PT would be structured with this data. Can you please provide?
See the policy server project: https://github.com/smartdevicelink/sdl_server. OEMs take this, clone it and deploy it. To my knowledge, no one outside of Ford is using a custom, unrelated policy server implementation as it is extremely cumbersome to create one. Therefore, how the server gets updated to handle this is important.

Since OEM would clone this project, then adding new vehicle data item should be similar to how existing data items are added. Now i am not familier with SDL_Server project at all. Could you point out where the existing vehicle data items are listed. Is that a list or hardcoded in to code file?

Can you point me to the code in Core where you believe PT checks are happening?
**Can you please provide a sample policy table entry that can be used to check against the custom vehicle data schema? **

I need to check with either LuxOft or Zhimin/Markos on this. Let's discuss in the call.

No, you can create a new capability for VehicleDataCapability that would hold the information about vehicle data. I would assume one param would be something like an array of they keys as strings that hold the custom vehicle data names.
I believe it still works and can be an important part of how apps can retrieve what vehicle data is supported by the module they are connected with. Right now it's a try and fail method for each data type; it would be much better as a queryable data set.

Okay, agreed that it provides updated capabilities to apps. But we need to be mindful of following points:

  • It will provide update to all apps which have subscribed for updates irrespective of whether app has access to that or NOT. OEM may or may not want that information to be sent to 3rd party apps
  • This is in addition to onPermissionsChange as that'd anyways be sent to apps that have access to new vehicle data

I actually rescind my comment here, you were correct. The RPC itself is flawed, but your solution is in alignment with it so I am fine with it. While it's very inefficient it is likely the best way to structure the message sent to Core.

OK, then no change would be made to proposal for this.

Maybe we could just modify VehicleDataResult. Add a new enum element to the VehicleDataType of OEM_SPECIFIC, GENERIC, or CUSTOM. Then add a new optional parameter to VehicleDataResult that holds the specific type as a string. Call it subDataType or similar.

I am fine with this approach as well.

@jordynmackool
Copy link
Contributor

jordynmackool commented Feb 13, 2019

The Steering Committee voted to keep this proposal in review to allow the author to gain assistance from the Project Maintainer policy server specialist (@nickschwab) in response to this question: “It also seems like the policy table additions would have to be flushed out so the SDL Policy Server could facilitate this type of validation” in this comment. The issue will remain open to allow additional members to comment if needed. The author is to clearly state what will be changed in a comment on the issue once a clear path is defined.

  • The Project Maintainer noted that there is an entire UX around building the policy tables, and the policy server app would need to be updated to a point where an OEM could add custom vehicle data elements and assign them into specific functional groups.

  • The vehicle data set types are hardcoded into the policy server because those are based on the rps_spec themselves, so when an rpc_spec is released, there is an update to the policy server. Changes can’t be made to for custom vehicle data because the vehicle data is unknown. The goal is to not make the OEM go into the code and implement those custom data items. OEMs can use the front end of this application to actually define the functional groups for themselves and move the requests etc. into new ones, delete them and modify them however they want. Currently there is no way is to inject custom parameters.

  • Vehicle data items need to align with what is in the rpc_spec that is in the open SDL. For example, if engineState was originally a custom OEM data type that was entered into specific functional group as part of the vehicle data rpc requests and it could later get added to the rpc_spec with a standardize structure that might differ from the original definition. The new update for the policy server is going to be hectic and it will be hard for OEMs to update their versions of the policy server because there will be conflicting changes, **custom engineState vs SDL defined engineState.

  • Additionally, the question was asked: What will the entry in the policy table look like for a custom parameter?

  1. Ford noted that it should be easy to add in policy table updates as long as the server-side implementation allows it, essentially the vehicleobject is sent to core and core validates it with whatever local vehicledata it can support it.
  • It was concluded that there should be a separate conversation about building out a vehicledatamanager that could handle using the vehicle permission manager to relay information about vehicle data that is available.

  • The PM rescinded his original argument against using onPermissionChange and agreed with the author that systemcapability did not need to be included because vehicledata uses the onPermissionChange RPC about availability making it redundant.

@nickschwab
Copy link
Contributor

VehicleData param Policy Table entries should directly match the RPC Spec schema and adhere to data structure standards.

  • id to become name
  • dataType to become type
  • params to be an array of objects instead of a map
  • addition of since, until (Strings)
  • addition of removed, deprecated (Booleans)
  • addition of minvalue, maxvalue, minsize, maxsize, minlength, maxlength (Integers)
  • recursion-friendly params

For example:

....
      "BaseBeforeDataConsent"
        ]
      }
    },
	"VehicleDataItems":
	[
		{
			"name":"rpm",
			"type":"Integer",
			"reference":"OEM_REF_RPM",
			"array":false,
			"mandatory":false,
			"since": "5.1",
			"params": []
		},
		{
			"name":"headLampStatus",
			"type":"Struct",
			"reference":"OEM_REF_HLSTATUS",
			"array":false,
			"mandatory":false,
			"since": "5.1",
			"params":[
				{
					"name":"ambientLightSensorStatus",
					"type":"AmbientLightStatus",
					"reference":"OEM_REF_AMB_LIGHT",
					"array":false,
					"mandatory":false,
					"params": []
				},
				{
					"name":"highBeamsOn",
					"type":"Boolean",
					"reference":"OEM_REF_HIGH_BEAM",
					"array":false,
					"mandatory":true,
					"params": []
				},
				{
					"name":"lowBeamsOn",
					"type":"Boolean",
					"reference":"OEM_REF_LOW_BEAM",
					"array":false,
					"mandatory":true,
					"params": []
				}
			]
		},
		{
			"name":"messageName",
			"type":"String",
			"reference":"OEM_REF_MSG",
			"array":false,
			"mandatory":false,
			"since": "5.1",
			"params": []
		}
	]
....

The proposed Policy Table structure assumes that SDL enums are defined in the RPC Spec and are not defined within the Policy Table.

Behavioral notes:

  • VehicleData item availability and definitions in the official SDL RPC Spec will always take precedence over custom ("proprietary") parameters.
  • VehicleData items in the official SDL RPC Spec are immutable and therefore cannot be extended with proprietary data
  • All VehicleData items must be defined in the Policy Table (not just the custom/proprietary items), per recommendation by PM's Core team
  • Root-Level vehicle data type items such as rpm are always mandatory:false as defined by the RPC Spec for the vehicle data RPCs. If a data item is of type Struct, that struct can have mandatory parameters but the struct itself cannot be mandatory for the root-level item.

This proposal requires substantial additions/modifications to SDL Policy Server and SHAID.

Required SHAID additions:

  • Conversion and transfer of full RPC Spec VehicleData parameters and enums into SHAID's database
  • New/updated API endpoint(s) for synchronization of the VehicleData parameters and enums to instances of Policy Server

Required Policy Server additions:

  • Enhanced synchronization logic to retrieve standard VehicleData parameters and enums from SHAID
  • New interface to create, update, and delete custom VehicleData parameters
  • Modified interface to create "Proprietary Functional Groups" with associated RPCs and custom VehicleData parameters
  • Ability to manually grant zero-to-many "Proprietary Functional Groups" to an application during application review. (Note: auto app approval will not grant any Proprietary Functional Groups)
  • Injection of VehicleData parameters into the generated Policy Table

@kshala-ford
Copy link
Contributor

I haven't read all the comments here but the last one. @nickschwab I'm confused, why do we need the versioning parameters? The mobile API reflects many different versions but only a specified version needs to be reflected in policies.

@nickschwab
Copy link
Contributor

@kshala-ford The idea is to build every VehicleData param (except for those with platform="documentation") that you see in the GetVehicleData RPC Spec definition (which would require every attribute currently possible there) as well as proprietary VehicleData params defined by OEMs in their Policy Server. To my knowledge, Policy Tables are not restricted to a specific RPC Spec version and Core does not send its supported RPC Spec version to a Policy Server when making a Policy Table Update request.

Also, please read the entire proposal and comments before partaking in discussions. I know this one is rather long, but it's important for everyone participating to be up-to-date on the issue in order to achieve the best possible outcome.

@atiwari9
Copy link
Contributor

@nickschwab

  • id to become name
  • dataType to become type
  • params to be an array of objects instead of a map
  • addition of since, until (Strings)
  • addition of removed, deprecated (Booleans)
  • addition of minvalue, maxvalue, minsize, maxsize, minlength, maxlength (Integers)
  • recursion-friendly params

Agreed, i'd add this as supplemental information next to schema definition and update the example schemas. To be clear, since, until, removed, deprecated, minvalue, maxvalue, minsize, maxsize, minlength, maxlength are optional and would be used as needed per vehicle data item.

The proposed Policy Table structure assumes that SDL enums are defined in the RPC Spec and are not defined within the Policy Table.

True.

Behavioral notes:

  • VehicleData item availability and definitions in the official SDL RPC Spec will always take precedence over custom ("proprietary") parameters.
  • VehicleData items in the official SDL RPC Spec are immutable and therefore cannot be extended with proprietary data
  • All VehicleData items must be defined in the Policy Table (not just the custom/proprietary items), per recommendation by PM's Core team
  • Root-Level vehicle data type items such as rpm are always mandatory:false as defined by the RPC Spec for the vehicle data RPCs. If a data item is of type Struct, that struct can have mandatory parameters but the struct itself cannot be mandatory for the root-level item.

This is in line with what we discussed during our meeting. I'd add this to the proposal.

This proposal requires substantial additions/modifications to SDL Policy Server and SHAID.

Required SHAID additions:

  • Conversion and transfer of full RPC Spec VehicleData parameters and enums into SHAID's database
  • New/updated API endpoint(s) for synchronization of the VehicleData parameters and enums to instances of Policy Server
    Required Policy Server additions:
  • Enhanced synchronization logic to retrieve standard VehicleData parameters and enums from SHAID
  • New interface to create, update, and delete custom VehicleData parameters
  • Modified interface to create "Proprietary Functional Groups" with associated RPCs and custom VehicleData parameters
  • Ability to manually grant zero-to-many "Proprietary Functional Groups" to an application during application review. (Note: auto app approval will not grant any Proprietary Functional Groups)
  • Injection of VehicleData parameters into the generated Policy Table

This should also be added to the proposal, i'll add it.

Another point to add is that PTU for app permissions/functional groups would add new vehicle data items in a similar way as current vehicle data items are handled. In given example, engineState would be added along with rpm, headLampStatus etc. Of-course OEM can have different combinations of functional-groups and vehicle data items per app.

@nickschwab
Copy link
Contributor

@atiwari9 Great!

To be clear, since, until, removed, deprecated, minvalue, maxvalue, minsize, maxsize, minlength, maxlength are optional and would be used as needed per vehicle data item.

Correct, it is my understanding that these attributes should be optional. @JackLivio can you please confirm based on your expertise with Core?

Another point to add is that PTU for app permissions/functional groups would add new vehicle data items in a similar way as current vehicle data items are handled. In given example, engineState would be added along with rpm, headLampStatus etc. Of-course OEM can have different combinations of functional-groups and vehicle data items per app.

App access to this data will be granted/limited through the use of Functional Groups. This introduces a slightly new dynamic to how Proprietary Functional Groups are assigned to an app when a Policy Table is generated (through explicit selection by the OEM during application review), but the behavior and structure of app_policies remains unchanged.

@Jack-Byrne
Copy link
Contributor

Yes those attributes are optional but they all have best practice reasons on when they should be used.

Another note: I think all "custom" vehicle data parameters that are not a part of the rpc spec should not have any version related tags included (since, until, removed, deprecated). "Custom" vehicle data parameters would not be able to have the same versioning system as the rpc spec since any version number supplied would not be the version associated with any known public rpc spec.

@atiwari9
Copy link
Contributor

@JackLivio -

Another note: I think all "custom" vehicle data parameters that are not a part of the rpc spec should not have any version related tags included (since, until, removed, deprecated). "Custom" vehicle data parameters would not be able to have the same versioning system as the rpc spec since any version number supplied would not be the version associated with any known public rpc spec.

Yes of-course, i guess that is understood. But i'd still mention that in revised proposal.

@atiwari9
Copy link
Contributor

To summarize, final revision would have updates as per following comments:

VehicleDataResult update
SDL Server related updates
Custom RPC versioning comment

I propose to accept the proposal with these revisions during the meeting.

@smartdevicelink smartdevicelink locked and limited conversation to collaborators Feb 19, 2019
@jordynmackool
Copy link
Contributor

The Steering Committee voted to accept this proposal with revisions. The author is to add a statement explaining how old core implementations using this method can be updated to expose new standardized vehicle data items. Additionally, the author is to add the three revisions noted in this comment.

@jordynmackool
Copy link
Contributor

@atiwari9 please advise when proposal has been updated to reflect agreed upon revisions.

@theresalech
Copy link
Contributor Author

theresalech commented Mar 12, 2019

Proposal has been revised to reflect revisions and issues have been entered:
RPC Spec
Core
Android
iOS
SDL Server
SDL HMI

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants