-
Notifications
You must be signed in to change notification settings - Fork 121
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
Comments
I have a few questions & notes on this.
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. |
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.
Policies would prevent unauthorized access to this API, does this answer the question? |
@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. |
@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. |
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. |
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. |
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:
Concerns:
|
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. |
@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. |
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, <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, "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 deviceDuring 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 itemThe 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. |
Has the author had time to review my previous comment? |
@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.
|
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. |
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. |
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. |
The Steering Committee voted to defer this proposal until the author and project maintainer are able to complete their investigation and suggest revisions. |
The requested revisions have been made. This review begins now and runs through February 12, 2019. |
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.
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 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?
|
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.
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.
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.
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)
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.
Correct me if i am wrong, but vehicle data is NOT a SystemCapability item today.
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.
Get/Subscribe/UnSubscribe are Boolean as these are just requesting vehicle data. Refer to GetVehicleDataResponse/OnVehicleData methods for actual data return type.
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. |
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.
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?
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
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?
There are a lot of leaps here that I feel need to be addressed.
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.
|
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.
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.
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.
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.
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.
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.
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.
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.
Thanks for pointing this out. I guess then we should use the parent RPCStruct instead or add a new result class GenericVehicleDataResult with:
|
Key implies that it itself is static but the value it is associated to can change. So the value can be unique per platform.
I am looking for how the PT would be structured with this data. Can you please provide?
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? **
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.
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.
Maybe we could just modify |
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?
I need to check with either LuxOft or Zhimin/Markos on this. Let's discuss in the call.
Okay, agreed that it provides updated capabilities to apps. But we need to be mindful of following points:
OK, then no change would be made to proposal for this.
I am fine with this approach as well. |
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.
|
VehicleData param Policy Table entries should directly match the RPC Spec schema and adhere to data structure standards.
For example:
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:
This proposal requires substantial additions/modifications to SDL Policy Server and SHAID. Required SHAID additions:
Required Policy Server additions:
|
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. |
@kshala-ford The idea is to build every VehicleData 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. |
Agreed, i'd add this as supplemental information next to schema definition and update the example schemas. To be clear,
True.
This is in line with what we discussed during our meeting. I'd add this to the proposal.
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. |
@atiwari9 Great!
Correct, it is my understanding that these attributes should be optional. @JackLivio can you please confirm based on your expertise with Core?
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 |
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. |
@JackLivio -
Yes of-course, i guess that is understood. But i'd still mention that in revised proposal. |
To summarize, final revision would have updates as per following comments: VehicleDataResult update I propose to accept the proposal with these revisions during the meeting. |
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. |
@atiwari9 please advise when proposal has been updated to reflect agreed upon revisions. |
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:
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
[email protected]
The text was updated successfully, but these errors were encountered: