{"action":"created","issue":{"url":"https://api.github.com/repos/smartdevicelink/sdl_android/issues/919","repository_url":"https://api.github.com/repos/smartdevicelink/sdl_android","labels_url":"https://api.github.com/repos/smartdevicelink/sdl_android/issues/919/labels{/name}","comments_url":"https://api.github.com/repos/smartdevicelink/sdl_android/issues/919/comments","events_url":"https://api.github.com/repos/smartdevicelink/sdl_android/issues/919/events","html_url":"https://github.com/smartdevicelink/sdl_android/issues/919","id":378400791,"node_id":"MDU6SXNzdWUzNzg0MDA3OTE=","number":919,"title":"Secondary transport is not set up as soon as its available","user":{"login":"mrapitis","id":8443128,"node_id":"MDQ6VXNlcjg0NDMxMjg=","avatar_url":"https://avatars1.githubusercontent.com/u/8443128?v=4","gravatar_id":"","url":"https://api.github.com/users/mrapitis","html_url":"https://github.com/mrapitis","followers_url":"https://api.github.com/users/mrapitis/followers","following_url":"https://api.github.com/users/mrapitis/following{/other_user}","gists_url":"https://api.github.com/users/mrapitis/gists{/gist_id}","starred_url":"https://api.github.com/users/mrapitis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mrapitis/subscriptions","organizations_url":"https://api.github.com/users/mrapitis/orgs","repos_url":"https://api.github.com/users/mrapitis/repos","events_url":"https://api.github.com/users/mrapitis/events{/privacy}","received_events_url":"https://api.github.com/users/mrapitis/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-11-07T17:52:24Z","updated_at":"2018-11-08T01:41:38Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"### Bug Report\r\nThe secondary transport is not set up as soon as the proxy is notified of available transport types.  The accepted multiple transport proposal (linked below) indicates that \"Proxy should set up Secondary Transport as soon as it is notified of available transport types for Secondary Transport and information that is necessary to set up the transport becomes available.\"\r\nhttps://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0141-multiple-transports.md\r\n\r\nIt is understood that best practices indicate that wifi sockets should only be opened and used when required to reduce network battery drain as indicated in the link below, however this issue is being reported because the current Android proxy implementation does not follow the accepted proposal above.\r\nhttps://developer.android.com/topic/performance/power/network/\r\n\r\n\r\n##### Reproduction Steps\r\nDownload and build the Android 4.7.1 proxy and build the SdlStreaming sample application.\r\nRegister the sample application with SDL Core, ensure SDL Core logs have been turned on.\r\nVia SDL Core logs take note that the secondary transport is only registered just before the application starts a nav or pcm service (as opposed to when the application initially registered).\r\n\r\n##### Expected Behavior\r\nThe secondary transport will be registered after its notified of available transport types (based on accepted proposal).\r\n\r\n##### Observed Behavior\r\nThe secondary transport is only registered just before the application starts a nav or pcm service.\r\n\r\n##### OS & Version Information\r\n* Android Version: Android 9\r\n* SDL Android Version: 4.7.1\r\n* Testing Against: SDL Core 5.0.0, WebHMI, & Proprietary Head Unit Implementation\r\n\r\n##### Test Case, Sample Code, and / or Example App\r\nSdlStreaming"},"comment":{"url":"https://api.github.com/repos/smartdevicelink/sdl_android/issues/comments/436842535","html_url":"https://github.com/smartdevicelink/sdl_android/issues/919#issuecomment-436842535","issue_url":"https://api.github.com/repos/smartdevicelink/sdl_android/issues/919","id":436842535,"node_id":"MDEyOklzc3VlQ29tbWVudDQzNjg0MjUzNQ==","user":{"login":"shoamano83","id":26382896,"node_id":"MDQ6VXNlcjI2MzgyODk2","avatar_url":"https://avatars1.githubusercontent.com/u/26382896?v=4","gravatar_id":"","url":"https://api.github.com/users/shoamano83","html_url":"https://github.com/shoamano83","followers_url":"https://api.github.com/users/shoamano83/followers","following_url":"https://api.github.com/users/shoamano83/following{/other_user}","gists_url":"https://api.github.com/users/shoamano83/gists{/gist_id}","starred_url":"https://api.github.com/users/shoamano83/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/shoamano83/subscriptions","organizations_url":"https://api.github.com/users/shoamano83/orgs","repos_url":"https://api.github.com/users/shoamano83/repos","events_url":"https://api.github.com/users/shoamano83/events{/privacy}","received_events_url":"https://api.github.com/users/shoamano83/received_events","type":"User","site_admin":false},"created_at":"2018-11-08T01:41:38Z","updated_at":"2018-11-08T01:41:38Z","author_association":"CONTRIBUTOR","body":"Hi Laura and Joey, I'm copying our dialog over e-mails.\r\n\r\nOn Thu, Nov 1, 2018 at 4:23 AM Sho Amano <samano@xevo.com> wrote:\r\n\r\n> I'm working with Ford team to investigate an issue regarding sequence of Wi-Fi secondary\r\n> transport registration. I have created a fix for it, but before submitting a PR I would like to\r\n> discuss with you. Since it's likely a sequence change, I want to make sure it will align with\r\n> your thought.\r\n>  \r\n> I would appreciate if you could go through the descriptions of our issue and the change. If you\r\n> have any feedback on it - whether it's acceptable or you have any concerns, please kindly share.\r\n> \r\n> ## Summary of the issue\r\n> \r\n> The proposal of multiple transports (https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0141-multiple-transports.md) states:\r\n> \r\n> > Starting Secondary Transport\r\n> >\r\n> > Proxy should set up Secondary Transport as soon as it is notified of available transport types for Secondary Transport and information that is necessary to set up the transport becomes available.\r\n> \r\n> However, Android library (as of v4.7.1) doesn't set up secondary transport when it receives\r\n> Transport Event Update frame with valid IP address and port number. Instead, it initiates\r\n> the secondary transport when a NAV or PCM service is requested:\r\n> https://github.com/smartdevicelink/sdl_android/blob/master/sdl_android/src/main/java/com/smartdevicelink/protocol/SdlProtocol.java#L782\r\n> So the behavior doesn't align with the proposal.\r\n> \r\n> ## Why it's a problem\r\n> \r\n> Ford's HMI limits a streaming app to work only with USB transport or Wi-Fi secondary transport. For this purpose, it uses\r\n> the registration state of TCP secondary transport. When the secondary transport isn't registered, HMI doesn't allow\r\n> the app to go into FULL level.\r\n>\r\n> Unfortunately this design doesn't work well with current Android library. Since HMI doesn't make the app FULL, the app\r\n> will not initiate streaming, and it won't call startRemoteDisplayStream(). So NAV/PCM services won't be started, which\r\n> in turn will not initiate secondary transport connection/registration.\r\n> \r\n> ## Solution\r\n> \r\n> My solution is to add a trigger of TCP transport connection and secondary transport registration after receiving\r\n> Transport Event Update frame. This behavior should align with the proposal, and also with iOS library.\r\n> \r\n> You can find my fix here:\r\n> https://github.com/XevoInc/sdl_android/tree/fix/secondary_transport_sequence_3\r\n> \r\n> I tried to make the change as minimum as possible so that hopefully it won’t break anything.\r\n> \r\n> The only concern is that this is a change in sequence. If there is an app / a HMI that relies on current sequence,\r\n> this change may give negative impact. If you know any background of current sequence, or have any concerns,\r\n> please kindly share.\r\n\r\n\r\nJoey replied:\r\n\r\n> Hi Sho,\r\n> \r\n> I understand the concern. The issue is that all apps will try to open a TCP connection regardless of one ever being used or not. This is very irresponsible from a coding and resource perspective; the library would create and maintain a TCP connection regardless of any intention of ever being used. The sequence works with the open source Core library as well so it has proven to be successful with no limitations. Since the library itself is already in production I would recommend investigating a sequence change on Ford's software. It seems as though an arbitrary sequence that can be corrected on the hardware with limited impact where changing the library will have a resource and processing power impact which looks bad to developers.\r\n\r\n\r\nOn Fri, Nov 2, 2018 at 2:29 AM Sho Amano <samano@xevo.com> wrote:\r\n\r\n> Hi Joey and Markos,\r\n> \r\n> Thank you for your feedback Joey.\r\n> I do not own Ford's head unit code so I must ask Ford team whether such change in\r\n> head unit is acceptable. Markos, do you have a good idea on how to change the\r\n> sequence on head unit?\r\n>  \r\n> Actually I do understand Joey’s concern that with my change, even a non-streaming\r\n> app will set up a TCP secondary transport which is waste of resources. iOS library has\r\n> less impact, as it sets up TCP transport only when the app is in foreground. All the\r\n> other apps in background don't set up it.\r\n>  \r\n> If that's the major concern, then an alternative is to trigger TCP secondary transport\r\n> connection and registration with Transport Event Update frame only when\r\n> _requiresHighBandwidth_ is true. Therefore the transport will be established with\r\n> streaming apps only. I think this also comes with a small benefit that secondary\r\n> transport will be set up prior to the app being FULL, so streaming would be started\r\n> immediately once the app would get FULL and be shown.\r\n> To Joey and Markos, do you have thoughts on this?\r\n\r\nJoey replied:\r\n\r\n> Hi Sho,\r\n> \r\n> I think the second solution is better, but it will still connect over a transport with no guarantee that it will be used. For example, a user connecting over bluetooth and then TCP automatically every time they get into the car even though most of the time they will likely not need the video streaming service. This will drain battery as well especially if the user gets in and out of their car a lot, or even when running errands.  \r\n> \r\n> Also, the requiresHighBandwdith flag can still be set to false and still intend to stream. The flag essentially allows developers to create a low bandwidth implementation and a high bandwidth one of they wish. For example, simple turn by turn when only low bandwidth is available, and then do actual streaming when high is available. If a developer doesn't want to do that, they set the flag to true and will only register when a high bandwidth is available (either already connected or can be connected). Also we haven't really noticed any performance issues in terms of the time taken to wait until a streaming service wishes to be started instead of automatically when info is available."}}
{"actor":{"display_login":"shoamano83"}}