smartdevicelink / sdl_evolution

Tracking and proposing changes to SDL's public APIs.
https://smartdevicelink.github.io/sdl_evolution/
BSD 3-Clause "New" or "Revised" License
33 stars 122 forks source link

[Accepted with Revisions] SDL 0293 - Enable OEM exclusive apps support #971

Closed theresalech closed 3 years ago

theresalech commented 4 years ago

Hello SDL community,

The review of the revised proposal "SDL 0293 - Enable OEM exclusive apps support" begins now and runs through November 24, 2020. Previous reviews of this proposal took place March 17 - 24, 2020, June 16 - July 27, 2020, and October 7 - November 10, 2020. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0293-vehicle-type-filter.md

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

https://github.com/smartdevicelink/sdl_evolution/issues/971

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:

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

joeljfischer commented 4 years ago

I understand the need here, and I think that moving these things down to the protocol layer is a good idea. It will allow us to block apps from connecting before the app shows on the head unit.

Questions

1.

This would help apps to show the vehicle brand logo depending on the vehicle type to which it's connected. In the current implementation, the lock screen configuration is set along with the SDL lifecycle/manager configuration. Hence while configuring the lock screen, the app does not know to which vehicle it's connecting. The apps cannot change the configuration of the lock screen once the app receives the RegisterAppInterface response with vehicle details. Some app partners have expressed their interest in showing the vehicle brand logo on the lock screen of the app depending on vehicle type information, which is not possible with the current implementation of SDL due to reasons described above.

This is entirely not true. Developers do not have to modify the lock screen or show their own to brand it by the OEM. All that needs to happen is that the OEM has to support and set a lock screen icon. This is then sent to the library using OnSystemRequest and shown on the lock screen (or even a dev's custom lock screen). I recommend removing this section from the proposal.

2. What is the purpose of a Get Vehicle Type packet? This would completely change the startup flow for head units that support this feature and would split the requirements for the app libraries when connecting to different versions of head units. Why was this not added to the Start Service ACK RPC service packet instead?

3. If you do move forward with Get Vehicle Type NACK, I believe that it should have a reason parameter.

4. This is missing any mention of the JavaScript Suite library implementation details.

5. What is the purpose of having the app developer define this information in static resources like the info.plist or Android resources. This precludes the possibility of an app developer updating this information in any way other than an app update (e.g. a download from a server or a code switch). It also separates the configuration from other SDL configurations in the SDLConfiguration class (iOS).

6.

The iOS proxy can implement an OnSDLEnabled notification similar to that of the Android proxy to notify the app about SDL connection with the supported vehicle. The app can provide lifecycle configuration to the proxy upon receiving the OnSDLEnabled notification.

Is this a requirement? The wording is unclear. Who decides if it "will"? The maintainers?

joeygrover commented 4 years ago

7.

If there are any apps that work with specific OEMs, they would need to send the RegisterAppInterface RPC to know vehicle details. If the vehicle details do not match the supported vehicle types, they would need to implement some mechanism to unregister the app.

They would simply send an UnregisterAppInterface request and close their app. This is very simple. Is the author trying to imply there would be more to this?

8.

The exclusive apps should not host Android Router Service on unsupported SDL systems. The Android system requires all apps using foreground services to show a notification. An SDL enabled Android app can have two foreground services. The Android Router Service is started by the Android proxy and another foreground service will be started by an SDL enabled app.

For clarification, only a single app will have two notification which is by design. So therefore the more apps, the less likely the app will be required to display two.

9.

The only way to remove notifications from an OEM App when it connects to another vehicle is to force stop both the services so that both notifications are removed. In this case, other apps connected through the OEM App's router service will also get disconnected. This is not a recommended approach

This is not exactly true. The OEM app has full control on their notification of their SDL service and could provide users a way to close that service.

10.

They would probably think that OEM App on their mobile device is working with another OEM's SDL enabled IVI system. This proposal will define ways to mitigate this issue.

At this point this statement is conjecture and should be supported with actual customer statements. Do we have any evidence of this from users?

11. rpcSpecVersion String RPC message spec version

The proposed solution is implicitly changing how RPC negotiation works. This should be considered out of scope for this proposal or clearly laid out as intention of the proposal.

12.

This proposal recommends starting the RPC service by the Android Router Service to get vehicle type information from an SDL enabled IVI system before notifying clients about an SDL connection.

This completely changes the scope of the router service and is not ideal. This will start a session for the router service in which will never be used beyond this corner case usage of a control frame. There is simply no need to start an RPC service for the router service since it will never be used. This will also introduce a delay for apps starting and showing on the vehicle which OEMs should be aware of.

13.

If the Android Router Service supports the connected IVI system, then the Android Router Service will notify the client and provide vehicle type information.

How? Where are those messages defined?

14.

If the vehicle type is not supported by the Android Router Service, the proxy will deploy the next router service. The exclusive apps will not register on the SDL enabled system.

How? Where are those code changes defined? How will the exclusive app know not to register at this point?

15.

The next router service deployed will not start the RPC session if vehicle type information is available. It will forward vehicle type info to its clients.

How? Where are those messages defined?

This creates another state for the router service which is not ideal. It also opens up the possibility that a bad actor could simply attach a supported vehicle model in their start intent and then the router service would be started regardless of if it actually supports the current vehicle or not.

16.

If the GetVehicleType message is not supported by the protocol version, the exclusive apps will try deploying the next router service. The exclusive apps will host the router service only if there are no other SDL app available on the user's device to host a router service. The exclusive apps, in this case, will rely on vehicle type info received in the RegisterAppInterface response. In such a case, if vehicle type is not supported, the exclusive apps will be allowed to unregister apps from the SDL system and stop the Android Router Service.

This will break older systems that the app actually supports. For example Ford's Gen3 modules will not be able to send this new packet. Therefore, the Ford app will not start the router service, and then it won't register. I would imagine for that reason alone this proposal should be returned or rejected.

17.

There is no supporting code on how the router service should function for this feature. I am not willing to support a proposal that wants to make changes to such an important piece of the library without supporting code snippets. Only stating requirements is not sufficient for this case; the router service is a complex piece of code that can't simply be massaged into new requirements without serious thought.

18.

What is the opinion of the SDLC on the probability that a user will have downloaded OEM specific apps that are not of the OEM their vehicle is from? Have we had any reports from actual customers that this is a problem? Or are we trying to solve a problem that seems like a problem?

theresalech commented 4 years ago

The Steering Committee voted to defer this proposal, to allow the Ford team time to identify a representative who can respond to the comments on the author's behalf, and take the lead on the proposal.

theresalech commented 4 years ago

Closing as inactive. The issue will remain in a deferred status and unlocked so the author or another member of the Ford team can notify the Project Maintainer when it is ready to be revisited.

jordynmackool commented 4 years ago

The author has advised that he is able to respond to the comments provided on the issue. This proposal is ready to be brought back into review, pending Steering Committee vote.

theresalech commented 4 years ago

The Steering Committee voted to bring this proposal back into review, as the author is able to respond to the comments and continue discussion. The proposal is now in review until June 23, 2020.

ashwink11 commented 4 years ago
  1. “All that needs to happen is that the OEM has to support and set a lock screen icon. This is then sent to the library using OnSystemRequest and shown on the lock screen (or even a dev's custom lock screen)” Since not all IVI supports sending lock screen icon using OnSystemRequest. Should we allow App to change the Lock screen icon based on Vehicle Type info received using described method? Since this is not the main idea of the proposal, I can remove it. However, this is just one of the possible additional use cases, that we can achieve with this proposal. It was also requested by one of our App partners. Let us know your thoughts.

  2. We are sending identification information and since this has nothing to do with ‘Starting a Service’, I added a new protocol message.

  3. Ok. I will add. The only possible reason for sending NACK is DATA not being available with SDL Core. Please do let me know if you see any additional reasons.

  4. Do we just need to add in affected platforms?

  5. In the case of Android apps, we want apps to read supported vehicle data from manifest files before asking them to start Router service. Please refer to section Metadata for Android Router Service.

  6. Yes, this is a requirement.

  7. Some app partners were looking for this info, but could not find in SDL documentation on the website. Please do let me know, how can we sent UnregisterAppInterface request, when using Manager APIs.

  8. Agreed. Only one app will show 2 notifications, which starts the SDL router service. Other apps will show only one notification. This proposal defines the requirement that exclusive apps should not show notifications or connect if the device is connected to an unsupported SDL system.

  9. Some app partners were looking for this info, but could not find in SDL documentation on the website. Please do let me know how apps can remove SDL notifications if they don’t want it.

  10. This requirement came from OEM and App developers. It did not come after customer complaints. a. We have some proprietary apps, which should register only on Ford systems and does not register on other IVI system. b. Also, I guess all OEMs would agree that => Apps which includes branding info of another OEM and meant for another OEM vehicle should not register or work on their system. c. We received some complaints from App partners as well. One of our app partners complained that their app registers on non-Ford IVI systems. They wanted to restrict the usage of their SDL app only to Ford systems. Another app partner mentioned, since they signed a contract with Ford, they want their app to work only on Ford systems.

  11. Please check section Messaging from the IVI to the app. rpcSpecVersion is one of the parameters of Get Vehicle Type ACK. The parameter is included here, to give information to the mobile App. If OEM App decided not to register on the system with the older RPC spec version, they can use info from this parameter. Mobile Apps would not need to send RAI to know RPC spec version in this case. Could you please explain why does this parameter in Get Vehicle Type ACK will change RPC negotiation?

  12. Please refer to section Determining Vehicle Type Info. If the SDL IVI system is supported by an App, the session info will be reused by the app hosting router service. This proposal recommended this, the app needs vehicle info before registration. Please do let us know if you have any suggestions.

  13. I believe the vehicle info needs to be shared along with the “OnSDLEnabled” method in Broadcast receiver of an App. I found that SDL Broadcast Receiver receives intent “START_ROUTER_SERVICE_ACTION”. It also has extra information likeSTART_ROUTER_SERVICE_SDL_ENABLED_APP_PACKAGE. I propose to include vehicle info as well with this intent. START_ROUTER_SERVICE_SDL_ENABLED_VEHICLE_INFO. I did not find any spec where these intents are defined, please do share the link to spec and I will include additions required for vehicle info.

  14. The SDL router service code already has a mechanism to deploy router service based on some criteria. I propose to add Vehicle Info as well as one of the criteria to deploy router service.

  15. See point 13.

  16. The text you quoted from the proposal describes a scenario where IVI does not support this new protocol message. The following requirements are mentioned in the quoted text.
    a. Only One SDL App installed on the user’s device – Exclusive apps starts router service b. Multiple SDL Apps installed on the user’s device – Exclusive app does not start router service.

When this proposal was written SDL Router service was the first component to establish a connection with the vehicle. However, that has changed now with the introduction of SDL proposal SDL-0301. Since the SDL device listener is the first component now to establish a connection with Vehicle, I think we should use an SDL device listener instead of an SDL router service? In case you are looking for changes in the Router service described in the current proposal. Please find the sample code below

Exclusive Apps defining vehicle filter

<service
    android:name=".SdlRouterService"
    android:exported="true"
    android:process="com.smartdevicelink.router">
    <intent-filter>
        <action android:name="com.smartdevicelink.router.service"/>
    </intent-filter>
    <meta-data android:name="@string/sdl_router_service_version_name"  android:value="@integer/sdl_router_service_version_value" />
    <!-- Defining vehicle filter is not mandatory. Only exclusive apps will need to define it. -->
    <meta-data
        android:name=" com.smartdevicelink.router.vehicle_filter"
        android:resource="@xml/vehicle_filter" />
</service>

Changes in SDLAppInfo class

    //Add member variables for storing vehicle type info
    ArrayList<VehicleType> supportedVehicleTypeList = new ArrayList<>();
    boolean isExclusiveApp = false;

    //updates in constructor
    public void  SdlAppInfo(ResolveInfo resolveInfo, PackageInfo packageInfo, Context context){
       .
       .
       .
                if (metadata.containsKey("com.smartdevicelink.router.vehicle_filter")){
                    this.isExclusiveApp = true;
                    //read vehicle filter list
                    this.supportedVehicleTypeList = processVehcleFilterXML(resources.getXml(resourceId));
                }
    }

Changes in SDL Router Service

    public void onTransportConnected(final TransportRecord record){
        if(!checkVehileTypeInfoAvailable()){
            byte[] serviceProbe = SdlPacketFactory.createStartSession(SessionType.RPC, 0x00, (byte)1, (byte)0x00, false).constructPacket();
            isVehicleTypeProbe = manuallyWriteBytes(record.getType(), serviceProbe,0,serviceProbe.length);
            if(isVehicleTypeProbe){
                return;
            }
        }
        /***
         * If vehicle type info available or service probe not sent
         * We will notify transport connection to interested parties.
         */
        notifyTransportConnection(record);
    }

    private void notifyTransportConnection(final TransportRecord record){
        cancelForegroundTimeOut();
        enterForeground(createConnectedNotificationText(),0,true);

        if(packetWriteTaskMasterMap == null){
            packetWriteTaskMasterMap = new ConcurrentHashMap<>();
        }

        TransportType type = record.getType();
        PacketWriteTaskMaster packetWriteTaskMaster = packetWriteTaskMasterMap.get(type);

        if(packetWriteTaskMaster!=null){
            packetWriteTaskMaster.close();
            packetWriteTaskMaster.alert();
        }
        packetWriteTaskMaster = new PacketWriteTaskMaster();
        packetWriteTaskMaster.setTransportType(type);
        packetWriteTaskMaster.start();
        packetWriteTaskMasterMap.put(type,packetWriteTaskMaster);

        Intent startService = new Intent();
        startService.setAction(TransportConstants.START_ROUTER_SERVICE_ACTION);
        startService.putExtra(TransportConstants.START_ROUTER_SERVICE_CONNECTED_VEHICLE_INFO, vehicleInfoObject);

        startService.putExtra(TransportConstants.START_ROUTER_SERVICE_SDL_ENABLED_EXTRA, true);
        startService.putExtra(TransportConstants.FORCE_TRANSPORT_CONNECTED, true);
        startService.putExtra(TransportConstants.START_ROUTER_SERVICE_SDL_ENABLED_APP_PACKAGE, getBaseContext().getPackageName());
        startService.putExtra(TransportConstants.START_ROUTER_SERVICE_SDL_ENABLED_CMP_NAME, new ComponentName(this, this.getClass()));

        if(record!= null && record.getType() != null){
            startService.putExtra(TransportConstants.START_ROUTER_SERVICE_TRANSPORT_CONNECTED, record.getType().toString());
        }

        if(Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN) {
            startService.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
        }

        AndroidTools.sendExplicitBroadcast(getApplicationContext(),startService, null);

        //HARDWARE_CONNECTED
        if(!(registeredApps== null || registeredApps.isEmpty())){
            //If we have clients
            notifyClients(createHardwareConnectedMessage(record));
        }
    }

        public void onPacketRead(SdlPacket packet){
        .
        .
        .
            if(isVehicleTypeProbe){
                processVehicleTypeProbeMsg(packet);
                return;
            }
        }

private void processVehicleTypeProbeMsg(SdlPacket packet) {
        if(packet.getVersion() < 6)
        {
            //End service probe. Vehicle type is not supported.
            isVehicleTypeProbe = false;
        }
        if (!checkVehileTypeInfoAvailable() && packet.getVersion() >=  6 && packet.getFrameInfo() == SdlPacket.FRAME_INFO_END_SERVICE_ACK) {
            //request vehicle Type info from connected system
            byte[] serviceProbe = SdlPacketFactory.createGetVehicleType(SessionType.RPC, (byte)packet.getSessionId(), (byte)1, (byte)0x00).constructPacket();
            isVehicleTypeProbe = manuallyWriteBytes(packet.getTransportRecord().getType(), serviceProbe,0,serviceProbe.length);
        }
        if (packet.getFrameType() == FrameType.Control && packet.getFrameInfo() == SdlPacket.FRAME_INFO_GET_VEHICLE_TYPE_ACK) {
            //We received a vehicle type info
            //Router service will save it and check if app hosting router service supports this vehicle
            processVehicleTypeInfo(packet);
            isVehicleTypeProbe = false;
            if(checkifAppHostingRouterSupportsVehicle()){
                //If router service supports connected vehicle.
                //Notify transport connection
                notifyTransportConnection(packet.getTransportRecord());
            }
            else{
                //Since vehicle type is not supported by app hosting router service
                //deploy next router service which supports this connected vehicle
                deployNextRouterService();
            }
            return;
        }
    }
  1. See point 10.
theresalech commented 4 years ago

Please find responses from the PM below:

1. Because there is a defined mechanism for this, we should not define a second, worse, method for doing this. The author's IVI systems should update to support sending a lock screen icon URL instead. This section should be removed.

2. But it's unnecessary to add a new packet. Why start the RPC service and then decide? The head unit could respond with a StartServiceACK with their vehicle information and the app would disconnect. There would be no need to check version, etc., the library would only have to check if that payload exists. It simplifies the requirements and flow significantly.

3. Thanks! Author to make following revision: Add reason parameter to Get Vehicle Type NACK.

4. You need to describe how the app will set up its configuration (you have an info.plist on iOS, manifest on Android), and any other platform-specifics needed for JavaScript apps, WebEngine apps, etc.

5. This doesn't answer why it's added where it is in iOS. I would strongly recommend changing this to be an update to the SDLLifecycleConfiguration.

The section referenced doesn't contain the logic you are referring to. In the next section Determining Vehicle Type Info point 5 attempts to describe this. The wording used is incorrect however. The current router service will be the one to determine the next router service to start, not the "proxy". Please be as specific as possible here as everything related to the router service must be extremely clear.

6. Please update the wording to use requirement language (e.g. "must", "will", etc.)

7. The same way they send any other RPC:

iOS:

let rpc = SDLUnregisterAppInterface()
sdlManager.send(rpc)

Java

UnregisterAppInterface rpc = UnregisterAppInterface();
sdlManager.send(rpc)

Alternatively you could just call sdlManager.dispose and it will close the session for Java

8. Ok can we agree to add the clarifying language?

9. This is Android specific and outside the scope of SDL, but they only have to make their notification for their SDL Service not persistent or remove the notification programmatically. This is well documented in the Android SDK.

10. Again I think apps can unregister or stop their SDL service if they don't wish to work with a certain vehicle make/model. The only item would be them hosting the router service.

11. I meant RPC Version negotiation, and given your statement on this it should be obvious how that has been altered. It no longer takes place in the RAI request response, but an arbitrary control packet that is sent on the control service and not the RPC.

12. That is even worse than previously mentioned. I do not agree that this information should start at this layer and be passed. It adds an enormous amount of complexity that the application layer will have to deal with and is not worth the effort. Not to mention there is simply no description on how the manger layers or any other layers would handle this. I strongly oppose this.

13. There is no spec, the intents are defined in the TransportConstants file. This new extra must be documented in the proposal and that is what I am asking from the author. It should be clear in the proposal how these messages are being sent and when.

14. Again I want to see the exact flow for this. I understand the methodology for deploying the next router service, but your proposal adds more states and messages that must be taken into account so it is not as simple as just saying "I will call this method because it exists."

15. 13 does not answer this question. There are two different functions and intents. If you intended to use the same extra naming then it should be clear that that is what you wish. However, if that is the case the naming should be generic enough for both scenarios.

16. Just another note here, it should be clarified that if the OEM app does not support the hardware it will only shutdown it's router service and not any other router service that could be running.

17. The SdlDeviceListener will only connect a single time per bluetooth device. After that it skips that connection because it is unnecessary; this is all described in that proposal. Therefore, unless you intend to change the proposal to reflect this I don't think it will work.

What is vehicleInfoObject that you are sending in the intent? I would be looking for you as the author to clarify exactly how you are sending these messages and what is included.

There is so much left out code and hand waving that I still do not clearly see what's going on. If you are not going to include what your new methods are, they should be clearly documented on what you intend for them to do, eg checkVehicleTypeInfoAvailable.

This will obviously not take into account the case where multiple transports have connected in very close timing creating race conditions.

It seems like it will just drop any packets that aren't the new GetVehicleType, but obviously it seems like this needs to listen for the StartServiceACK as well or else this feature will not work.

I would ask the author to really try to work through all of these complicated flows because as I've stated before, this is a very important piece of the library and any bug can cause very large issues.

18. I would like to request the rest of the SDLC weigh in on this topic, I understand that author sees this as an issue.

19. This flow change would change quite a bit about the protocol layer / lifecycle layer startup. This impact and required changes to lower-level layers of the app libraries needs to be clearly noted. For example, the protocol layer would have to alter its startup flow and send a GetVehicleType packet after the RPC Start Service in some cases and only start the lifecycle layer depending on whether the packet is sent and the response.

theresalech commented 4 years ago

The Steering Committee voted to keep this proposal in review, to allow the author time to respond to the last comment from the Project Maintainer.

ashwink11 commented 4 years ago

1. Ok. I will remove it.

2. StartService is used for RPC, Audio and Video. I guess we should not be sending vehicle type info in all StartServiceACK. Right? So, should we send vehicle info in StartServiceACK for RPC only?

4. 5.

The only reason for adding a vehicle filter using info.plist in iOS was to keep a similar design with Android. If you are OK with different designs for Android and iOS, then we can use SDLLifecycleConfiguration for iOS. I think we can use the SDLLifecycleConfiguration for Javascript Apps and WebEngine apps as well. In Android, the Router service needs to know the supported vehicle list, hence I proposed to keep the vehicle filter list as meta-data.

6. Sure, will do.

7. Thank you.

8. Sure, I will do.

9.

This is not exactly true. The OEM app has full control on their notification of their SDL service and could provide users a way to close that service.

Consider a scenario: If the App is hosting a router service and connected to the SDL system. Later, the App receives the vehicle type info and decided to unregister. How can an App remove notification? Can the app stop the router service to remove SDL router service notification?

10. For our OEM exclusive app we do UnregisterAppInterface. However, the HMI will show the app for a split second which causes customers to complain. We do have customers reporting this to be an issue. Please note that Ford Motor Company has two brands Ford and Lincoln. Also we do have two “brand” specific apps FordPass and LincolnWay. There are customers that have both brands and both apps installed. These customers were complaining about apps flickering on both their cars.

11. I included rpcSpecVersion just to provide information to the app before the app sends RAI. RPC Version negotiation and Mobile API versioning should remain as is. Do you still think it will have an impact on "RPC Negotiation"?

12. Ok, then we will not pass the information, however, even if we send vehicle type Info in StartServiceACK, we will have to send one start service protocol message from the SDL router service. So, the router service knows the vehicle type info before notifying clients. If the current app hosting the router service does not support connected vehicles, it will have to find a supported app and deploy another Router service.

13. Ok. I will define new Extras in the proposal.

14. I will add states and Extras for intents in the proposal. What do you mean by "messages" here?

15. Does intent extra with the name "SDL_ENABLED_VEHICLE_INFO" works?

16. Sure, I will add that exclusive Apps will be allowed to stop router service they are hosting and not other router service.

17. vehicleInfoObject is the object of the class which holds the vehicle type info received from SDL core. In previous comments I wrote a "skeleton code", which of course can be modified by PM as required for implementation. Implementation details can be decided by PM and hence were not part of the proposal.

Updates in existing VehicleType class. If you want you can define seperate class as well.

class VehicleType implements Parcelable{
    String make;
    String model;
    String model;
    String trim;
    String systemSoftwareVersion;
    String systemHardwareVersion;
    String rpcSpecVersion;

    //ToDO: getters, setters and mandatory methods for Parcelable as per Android SDK documentation
}
boolean checkVehicleTypeInfoAvailable(){
    //check if vehicle type info is available
    //with router service.
    //return true if vehicle type info is available
    //return false if not available
}

It seems like it will just drop any packets that aren't the new GetVehicleType, but obviously it seems like this needs to listen for the StartServiceACK as well, or else this feature will not work.

Sorry for the typo in earlier comment. processVehicleTypeProbeMsg will look for Start Service ACK.

    private void processVehicleTypeProbeMsg(SdlPacket packet) {
        if(packet.getVersion() < 6)
        {
            //End service probe. Vehicle type is not supported.
            isVehicleTypeProbe = false;
        }
        if (!checkVehileTypeInfoAvailable() && packet.getVersion() >=  6 && packet.getFrameInfo() == SdlPacket.FRAME_INFO_START_SERVICE_ACK) {
            //request vehicle Type info from connected system
            byte[] serviceProbe = SdlPacketFactory.createGetVehicleType(SessionType.RPC, (byte)packet.getSessionId(), (byte)1, (byte)0x00).constructPacket();
            isVehicleTypeProbe = manuallyWriteBytes(packet.getTransportRecord().getType(), serviceProbe,0,serviceProbe.length);
        }
        if (packet.getFrameType() == FrameType.Control && packet.getFrameInfo() == SdlPacket.FRAME_INFO_GET_VEHICLE_TYPE_ACK) {
            //We received a vehicle type info
            //Router service will save it and check if app hosting router service supports this vehicle
            processVehicleTypeInfo(packet);
            isVehicleTypeProbe = false;
            if(checkifAppHostingRouterSupportsVehicle()){
                //If router service supports connected vehicle.
                //Notify transport connection
                notifyTransportConnection(packet.getTransportRecord());
            }
            else{
                //Since vehicle type is not supported by app hosting router service
                //deploy next router service which supports this vehicle
                deployNextRouterService();
            }
            return;
        }
    }

18. please refer to the response of 10.

theresalech commented 4 years ago

@ashwink11 thanks for your responses! It seems we are in agreement about a lot of items. Please find PM feedback - grouped based on status - below

Author to Revise

1. Remove the following section:

This would help apps to show the vehicle brand logo depending on the vehicle type to which it's connected. In the current implementation, the lock screen configuration is set along with the SDL lifecycle/manager configuration. Hence while configuring the lock screen, the app does not know to which vehicle it's connecting. The apps cannot change the configuration of the lock screen once the app receives the RegisterAppInterface response with vehicle details. Some app partners have expressed their interest in showing the vehicle brand logo on the lock screen of the app depending on vehicle type information, which is not possible with the current implementation of SDL due to reasons described above.

2. Update proposal to send vehicle info in StartServiceACK for RPC only.

3. Add reason parameter to Get Vehicle Type NACK

6. Update wording to use requirement language (e.g. "must", "will", etc.) in the following section:

The iOS proxy can implement an OnSDLEnabled notification similar to that of the Android proxy to notify the app about SDL connection with the supported vehicle. The app can provide lifecycle configuration to the proxy upon receiving the OnSDLEnabled notification.

7. Proposal should be updated to: If there are any apps that work with specific OEMs, they would need to send the RegisterAppInterface RPC to know vehicle details. If the vehicle details do not match the supported vehicle types, they would need to send an UnregisterAppInterface request RPC.

8. Add clarifying language to the following section:

The exclusive apps should not host Android Router Service on unsupported SDL systems. The Android system requires all apps using foreground services to show a notification. An SDL enabled Android app can have two foreground services. The Android Router Service is started by the Android proxy and another foreground service will be started by an SDL enabled app.

It should be clarified that only a single app will have two notifications, which is by design. Therefore, the more apps, the less likely the app will be required to display two.

13. Define START_ROUTER_SERVICE_SDL_ENABLED_APP_PACKAGE and make it clear how the messages are being sent and when, as intents are defined in the TransportConstants file.

16. In the following section, clarify that exclusive apps will be allowed to stop the router service that they are hosting, and not other router services:

If the GetVehicleType message is not supported by the protocol version, the exclusive apps will try deploying the next router service. The exclusive apps will host the router service only if there are no other SDL app available on the user's device to host a router service. The exclusive apps, in this case, will rely on vehicle type info received in the RegisterAppInterface response. In such a case, if vehicle type is not supported, the exclusive apps will be allowed to unregister apps from the SDL system and stop the Android Router Service.

Open Items

4/5. We need to discuss this further internally. We'll get back to you on this item soon.

9. As the proposal states "their SDL Service" which is not the same as the SdlRouterService. Their SDL Service will be the service that hosts the SDL Manager integration. I think there is just a misunderstanding here. The proposal just needs to be updated to reflect that your statement is about the router service, not their SDL Service.

10. Then the proposal should include such information. Adding as part of the problem statement/motivation section that apps do have the ability to unregister based on make/model but due to this information coming in the RAI response, their app will likely flicker on screen for certain OEMs. (Other OEMs hardcode their app list so this is not an issue).

11. Yes, the current flow is that the app sends their max version of RPC spec supported in the RAI request. The IVI system would then respond with the negotiated max version between the app's and the IVI. What you are proposing is that we first send the max RPC spec version of the IVI system in the new GetVehicleType ACK. The app would then essentially be responsible for finding the negotiated RPC spec version max. (Also the fact that such information comes from the control service just doesn't really make sense in the protocol view. Coming back in the SS_ACK I think does make more sense if we can't just send an unsolicited control message.) I'm not totally against changing the negotiation flow, but it has to be clearly laid out and discussed as such. We need to avoid just subtly changing the flow without proper discussion on how best to achieve the result.

12. I'm not exactly sold on this solution yet. I hate to see the layers get mixed up if we can avoid it. Because even if it did come in the SS ACK for the RPC service that the router service started, you would have to manage this session as well. For example, if the router service was an OEM app that didn't support the current IVI system and it attempts to start a new router service, we would have to make sure that that session is properly ended as well and not orphan it so that it stays open until the next ignition cycle.

14. Sorry, I meant in general not just in respects to this number 14. I would look towards the newly defined extras and states.

15. I think that's better, but it could be `CONNECTED_VEHICLE_INFO"

17. I appreciate the author's statement that the production code will be left up to the project maintainers and I think that's probably best for items with the router service. It would be required to add this language to the proposal as well so it is agreed to by all. I do want to make sure that when we accept proposals that affect the router service there is a clear understanding of what it will take to accomplish it and if it is even a feasible change.

There are still some considerations to think through. First if we extend the VehicleType class to implement parcelable, that will affect the RPC generation of the class forever. It adds a corner case that we must ensure is understood. Just thinking through a possible alternative we could make the base class RPCStruct implement parcelable by using the Hashtable functionality. I would have to think through this more. Obviously the easier answer is to create a new class of something like VehicleInfo that is not attached to the RPC spec in any way.

Next, I am not going to nitpick through code in this forum, but I want to make sure I understand what your flow is. In the newly supplied snippet for processVehicleTypeProbeMsg, if the protocol level is less than 6 you need to add some additional logic here, right? For example, as it pertains to point 16, the RS would have to find if it is the only router service and stay open until the RAI response in the app to see if the app supports the vehicle. In the case that it is not the only router service, it would attempt to deploy the next router service I suppose. However, let's say in the example of the two branded apps from Ford, if the Ford app started the router service, sent the SS and received a SS_ACK back with protocol version < 6, it would try to deploy the next router service. In this case it would be the Lincoln brand. However, when that router service starts it either does the same thing or is forced to stay open. If the connected vehicle was a Ford and not Lincoln brand, the owner would still see the Lincoln app hosting the router service and the Lincoln app flash on screen, thus the proposal not solving the problem it set out to but adding extra complexity. It realistic terms, it seems if the protocol doesn't support the feature, the router service should just stay open instead of complicating the flows with a solution that only has a chance of working right.

Also, it would be nice to see a flowchart on how the router service should go through all of these newly added states and methods to really tie the feature together.

18. No need for the author to respond to this item again; as mentioned previously, we are looking for feedback from other SDLC members.

19. Author still needs to respond on this item. Since the author is okay with using the StartServiceACK now, this could be accomplished by updating the flow chart in the proposal.

ashwink11 commented 4 years ago

@theresalech I agree with 1, 2,3,6,8,13 and 16. however, can you please clarify point 7?

7.

Proposal should be updated to: If there are any apps that work with specific OEMs, they would need to send the RegisterAppInterface RPC to know vehicle details. If the vehicle details do not match the supported vehicle types, they would need to send an UnregisterAppInterface request RPC.

This statement should be added in Motivation and not in the proposed solution. Right? This statement is true for the current Implementation. However, the proposed solution is to share vehicle details using the protocol message. The exclusive apps on receiving connected vehicle details can perform one of the following: a. If the vehicle type is supported, sends RegisterAppInterface b. If the vehicle type is NOT supported, the exclusive apps do not send RegisterAppInterface

I will get back to you on other points.

ashwink11 commented 4 years ago

9. I could not find the "their SDL Service" phrase in the proposal. Can you please clarify which statement in the proposal you were referring too? 10. This is already mentioned in the Motivation section. Please refer section Handling Proprietary app registrations. 11. Ok. Since it is not absolutely required at the moment, I will remove the RPC spec version from the protocol message. 12. If router service is the first component to start communication with the connected vehicles, then I think, we will have to start the session to find vehicle info in router service. We can send an "End Service" protocol message so that the session is not orphaned. Please do let me know if you have any other suggestions. 14. Ok. 15. I will add this. 17.

It would be required to add this language to the proposal as well so it is agreed to by all.

Sure, I will add in the proposal that implementation details will be decided by PM.

In the newly supplied snippet for processVehicleTypeProbeMsg, if the protocol level is less than 6 you need to add some additional logic here, right?

True, if the protocol version is less than 6, I propose that router service should notify clients about SDL connection.

It realistic terms, it seems if the protocol doesn't support the feature, the router service should just stay open instead of complicating the flows with a solution that only has a chance of working right.

I agree with your suggestion, that the router service should stay open if the protocol version does not support vehicle type payload. Please find the updated flow.

The below flow is modified from SDL 0301 proposal. Starting Section A and Section C, we would like to extend the scope of SDL Listener.

OEM_APPS_1

In the below flow chart, SDL Listener will check vehicle type info for the connected vehicle. After detecting successful BT connection, the SDL listener will send Start Service for RPC, to know vehicle type Info. The SDL Listener will then end the RPC session and close BT socket connection. If Vehicle Info is available in StartServiceACK, the SDL Device Listener should save this info and use it to decide the host of the Router service on subsequent connections.

OEM_APPS_2

On finding the Vehicle Info, SDL Listener should find the appropriate app to start the Router service. The vehicle filter of an App will be available in the meta-data of router service in manifest file. The SDL listener will find the supported app using the meta-data.

OEM_APPS_3

The below flow chart shows, the Application receiving on SDL enabled. The vehicle details will be shared with apps using Intent extra.

OEM_APPS_4

We found that the original flow proposed has to be changed since the introduction of SDL 0301 proposal. The problem is, that if any unsupported app is connected to a vehicle with protocol version 5 (or protocol version that does not support vehicle payload in StartServiceAck), the SDL device Listener will open BT connection and close it. If an unsupported app starts SDL router service, it will open BT connection and close it again. To avoid this, we need to consider SDL Device Listener as well for this proposal, which establishes BT socket connection and closes it as defined in SDL 0301 proposal. I would like to propose using this SDL Listener BT connection to complete vehicle type probe, instead of starting another SDL BT connection from Router service of an unsupported app.

19. Sure, I will update the flow chart for StartServiceACK.

theresalech commented 4 years ago

Hi @ashwink11, thanks for your responses! Please find feedback from the PM below.

Author to Revise Items 1, 2, 3, 6, 8, 11, 13, and 16 from this comment.

7. (clarification)

This statement should be added in Motivation and not in the proposed solution. Right?

Yes, in the motivation is a good spot. The reason for this is that it is important to make note of what solution already exists for the problem you are trying to solve and note all the shortcomings. So when a reader goes through your proposal they can clearly understand why we should add this new feature and that the current state of the library is inadequate for the reasons provided by the author.

11. Author to remove RPC spec version from the protocol message.

As a side note, we believe there's potential for improving the handshake further in a separate, more targeted proposal.

12. Router service will be responsible for managing the temporary RPC service. Starting the service only if the app has supplied the meta-data and stopping the service as soon as it receives the StartServiceACK.

14. Author to update code snippets with explicitly named extras and any messages.

15. Author to add CONNECTED_VEHICLE_INFO intent extra as part of the supplied code snippets.

19. Author to update flowchart for StartServiceACK.

Open Items

4/5. It would be best to try and keep the APIs the same between the different platforms, so to do that all of the config classes through the different projects should look similar. Provided are the iOS and Java Suite versions. The Android developers will still need to add the list to an xml resource to include in their manifest, but can easily access those values when receiving the callback to the vehicle filter.

@interface SDLLifecycleConfiguration
// Additions
// A type definition for a lambda that takes an SDLVehicle type and versions and return YES if the app should continue to connect and NO if the app should not connect.
typedef BOOL (^SDLVehicleInfoFilter)(SDLVehicleInfo *vehicleInfo);
@property (copy, nonatomic) SDLVehicleInfoFilter vehicleInfoFilter;

@end

// New Class
@interface SDLVehicleInfo: NSObject

@property (copy, nonatomic, nullable) NSString *make;
@property (copy, nonatomic, nullable) NSString *model;
@property (copy, nonatomic, nullable) NSString *year;
@property (copy, nonatomic, nullable) NSString *trim;
@property (copy, nonatomic, nullable) NSString *systemSoftwareVersion;
@property (copy, nonatomic, nullable) NSString *systemHardwareVersion;

@end

Usage:
lifecycleConfiguration.vehicleInfoFilter = ^BOOL(SDLVehicleType *vehicleType, NSString *systemSoftwareVersion, NSString *systemHardwareVersion) {
    if ([vehicleType.make isEqualToString:@"Ford"]) { return YES; }

    return NO;
};
//BaseSdlManager.Builder
Builder setSdlVehicleFilter(SdlVehicleFilter filter){
    this.sdlVehicleFilter = filter;
    return this;
}
//New interface
public interface SdlVehicleFilter {
    boolean onSdlVehicleConnected(VehicleInfo info);
}
//New class
class VehicleInfo implements Parcelable {
    String make;
    String model;
    String year;
    String trim;
    String systemSoftwareVersion;
    String systemHardwareVersion;
}

9. It's the statement I already quoted. It states the "only way" to remove "both" notifications which includes both the router service and their SDL Service in that statement.

17. I see the new logic and can understand where you are going with it.

We've seemed to agree to the following: 17a. Author to specify in proposal that implementation details will be decided by PM

17b. Author to describe logic if protocol version does not support feature.

These items are still under discussion, as we have some concerns:

17c. Not all bluetooth chips used by hardware will actually have different MAC addresses. While technically each IVI system should have unique addresses, in the past we have seen this as not the case. This is especially true for lower costs devices. So we would have to make an assumption that if the first device that connected with that MAC address passed or failed the filter, that we would be ok with assuming all IVI systems with that MAC address would be of the same make, model, trim, etc. The known issue would be that if an IVI system is shared between multiple models or trims the filter would only be good for make.

17d. This will prolong the start of the router service and might cause the router service to miss the IVI's attempts at connecting to the SPP channel. The library is already at a little bit of a risk of not connecting the router service on first successful connection of the SdlDeviceListener, and adding another step in the process will put that risk higher. So we would have to be ok with that risk vs how this would solve some of the issues.

18. We are looking for input from other SDLC Members on the following:

What is the opinion of the SDLC on the probability that a user will have downloaded OEM specific apps that are not of the OEM their vehicle is from? Have we had any reports from actual customers that this is a problem? Or are we trying to solve a problem that seems like a problem?

Closed - no action required

10. Got it, thanks!

theresalech commented 4 years ago

We did not have a quorum present during the 2020-07-07 Steering Committee meeting, so voting on this proposal could not take place. This proposal will remain in review until 2020-07-14.

Sohei-Suzuki-Nexty commented 4 years ago

17. This flow is only for a BT connection, but how about a USB connection?

ashwink11 commented 4 years ago

4/5.

The Android developers will still need to add the list to an XML resource to include in their manifest, but can easily access those values when receiving the callback to the vehicle filter.

Please refer to the flow chart in my previous comments for Android. The app will get vehicle details in the "OnSDLEnabled" callback. If the vehicle details are known to App before SDL proxy initialization and vehicle filter is also mentioned in XML, then we do not need to initialize SDL Manager and hence no need for method "setSdlVehicleFilter" to set a callback for vehicle filter. If App initializes SDL manager for the unsupported vehicle, the App will need to start background services and hence end up showing notifications to the user, which will cause intrusion. The exclusive apps should not show any notifications for unsupported vehicle connection.

    @Override
    public void onSdlEnabled(Context context, Intent intent, VehicleType vehicleType) {
        /*This method is in Application layer and app developers decide if they want to implement below check.*/
        /*App developers can start SdlService (or initialize proxy) without checking vehicle type as well.*/

        if(null == vehicleType || IsVehicleTypeSupported(vehicleType)){
            DebugTool.logInfo(TAG, "SDL Enabled");
            intent.setClass(context, SdlService.class);
            // SdlService needs to be foregrounded in Android O and above
            // This will prevent apps in the background from crashing when they try to start SdlService
            // Because Android O doesn't allow background apps to start background services
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
                context.startForegroundService(intent);
            } else {
                context.startService(intent);
            }
        }
    }

    private boolean IsVehicleTypeSupported(VehicleType vehicleType) {
        /*This method is in Application layer and app developers decide on implementation*/
        /* Read vehicle filter Return true if vehicle is supported, else false*/
    }

9.

The only way to remove notifications from an OEM App when it connects to another vehicle is to force stop both the services so that both notifications are removed.

From the proposal, I mentioned both the services, one being Router service, and another service started by an App for SDL proxy. The user needs to force stop app from Settings to get rid of these notifications. I do not know, another way to remove the router service notification. If there is any other way to remove these two service notifications mentioned above, please do let me know.

from your previous comment.

The proposal just needs to be updated to reflect that your statement is about the router service, not their SDL Service.

Please let me know what needs to be updated.

17.c

The known issue would be that if an IVI system is shared between multiple models or trims the filter would only be good for make.

Can you please clarify what would be the issue?

17.d This is just a one-time connection step and the delay mentioned would greatly depend on the time required by IVIs to respond to StartService for RPC. However, as mentioned earlier, the Start Service for RPC can be sent via Router service as well. Since SDL Listener is closing BT socket connection and limitation of RFCOMM channels we decided to move this logic to SDL Device Listener. Do you think that the risk will be lower if the Router sends Start Service to find vehicle type?

ashwink11 commented 4 years ago

@Sohei-Suzuki-Nexty

17.e

This flow is only for a BT connection, but how about a USB connection?

The below flow should work for the USB connection as well. Instead of Mac address, the SDK will save USB serial Numbers. Please do let me know if you have any other suggestions.
OEM_APPS_4

Sohei-Suzuki-Nexty commented 4 years ago

17e

The below flow should work for the USB connection as well

I don't know how to make SDL Device Listener start processing from a USB connection. Could you provide the flow to start from "USB Connects"?

theresalech commented 4 years ago

Hi @ashwink11, please find our responses to the open items below. Thank you!

Open Items

4/5. I am against creating differing APIs between the projects if we can avoid it. The vehicle information is sent as an extra in the intent to notify the apps that the router service has made a successful connection. This intent is already passed through the onSDLEnabled method, so developers can use this extra when present to decide when they wish to start their service or not; the library needs to make no changes to enable this beyond the inclusion of the extra. The case where the IVI system is older and does not support this information however will result in a null value and the app must start their service if they wish to connect on potentially acceptable IVI systems. This is where the developer would set their filter callback and it would mimic all platforms. So the suggested change is a necessary aspect of this feature, and developers will still have a chance to avoid starting their service without the changes suggested in your last comment.

6. Based on #5, I think that this point is no longer necessary. The developer would implement the filter callback that we laid out above, and this point can be removed from the proposal. Our new suggested revision for Item 6 is to remove the following language:

The iOS proxy can implement an OnSDLEnabled notification similar to that of the Android proxy to notify the app about SDL connection with the supported vehicle. The app can provide lifecycle configuration to the proxy upon receiving the OnSDLEnabled notification.

9. Your statement says "both services", that is untrue. The SDL Service can be made to have a non-persistent notification by the developer which means the user could swipe away the notification and close the service. Or the developer could add a close option on the notification. The sample notification we put out is only the basics of Android development for a notification. Therefore you need to remove "both" and just say the only way to remove the router service notification is by force closing.

17c. If the same MAC address is used between modules, but the app doesn't want to support SDL on all the systems that implement that module it will be a problem. If Ford had the same IVI system in a Focus and a Fiesta, but didn't get their IVI bluetooth certified and used the same MAC address for all IVI systems installed in those models, the app would not be able to differentiate between the Focus or Fiesta until after connection and thus the problem of notifications appearing might still occur. Obtaining a unique MAC address is part of the bluetooth certification process, but I don't believe we require that modules be bluetooth certified in order to use SDL.

17d. Well it is a bit tricky because the router service itself might open a second bluetooth connection that would need to be closed before a third connection must be established.

17e. The feature will possibly work for SDL Services but not the SDL Router service. The AOA connection will go through a different process to start up the router service and even so, the extremely feeble USB connection will not survive an opening and closing to follow the flow provided here. Other considerations would have to be put in place somehow.

theresalech commented 4 years ago

The Steering Committee voted to keep this proposal in review to allow for further discussion on the review issue about the open items. It was noted that this proposal will likely be in a spot to return for revisions during the next meeting, potentially with the acknowledgement that the remaining open items can be discussed during the review of the revised proposal.

ashwink11 commented 4 years ago

4/5.

This intent is already passed through the onSDLEnabled method, so developers can use this extra when present to decide when they wish to start their service or not; the library needs to make no changes to enable this beyond the inclusion of the extra.

Ok. So we will not change onSDLEnabled API. However, in earlier comments you suggested code regarding setSdlVehicleFilter for Android. I think this code is not required in Android App. Since, app developers are taking decision in OnSDLEnabled based on information provided in Intent extras. I do not see any need to provide setSdlVehicleFilter again in SDK. Also, as mentioned earlier, if App developers use setSdlVehicleFilter API, they would need to start SDL service use this API and hence they would end up showing notifications to users for unsupported vehicle connection.

//BaseSdlManager.Builder
Builder setSdlVehicleFilter(SdlVehicleFilter filter){
    this.sdlVehicleFilter = filter;
    return this;
}
//New interface
public interface SdlVehicleFilter {
    boolean onSdlVehicleConnected(VehicleInfo info);
}

6. Ok. I will remove this statement.

9. Ok. I will change this statement.

17c. I had never come across a scenario where BT MAC address in vehicles are the same. However, even if it does, I believe it would be a very rare case. I think it should be fine if the apps showing notifications in this case.

17d. Ok. I guess we can continue with the SDL device listener.

17e. Please find the flow chart for the USB connection below. For the first time case, the App would find the router service using the logic as today using service version number. For subsequent connections, the saved info will be used to find a supported vehicle.

SDL_RS_OEM_Apps_USB (1)

theresalech commented 4 years ago

Hi @ashwink11, thanks for your responses! Please find an updated summary, including feedback from the PM on open items, below. As mentioned during our last meeting, we would like to ask the Steering Committee to vote to return this proposal for the agreed upon revisions (1, 2, 3, 6, 7, 8, 9, 11, 12, 13, 14, 15, 16, 17a, 17b, 17d, and 19). This will allow SDLC members to have clearer visibility into the current status of the proposal, and be able to informatively speak to the remaining open items (4/5, 17d, 17e, and 18) when the revised proposal is brought back into review.

Author to Revise Items 1, 2, 3, 8, 11, 13, and 16 from this comment.

Items 7, 11, 12, 14, 15, 17a, 17b, and 19 from this comment.

6. Author to remove the following:

The iOS proxy can implement an OnSDLEnabled notification similar to that of the Android proxy to notify the app about SDL connection with the supported vehicle. The app can provide lifecycle configuration to the proxy upon receiving the OnSDLEnabled notification.

9. Author to change:

The only way to remove notifications from an OEM App when it connects to another vehicle is to force stop both the services so that both notifications are removed.

to

The only way to remove the router service notification from an OEM App when it connects to another vehicle is to force close.

17d. Extend the scope of SDL Device Listener as described in https://github.com/smartdevicelink/sdl_evolution/issues/971#issuecomment-654299341

Open Items

4/5.

These reasons make this addition necessary:

Can you please explain specifically why these are not needed?

17e. Where is the serial number coming from? Can you elaborate on how that will get saved by each app and how will that properly be mapped to vehicle info? Will this be a separate preference from the bluetooth MAC being saved?

18. We are looking for input from other SDLC Members on the following:

What is the opinion of the SDLC on the probability that a user will have downloaded OEM specific apps that are not of the OEM their vehicle is from? Have we had any reports from actual customers that this is a problem? Or are we trying to solve a problem that seems like a problem?

Closed - no action required

17c. 👍 we just wanted to make sure this was acknowledged, thanks!

Sohei-Suzuki-Nexty commented 4 years ago

17e In the current process, does the router service start with the USB connection? I'm not sure how the current flow is going and the process will be changed.

ashwink11 commented 4 years ago

4/5.

The vehicle filter code in the builder catches that case.

The App developers already provide vehicle filter XML and mentioned it in the manifest file. The proxy can read this XML file and determine whether vehicle info received in RAIR is supported or not. App developers should not need to provide another list in JAVA and set it in setSdlVehicleFilter as they would end up providing the same list twice, once using XML meta-data in manifest and another list set using setSdlVehicleFilter. Also, if in any case, where the list provided by the app developers in setSdlVehicleFilter does not match the list in the manifest file, it would give rise to issues.

API parity between platforms.

If the parity between platforms is desired, then we can provide Info.plist for iOS as described earlier. In the current proposal, the only reason for proposing Info.plist for the iOS platform was to provide parity between platforms.

17e.

Where is the serial number coming from? Can you elaborate on how that will get saved by each app and how will that properly be mapped to vehicle info? Will this be a separate preference from the Bluetooth MAC being saved?

As per the current implementation, the DEVICE_ADDRESS for connected USB device is resolved as below in the MultiplexUsbTransport class. If Serial Number is available, it will be used. Shared Preferences used are the same for both USB and BT devices. I do not intend to propose any changes in DEVICE_ADDRESS resolution or Shared Prefs.

                connectedDeviceAddress = bundle.getString(SERIAL);
                if(connectedDeviceAddress == null){
                    connectedDeviceAddress = bundle.getString(URI);
                    if(connectedDeviceAddress == null) {
                        connectedDeviceAddress = bundle.getString(DESCRIPTION);
                        if (connectedDeviceAddress == null) {
                            connectedDeviceAddress = bundle.getString(MODEL);
                            if (connectedDeviceAddress == null) {
                                connectedDeviceAddress = bundle.getString(MANUFACTURER);
                            }
                        }
                    }
                }

In the current process, does the router service start with the USB connection? I'm not sure how the current flow is going and the process will be changed.

The USBAccessoryAttachmentActivity is a proxy to listen for USB_ACCESSORY_ATTACHED intents. It finds appropriate Router service to be started. Please refer here. The flow would be slightly changed. However, if you have any suggestions, please do let me know. Link to USB connection Flow.

theresalech commented 3 years ago

The Steering Committee voted to return this proposal for revisions. The author will revise the proposal to incorporate the agreed upon items as summarized in this comment. It was acknowledged that discussion on the current open items (4/5, 17e, and 18) will resume during the review of the revised proposal.

theresalech commented 3 years ago

The author has updated this proposal based on the Steering Committee's agreed upon revisions, and the revised proposal is now in review until October 13, 2020.

The specific items that were updated since the last review can be found in this merged pull request: https://github.com/smartdevicelink/sdl_evolution/pull/1067.

joeljfischer commented 3 years ago

I'm not going to review the Java Suite library changes here, as I do not have sufficient knowledge. I'm also going to restart numbering:

1.

If the vehicle type is supported, the app library should also notify the app about the connected vehicle type so that the app can configure SDL as required.

This seems to be declaring that a new public API will be added, but it's not defined.

2.

If the vehicle type information is not available in StartServiceAck protocol message or the protocol version is less than that supporting the vehicle type info, the SDL app library will continue with the app registration and it will rely on vehicle type information received in the RegisterAppInterface response. If vehicle type is not supported, the exclusive apps will be allowed to unregister from the SDL enabled system.

What does "will be allowed to unregister" mean here? Will the app library do it or is the app responsible for it?'

3.

Above mentioned changes need to be implemented in SDL Core, the Java Suite app library, and the iOS app library.

At least the protocol changes also need implementation in the JavaScript Suite app library, and potentially an additional callback to give them information about what head unit they're connected to.

joeygrover commented 3 years ago

4.

It is necessary to check support for the StartServiceACK protocol message and vehicle type info before notifying the client with SDL enabled callback. If clients are informed before checking the mentioned info, the exclusive apps could end up registering on an unintended SDL enabled IVI system.

This conflicts with the flow chart further down in the proposal. This should read that the developer will be responsible for checking in the onSDLEnabled method against supported vehicle models before starting their SDL service. The router service will still notify all potential clients that the an SDL device has connected and supply the vehicle info for those potential client apps to decide if they wish to start their SDL services.

5.

If the StartServiceACK message does not have vehicle type info or the protocol version is less than that supporting the vehicle type info and if there are multiple SDL apps available on the user's device, the exclusive apps will not host the router service.

So if an app has the manifest entry for this metadata and the StartServiceACK does not contain vehicle data non of those apps should host the router service? This is likely to lead to very poor experiences as older systems don't provide this information so no apps will start the router service even though those systems are from supported OEMs.

6.

If the StartServiceACK message does not have vehicle type info or the protocol version is less than that supporting the vehicle type info and if all SDL apps available on the user's device are exclusive apps, the exclusive apps, in this case, will rely on vehicle type info received in the RegisterAppInterface response. In such a case, if vehicle type is not supported, the exclusive apps will be allowed to unregister apps from the SDL system and stop the Android Router Service. The exclusive app will be only allowed to stop the Android Router Service that it hosts.

I'm still a bit concerned that this will lead to an infinite loop of starting & stopping the router service as it gets the vehicle information. Apps will continually flash on and off the screen as router services are spun up and taken down. I'm not sure of how to better suggest a behavior that will be acceptable to the author, but I would say that the router service should remain up for the remainder of that connection. On second connection the vehicle info should be saved from the RAI response and can follow the previously defined flows. I think while that doesn't prevent all appearances of the OEM exclusive app from appearing it allows a much more graceful approach.

7.

public static final String CONNECTED_VEHICLE_INFO  = "connected_vehicle_info_for_router_service";
public static final String START_ROUTER_SERVICE_SDL_ENABLED_CONNECTED_VEHICLE_INFO  = "connected_vehicle_info";

I don't think these need to be different constants. Basically this is just a key in the bundle stating there is an extra in this intent by this name acting as a key value pair. The intent action should supply the contextual information of how to use this data. It should really just be public static final String VEHICLE_INFO = "vehicle_info";

Sohei-Suzuki-Nexty commented 3 years ago

17e

The USBAccessoryAttachmentActivity is a proxy to listen for USB_ACCESSORY_ATTACHED intents. It finds appropriate Router service to be started.

Thank you for replying. I understand.

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review, so that the author can respond to the comments provided on the review issue before the next meeting.

ashwink11 commented 3 years ago

Thank you for the review. Please find my comments below.

  1. In the previous version of this proposal, I proposed to implement a connection callback function "onSDLEnabled" similar to the Android App library. However, in our previous discussions items number 4/5 were open items, which would change the requirement of "onSDLEnabled" callback. And hence, I removed requirement of "onSDLEnabled" callback in iOS App library, since open items were not resolved. If you could confirm that items number 4/5 from previous discussions are now resolved, I would like to propose the implementation of SDL connection callback before setting the SDL lifecycle configuration. Please do let me know if you have any other suggestions.

  2. I meant that App should be responsible for making decisions whether they want to stay connected or unregister from the system. The App library should not do it on its own. I can rephrase it if it's required.

  3. I agree.

  4. Ok. I will change this as per recommendations.

  5. I was actually referring to a case where the users' device has many SDL apps which are both exclusive and non-exclusive SDL apps. And if the StartServiceACK message does not have vehicle type info or the protocol version is less than that supporting the vehicle type info, the exclusive apps will not start router service. However, non-exclusive apps could still start a router service. If all the SDL-enabled apps on users' device are exclusive apps, one of the exclusive apps will have to start a router service.

  6. Ok. I will change this as per recommendations that the router service should remain up for the remainder of that connection. On the second connection the vehicle info should be saved from the RAI response and can follow the previously defined flows.

  7. Ok. I will remove them and add just one constant public static final String VEHICLE_INFO = "vehicle_info";

joeygrover commented 3 years ago

4. 👍

5. This ties into number 6 a little bit. I believe the case should be if the SS_ACK does not contain vehicle info, the protocol version is less than that of the support of this feature, and no vehicle data is saved regarding this connected device the normal flow will be followed. However, after the session, once vehicle data is cached from the RAI response, it should be used in determining which app should start the router service.

6. 👍

7. 👍

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review, to allow for discussion to continue on the review issue.

ashwink11 commented 3 years ago
  1. I agree, I will change it as per suggestions.
joeljfischer commented 3 years ago

1. I apologize, it's been so long since this proposal was in review that I missed there are still open items. @joeygrover and may have to discuss this further internally. I think we can combine 4/5 into this new (1), and I would note that I don't believe this proposal can be accepted until this is resolved because the proposal is unimplementable.

2. Okay, so this is a bit confusing and relates to (1). If an app is able to unregister, you have to specify how the app will know that they can unregister, and the mechanism for doing so. I assume there's a callback to tell the developer that the app isn't connected to one of the specified vehicles. If you want the app to unregister, they also need to know where they are in the process and how to unregister. This section definitely needs a full rewrite to clarify when this will happen, and by what mechanism.

3. 👍

Let's make 17d into 8, 17e into 9, and 18 into 10.

I believe @joeygrover needs to talk about 8 / 9, and we are still looking for SDLC feedback on 10.

ashwink11 commented 3 years ago

@joeljfischer Please refer this comment. The open items are 4/5, 17e and 18 only from previous discussions. I believe 17d is resolved. image

Also, we already received response for 17e from previous discussions as well. Please refer comment here.

joeygrover commented 3 years ago

8. (formally 17d.) I believe we've agreed to the potentially solution here.

9. (formally 17e.) This item is still open, I missed it previously. The comment left does not fully answer the questions around how AOA will use this feature. The author included a code snippet of using different parameters that come with the AOA device on connection. However, these parameters are not useable for this purpose. All AOA devices that SDL connects with will have the same manufacturer, model, and version.

<usb-accessory
    manufacturer="SDL"
    model="Core"
    version="1.0" />

This means that based on the first IVI system the phone connects with it could associate that IVI's OEM make/model with all SDL AOA devices which is not an acceptable use case. The other parameters are not well regulated and could range from many different values including overlapping ones from different OEMs and thus putting the library in the same state (first IVI is assumed for all AOA devices). I'm not sure this feature can be used with AOA only.

Secondly, this would again go with 5/6 if this aspect were to move forward, the router service should stay running if it received an AOA device. As stated previously, this process is very fragile and it would not survive the opening and closing of it's file descriptor to then start another router service to try and do the same.

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review, to allow for further discussion on the review issue. It was noted that the following items are still in discussion: 1 (including Items 4/5 from past review), 2, 9 (formerly 17e), and 10 (formerly 18).

The status of Items 1, 2, and 9 can be found in the last few comments on this review issue. SDLC members should provide input on Item 10, below:

What is the opinion of the SDLC on the probability that a user will have downloaded OEM specific apps that are not of the OEM their vehicle is from? Have we had any reports from actual customers that this is a problem? Or are we trying to solve a problem that seems like a problem?

ashwink11 commented 3 years ago

9. I was not aware that all AOA devices that SDL connects with will have the same manufacturer, model, and version. In that case, I agree that the Android app library will not be able to differentiate between SDL-enabled IVI systems before starting SDL router service. Also, as we discussed previously in 5/6 that router service will be allowed to continue running, it would not be possible to use this feature for AOA. However, we can try to mitigate this issue. If any non-exclusive SDL apps are available on the user's device, then the Android app library will use it for starting router service on an AOA connection. If all apps installed on users' devices are exclusive apps, then one of the exclusive apps will have to start SDL router service on an AOA connection. Please do let me know your thoughts.

ashwink11 commented 3 years ago

2. As mentioned in Point 1, if OnSDLEnabled callback is implemented and the proposed StartServiceAck protocol message is supported by the connected vehicles, it will have vehicle Type info. If the proposed StartServiceAck protocol message is not supported by the connected vehicles, OnSDLEnabled will not have vehicle type Info and hence the app will rely on Register App Interface response.

If the vehicle type information is not available in StartServiceAck protocol message or the protocol version is less than that supporting the vehicle type info, the SDL app library will continue with the app registration and it will rely on vehicle type information received in the RegisterAppInterface response. On receiving vehicle type info, if an app determines that the vehicle is not supported, it will send an unregister app interface as shown below.

iOS:

- (void)hmiLevel:(SDLHMILevel)oldLevel didChangeToLevel:(SDLHMILevel)newLevel {
    if (![newLevel isEqualToEnum:SDLHMILevelNone] && ([self.firstHMILevel isEqualToEnum:SDLHMILevelNone])) {
      if([self checkIfVehicleTypeSupported: vehicleTypeFromRegisterAppInterface]){
            SDLRPCRequest* rpc = SDLUnregisterAppInterface()
            sdlManager.send(rpc)
         }
         .
         .
    }
theresalech commented 3 years ago

Please find a summary of the items in discussion, including new responses from the PM, below:

Open Items

1/2. We believe that the best API is one that allows the developer to receive information from either the protocol StartServiceACK or from the RegisterAppInterfaceResponse. The information is then sent to the SDLManagerDelegate / SdlManagerListener for the developer to handle.

iOS:

- (BOOL)didReceiveVehicleType:(SDLVehicleType *)type;

Java Suite:

boolean onVehicleTypeReceived(VehicleType type);

JavaScript Suite:

     /**
     * A way to determine if this SDL session should continue to be active while
     * connected to the determined vehicle type.
     * @param {SdlManager} sdlManager - A reference to an SdlManager instance.
     * @param {VehicleType} vehicleType - the type of vehicle that this session is currently active on.
     * @returns {Boolean} true if this session should continue, false if the session should end
     */
    onVehicleTypeReceived (sdlManager, vehicleType) {}

If the StartServiceACK comes in with vehicle data, this method will be called with the vehicle type information. If the developer responds true, the session continues, if false, the session ends with an EndService sent by the library. When the RegisterAppInterfaceResponse happens, if the this method will not be called a second time. If the StartServiceACK doesn’t have the vehicle data, when the RegisterAppInterfaceResponse comes in, this above methods will be called. If the developer responds with false, the library will send an UnregisterAppInterface request.

9. The problem with this is that even if the app is supposed to be exclusive to an OEM, the connecting USB device might just be one of their accepted vehicles. By attempting to avoid using those apps we are weakening the router service's ability to fix problems through the versioning system. So we would have to weight whats more important: not having apps host the router service that are supposed to be exclusive, or hinder our ability to fix transport problems quickly for all apps through the router service versioning. I wish I had a better alternative to help address this, but AOA is very difficult to deal with.

10. Awaiting input from SDLC Members on the following:

What is the opinion of the SDLC on the probability that a user will have downloaded OEM specific apps that are not of the OEM their vehicle is from? Have we had any reports from actual customers that this is a problem? Or are we trying to solve a problem that seems like a problem?

Author to Revise

3.

Above mentioned changes need to be implemented in SDL Core, the Java Suite app library, and the iOS app library.

Update to "Above mentioned changes need to be implemented in SDL Core, the Java Suite app library, the iOS app library, and the JavaScript Suite app library."

4.

It is necessary to check support for the StartServiceACK protocol message and vehicle type info before notifying the client with SDL enabled callback. If clients are informed before checking the mentioned info, the exclusive apps could end up registering on an unintended SDL enabled IVI system.

This conflicts with the flow chart further down in the proposal. This should read that the developer will be responsible for checking in the onSDLEnabled method against supported vehicle models before starting their SDL service. The router service will still notify all potential clients that the an SDL device has connected and supply the vehicle info for those potential client apps to decide if they wish to start their SDL services.

5.

If the StartServiceACK message does not have vehicle type info or the protocol version is less than that supporting the vehicle type info and if there are multiple SDL apps available on the user's device, the exclusive apps will not host the router service.

Update to state that if the SS_ACK does not contain vehicle info, the protocol version is less than that of the support of this feature, and no vehicle data is saved regarding this connected device, the normal flow will be followed. However, after the session, once vehicle data is cached from the RAI response, it should be used in determining which app should start the router service.

6.

If the StartServiceACK message does not have vehicle type info or the protocol version is less than that supporting the vehicle type info and if all SDL apps available on the user's device are exclusive apps, the exclusive apps, in this case, will rely on vehicle type info received in the RegisterAppInterface response. In such a case, if vehicle type is not supported, the exclusive apps will be allowed to unregister apps from the SDL system and stop the Android Router Service. The exclusive app will be only allowed to stop the Android Router Service that it hosts.

Update to reflect that the router service should remain up for the remainder of that connection. On the second connection the vehicle info should be saved from the RAI response and can follow the previously defined flows.

7.

public static final String CONNECTED_VEHICLE_INFO = "connected_vehicle_info_for_router_service"; public static final String START_ROUTER_SERVICE_SDL_ENABLED_CONNECTED_VEHICLE_INFO = "connected_vehicle_info";

Update to one constant: public static final String VEHICLE_INFO = "vehicle_info";

Closed - No Action Required 8. It's been determined this was incorporated into the current proposal in the last set of revisions.

ashwink11 commented 3 years ago

1/2. I believe this suggestion would work only for iOS and Javascript and not for Android. Please refer to our previous discussions regarding Android App potentially showing notification on connection with unsupported SDL-enabled IVI system. Also, we have already worked a lot on Android Javasuite design and all our efforts will go in vain if we have to start over. I would like to refrain from changing agreed-upon items with respect to Javasuite. However, I like your suggestions for iOS and Javascript and I think it would work for these two SDKs.

9. I agree with you. Exclusive apps could have a USB capability that might help to fix problems through the router service's versioning system. I believe we should then give weightage to fixing USB transport problems. If you agree, I would add that this proposal is not applicable for apps registered through AOA.

Edit: added item 9

joeljfischer commented 3 years ago

1/2. I apologize for the confusion. This comment isn't meant as a replacement for the Java Suite SDK's router service-based implementation, but as an addition to it. The three libraries shouldn't have divergent APIs and the API proposed could be useful for Java Suite even despite also having the router service block.

joeygrover commented 3 years ago

1/2. This API would need to be present for the Java SE and Java EE libraries. Because those libraries don't use the Android components it is necessary that we include the API into the Java Suite library for them. Android developers will still be able to use the vehicle data present in the intent to start their service, but will also have access to this API.

joeygrover commented 3 years ago

9. 👍 Yea I think because of AOA being so fragile it's best to hold off on implementing this for it at this time.

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review, to allow for more discussion between the author and Project Maintainer on Items 1/2 and 9, and to provide the opportunity for all SDLC Members to provide their input on Item 10.

Sohei-Suzuki-Nexty commented 3 years ago

10

What is the opinion of the SDLC on the probability that a user will have downloaded OEM specific apps that are not of the OEM their vehicle is from?

Am I correct in understanding that the intent of this question is to ask which of the following thoughts about the possibility of users downloading OEM-specific apps other than their own cars? "We think it's not a problem because it's unlikely" or "We think it's a problem because it's likely".

ashwink11 commented 3 years ago

1/2. Thank you for clarification. I will add it to the proposal.

vladmu commented 3 years ago

Hi, I look at the proposal and don't find the section regarding JS Suite changes. Only Android and iOS are described. Is it possible to add such a section to the proposal, especially regarding supported vehicle type files?