SDL stop sending OnInteriorVehicleData notifications after the app sent a GetInteriorVehicleData without optional parameter subscribe

[yang1070]

Bug Report

SDL stop sending OnInteriorVehilceData notifications after the app sent a GetnInteriorVehilceData without optional parameter subscribe.

In the remote control baseline proposal 0099. We have the following items
-API to read RC module status data. A RC application needs to send one request per RC module to read data from multiple modules.
-API to subscribe RC module status/setting change notifications. See parameter subscribe and isSubscribed in the above GetInteriorVehicleData RPC.
-API to unsubscribe RC module status/setting change notifications. See parameter subscribe and isSubscribed in the above GetInteriorVehicleData RPC.

In release 4.5, there is no way for mobile application to send a GetnInteriorVehilceData without subscribe or unsubscribe due to the default value of parameter subscribe is set to false in the mobile api.

Reproduction Steps

1. Build and run sdl core and sdl hmi
2. register and activate a remote control app from hmi
3. from the app, sends a GetnInteriorVehilceData RPC with moduleType=CLIMATE and subscribe=true, the app gets the climate control data, and starts receiving change notifications if there is any change on HMI.
4. from the app sends sends another GetnInteriorVehilceData RPC with moduleType=CLIMATE without parameter subscribe to get the current climate control data again

Expected Behavior

SDL will continue send OnInInteriorVehilceData notifications to the app if there is any changes on HMI side

Observed Behavior

SDL unsubscribe the module data and stop sending OnInInteriorVehilceData.

OS & Version Information

• Linux 16.04
• SDL Core Version: 4.5
• Testing Against: RC climate control

[yang1070]

It was fixed in the develop branch with mobile api and hmi api change.

     <param name="subscribe" type="Boolean" mandatory="false" since="5.0">
        <description>
            If subscribe is true, the head unit will register onInteriorVehicleData notifications for the requested moduelType.
            If subscribe is false, the head unit will unregister onInteriorVehicleData notifications for the requested moduelType.
           If subscribe is not included, the subscription status for the requested moduelType will remain unchanged.
        </description>
        <history>
            <param name="subscribe" type="Boolean" mandatory="false" defvalue="false" since="4.5" until="5.0"/>
        </history>

But the fix is not included in the 5.0 release candidate

[theresalech]

@yang1070 as this is a change to the original Remote Control proposal (https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0071-remote-control-baseline.md), can you please submit an Evolution Proposal to revise SDL 0071 to include this change?

[joeygrover]

As we discussed this internally we came up with the follow recommendations:

The defValue inclusion created unintended behavior from inception. This means that the 4.5 spec would require a hotfix to remove the defValue tag and update the description. In the 5.0 RPC spec we will add the history tag for the 4.5 to 4.5.1 change. This would require the Core releases to be updated that used the 4.5 spec. This obviously takes a bit of effort, but makes the most sense.

If this change intends to agree that the initial behavior works as designed, but now it should be changed, then the 5.0 spec will be updated as stated in this issue. The proxies would have to handle the change that from 4.5 to 5.0 the subscribe value changes behavior based on inclusion. This would require the proxies to maintain a cached state of subscriptions per module. This is less than ideal and could become messy.

The proposal should include this information on which path forward is to be agreed upon.

[yang1070]

@theresalech @joeygrover I can work on create a proposal for the change. But I'm confused by Joey's comments. I think it is bug of the original proposal. The proposal clearly list we have 3 functions. But the default value caused one of them not possible.

[joeygrover]

According to the spec

        <param name="subscribe" type="Boolean" mandatory="false" defvalue="false">
            <description>
                If subscribe is true, the head unit will register onInteriorVehicleData notifications for the requested moduelType.
                If subscribe is false, the head unit will unregister onInteriorVehicleData notifications for the requested moduelType.
            </description>
        </param>
    </function>

It does not specifically call out that not including the param has no effect and with the defValue inclusion it means that if a value is not supplied, false will be used.

EDIT: So with that in mind, only option 1 is really available as it would be considered the defValue was included by accident from the original proposal. But since we've released the RPC in 4.5 and Core versions, we have to go back and fix those releases with hotfixes.

[yang1070]

@joeygrover Yes, You are correct regarding the xml spec. The problem is that xml spec is not consistent with the contents/wording in the proposal. The proposal called out 3 related APIs shall be provided

• API to read RC module status data.
• API to subscribe RC module status/setting change notifications.
• API to unsubscribe RC module status/setting change notifications.
  The defvalue="false" has the side effect to eliminate the first API.

Anyway, we need a proposal. But I'm confused with way to go.

[joeygrover]

Right, I understand the description in the proposal. I'm saying the XML for the RPC spec has an error because the description in the proposal does not match the actual XML in the RPC spec. We already released a version of the RPC spec and core so that behavior is in production. We can't just change the spec in 5.0. We have to retroactively fix the errors which means hot fixes for the RPC spec and Core. The new proposal needs to state that that is required.

[yang1070]

@joeygrover Thank you for the clarification. Now I understand.

[yang1070]

@joeygrover if we propose to remove the default value, and use the first approach in the proposal, i.e. make a hot fix for 4.5, does that mean 5.0 will inherit the fix? Or 5.0 will still keep the default value?

[joeygrover]

Since 5.0 hasn't been released it will inherit the change but we will add the history entry in the 5.0 spec since the mobile versioning was added in 5.0.

[yang1070]

@joeygrover @theresalech I have create a PR on this issue smartdevicelink/sdl_evolution#605, please review

[theresalech]

PR to revise SDL 0071 has been accepted by the Steering Committee and merged (https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0071-remote-control-baseline.md). This change should now be implemented in Core and the RPC Spec, per the below:

• Hotfix to RPC Spec (4.5.1) to remove defValue tag and update the description
• History tag in RPC Spec 5.0 (scheduled to release 2018-10-19) for 4.5 to 4.5.1 change
• Update Core releases that used RPC 4.5 Spec.
• Update Core 5.0 (scheduled to release 2018-10-19) to use new RPC Spec 5.0, with defValue tag removed from OnInteriorVehicleData and parameter description updated