[SDL-0296] Possibility to update video streaming capabilities during ignition cycle #3468

LitvinenkoIra · 2020-07-31T10:48:45Z

Fixes #3465

This PR is [ready] for review.

Risk

This PR makes [minor] API changes.

Testing Plan

smartdevicelink/sdl_atf_test_scripts#2437

Summary

New additionalVideoStreamingCapabilities parameter of VideoStreamingCapability processing

Store additionalVideoStreamingCapabilities from UI.GetCapabilities response
Send additionalVideoStreamingCapabilities to the mobile app via GetSystemCapabilities
Resend additionalVideoStreamingCapabilities to the mobile app via OnSystemCapabilityUpdated notification from HMI

OnSystemCapabilityUpdated with videoStreamingCapabilities processing

The notification from HMI which contains appID parameter must be re-sent to appropriate mobile application only if it had been subscribed on it else it must be ignored
The notification from HMI which does not contain appID parameter must be ignored

OnAppCapabilityUpdated notification processing

The notification from the mobile must be re-sent to the HMI

Rpc_spec

smartdevicelink/rpc_spec#272

CLA

I have signed the CLA

…c repo

…HMI_API

…tification

Currently InterfaceGenerator is not able to generate recursive structures i.e. one or several structure attributes have its own type e.g.: struct VideoStreamingCapability { ... VideoStreamingCapability additionalVideoStreamingCapabilities; } because structure type fully defined only when all of it attributes are defined. Otherwise param with type of structure will have AlwaysFalseSchema for validation so this param will never be validated successfully. With this commit InterfaceGenerator is able to handle such problem.

Add the checking of the SystemCapabilityType::VIDEO_STREAMING in the message and setting it into the HMICapabilities.

.../rpc_plugins/sdl_rpc_plugin/src/commands/mobile/on_system_capability_updated_notification.cc

src/components/application_manager/test/include/application_manager/commands/commands_test.h

LitvinenkoIra · 2020-08-18T15:24:18Z

@atiwari9 please review this PR

atiwari9

Approved changed with below commits:

Logs:
2020-08-19_15-57-51.zip

TOTAL: 44
PASSED: 42
FAILED: 0
ABORTED: 0
SKIPPED: 2

SDL Core: 36f2f377929c52f84cb4b4216795fcc5c96f58b4
RPC Spec: 4d1312648b78b69f881d97b4b9fd0d8291736750
SDL HMI: 788499f2078e522d710dce3f413045277b278785
Test Scripts: e6d7f578d52c3b932bc2c5ed9465e9d0c8fd8979

Please resolve conflicts before merging.

…bility_to_update_video_streaming_capabilities

jordynmackool · 2020-12-07T20:18:02Z

@LitvinenkoIra, please advise when this PR is ready for Livio review. Thanks!

…nflicts resolving

…bility_to_update_video_streaming_capabilities

LitvinenkoIra · 2020-12-18T08:25:27Z

@jordynmackool @JackLivio This PR is ready for Livio review. Thank you!

...l_rpc_plugin/include/sdl_rpc_plugin/commands/hmi/bc_on_app_capability_updated_notification.h

...ins/sdl_rpc_plugin/src/commands/hmi/on_bc_system_capability_updated_notification_from_hmi.cc

...c_plugins/sdl_rpc_plugin/test/commands/hmi/bc_on_app_capability_updated_notification_test.cc

This reverts commit 42721d5.

…date_video_streaming_capabilities_testing Fix resolutions switching on streaming

…ility_to_update_video_streaming_capabilities

dboltovskyi · 2021-01-29T16:04:24Z

@JackLivio Please notice all required updates are made and this PR is ready for a new iteration of review.

Jack-Byrne · 2021-01-29T16:33:20Z

src/components/application_manager/src/application_manager_impl.cc

@@ -3556,9 +3556,7 @@ void ApplicationManagerImpl::ForbidStreaming(
 }

 void ApplicationManagerImpl::OnAppStreaming(
-    uint32_t app_id,
-    protocol_handler::ServiceType service_type,
-    const Application::StreamingState new_state) {


I think the old OnAppStreaming should be marked deprecated with functionality left as is. A new OnAppStreaming method can be created with the new signature/functionality.

This is to prevent a major version change for Core since OnAppStreaming is a public method in application manager.

Done in 3d6a459

Jack-Byrne · 2021-01-29T16:38:09Z

@dboltovskyi @AKalinich-Luxoft @LitvinenkoIra

Thank you for the update. The commit history is a little muddy with the different merges. Was the fix to revert the changes from #3488 ?

Do we need to reexamine the issue here? #3160

dboltovskyi · 2021-01-29T17:12:12Z

@JackLivio
Initial fix for #3160 was reverted since it introduced #3603.
Another version of fix has been implemented in a6a06d4 and 89e27c5 which covers both these issues altogether.

We have re-tested both of them and it looks they were fixed properly.

src/components/application_manager/include/application_manager/application_manager_impl.h

...c_plugins/sdl_rpc_plugin/test/commands/hmi/bc_on_app_capability_updated_notification_test.cc

src/components/media_manager/include/media_manager/streamer_adapter.h

...ger/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/bc_on_app_capability_updated_notification.cc

AKalinich-Luxoft · 2021-02-01T13:30:24Z

@dboltovskyi @AKalinich-Luxoft @LitvinenkoIra

Thank you for the update. The commit history is a little muddy with the different merges. Was the fix to revert the changes from #3488 ?

Do we need to reexamine the issue here? #3160

@JackLivio let me bring some clarity here.
As we noticed, fix #3488 provided in release 7.0 is causing an issue #3603. That fix itself was to replace a similar fix #3057 with a new one. Both fixes are doing the same - hold the connection channel (pipe or socket) opened even if there is no data received from mobile. But the logic is different - old fix is doing this by guessing a playback time of received audio data and holds the connection opened for the calculated time. The new fix is just holds the connection opened until mobile explicitly sends StopService, however such approach is not always working. Especially for the audio streaming where reconnect is necessary. That's why there was decided to revert #3488 and return back the old version of the fix. From the point of view of #3160 that is fine because an old fix (#3057) solves that issue too. Just a small adjustments were done in a6a06d4 to apply the old fix for all transport types.

…bility_to_update_video_streaming_capabilities

…ility_to_update_video_streaming_capabilities

IGapchuk and others added 12 commits July 23, 2020 11:26

Temporary commit to change URL of rpc_spec repository

fa49029

Change rpc_spec reference according to the new changes in the rpc_spe…

decd04a

…c repo

Add the new OnAppCapabilityUpdated RPC with appropriate types to the …

9ea01b6

…HMI_API

Add the smart objects keys for the new parameters

36e891b

Add OnAppCapabilityUpdatedNotification and BCOnAppCapabilityUpdatedNo…

ec4bef8

…tification

Add Unit tests for the OnAppCapabilityUpdated RPCs

e2ac645

Update OnBCSystemCapabilityUpdatedNotificationFromHMITest Unit tests

94577fc

Add the checking of the SystemCapabilityType::VIDEO_STREAMING in the message and setting it into the HMICapabilities.

Add additionalVideoStreamingCapabilities to default hmi_capabilities

359d596

Add Processing OnSystemCapabilityUpdated with videoStreamingCapabilities

2e2f1ef

Update Unit tests for OnSystemCapabilityUpdated notifications

995f8dc

Add app_id to OnAppCapabilityUpdated notification to HMI

95eca48

LitvinenkoIra commented Aug 17, 2020

View reviewed changes

.../rpc_plugins/sdl_rpc_plugin/src/commands/mobile/on_system_capability_updated_notification.cc Outdated Show resolved Hide resolved

LitvinenkoIra commented Aug 17, 2020

View reviewed changes

src/components/application_manager/test/include/application_manager/commands/commands_test.h Show resolved Hide resolved

Updates during code review

36f2f37

atiwari9 approved these changes Aug 19, 2020

View reviewed changes

kboskin mentioned this pull request Nov 13, 2020

Feature/0296 possibility to update video streaming capabilities smartdevicelink/sdl_java_suite#1434

Merged

4 tasks

LitvinenkoIra added 2 commits December 1, 2020 12:47

Merge remote-tracking branch 'origin/develop' into feature/0296_possi…

752f3bc

…bility_to_update_video_streaming_capabilities

Update logger

a36ff38

LitvinenkoIra force-pushed the feature/0296_possibility_to_update_video_streaming_capabilities branch 2 times, most recently from d61e005 to 618658a Compare December 5, 2020 12:10

LitvinenkoIra force-pushed the feature/0296_possibility_to_update_video_streaming_capabilities branch 2 times, most recently from ad9c15e to 9fbb67f Compare December 15, 2020 09:37

LitvinenkoIra added 3 commits December 15, 2020 16:57

Update link to rpc_spec

27459b0

Return back the driverDistractionCapability that was missed during co…

9b7293b

…nflicts resolving

Fix functionality that was broken during conflicts resolving

5393355

LitvinenkoIra force-pushed the feature/0296_possibility_to_update_video_streaming_capabilities branch from 9fbb67f to 5393355 Compare December 15, 2020 14:58

Merge remote-tracking branch 'origin/develop' into feature/0296_possi…

4c733a1

…bility_to_update_video_streaming_capabilities

vladmu mentioned this pull request Dec 30, 2020

Feature/0296 possibility to update video streaming capabilities smartdevicelink/sdl_javascript_suite#302

Merged

4 tasks

kboskin mentioned this pull request Jan 6, 2021

[0238] Keyboard Enhancements smartdevicelink/sdl_java_suite#1587

Merged

5 tasks

ShobhitAd requested changes Jan 26, 2021

View reviewed changes

LitvinenkoIra and others added 8 commits January 27, 2021 11:44

Updates during code review

8212d63

Update link to rpc_spec

d51b003

Revert "Fix streamer activity in case of suspend (smartdevicelink#3488)"

868eccd

This reverts commit 42721d5.

Adjust old fix for all streaming transports

a6a06d4

Merge pull request #176 from LuxoftSDL/feature/0296_possibility_to_up…

89e27c5

…date_video_streaming_capabilities_testing Fix resolutions switching on streaming

Change commit to rpc_spec

ccce573

Update rpc_spec

1f278a7

Merge remote-tracking branch 'smart/develop' into feature/0296_possib…

3396fd1

…ility_to_update_video_streaming_capabilities

This was referenced Jan 28, 2021

SDL is unable to stream different audio files #3603

Closed

[SDL-0296] Possibility to update video streaming capabilities during ignition cycle smartdevicelink/sdl_hmi#391

Merged

Jack-Byrne reviewed Jan 29, 2021

View reviewed changes

ShobhitAd requested changes Jan 29, 2021

View reviewed changes

LitvinenkoIra added 2 commits February 1, 2021 13:47

Mark function as deprecated

3d6a459

Updates during code review

f0de706

ShobhitAd approved these changes Feb 4, 2021

View reviewed changes

LitvinenkoIra and others added 5 commits February 12, 2021 12:25

Merge remote-tracking branch 'origin/develop' into feature/0296_possi…

177f079

…bility_to_update_video_streaming_capabilities

Remove trailing whitespaces

31b0e74

Update link to rpc_spec

bfade9a

Merge remote-tracking branch 'smart/develop' into feature/0296_possib…

b6c98a4

…ility_to_update_video_streaming_capabilities

Update rpc_spec

73f73ce

ShobhitAd merged commit 37e40da into smartdevicelink:develop Feb 12, 2021

ShobhitAd mentioned this pull request Feb 12, 2021

[SDL 0296] Possibility to update video streaming capabilities during ignition cycle #3465

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SDL-0296] Possibility to update video streaming capabilities during ignition cycle #3468

[SDL-0296] Possibility to update video streaming capabilities during ignition cycle #3468

LitvinenkoIra commented Jul 31, 2020 •

edited

Loading

LitvinenkoIra commented Aug 18, 2020

atiwari9 left a comment •

edited

Loading

jordynmackool commented Dec 7, 2020

LitvinenkoIra commented Dec 18, 2020

dboltovskyi commented Jan 29, 2021

Jack-Byrne Jan 29, 2021

LitvinenkoIra Feb 1, 2021

Jack-Byrne commented Jan 29, 2021

dboltovskyi commented Jan 29, 2021

AKalinich-Luxoft commented Feb 1, 2021

[SDL-0296] Possibility to update video streaming capabilities during ignition cycle #3468

[SDL-0296] Possibility to update video streaming capabilities during ignition cycle #3468

Conversation

LitvinenkoIra commented Jul 31, 2020 • edited Loading

Risk

Testing Plan

Summary

Rpc_spec

CLA

LitvinenkoIra commented Aug 18, 2020

atiwari9 left a comment • edited Loading

Choose a reason for hiding this comment

TOTAL: 44 PASSED: 42 FAILED: 0 ABORTED: 0 SKIPPED: 2

jordynmackool commented Dec 7, 2020

LitvinenkoIra commented Dec 18, 2020

dboltovskyi commented Jan 29, 2021

Jack-Byrne Jan 29, 2021

Choose a reason for hiding this comment

LitvinenkoIra Feb 1, 2021

Choose a reason for hiding this comment

Jack-Byrne commented Jan 29, 2021

dboltovskyi commented Jan 29, 2021

AKalinich-Luxoft commented Feb 1, 2021

LitvinenkoIra commented Jul 31, 2020 •

edited

Loading

atiwari9 left a comment •

edited

Loading

TOTAL: 44
PASSED: 42
FAILED: 0
ABORTED: 0
SKIPPED: 2