Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed video not resuming when switching between navigation apps #1947

Conversation

NicoleYarroch
Copy link
Contributor

@NicoleYarroch NicoleYarroch commented Mar 19, 2021

Fixes #1944

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Unit Tests

  • Tested switching between two navigation apps on SYNC 3.0. Testing switching when phone app is in background and in foreground.
  • Tested changing the screen size midstream on sdl_core while the phone app is in the foreground and in the background.

Core Tests

[List of tests performed against Core and behaviors verified]

Core version / branch / commit hash / module tested against:

  • SYNC 3.0
  • sdl_core: 7.1.0-RC

HMI name / version / branch / commit hash / module tested against:

  • SYNC 3.0
  • sdl_hmi: 5.5.0

Summary

If a navigation app is streaming video and it receives a notification that it's hmiLevel has changed to LIMITED (i.e. another navigation app has become active), an end video service message is sent to the module and video stops streaming. When the hmiLevel changes back to FULL, then a start video service request is sent to core and video streaming resumes once the module has ACKd the request.

When SDL-0296 (Feature/ possibility to update video streaming capabilities) was implemented, instead of sending a GetSystemCapability request for VIDEO_STREAMING every time the start service message is sent, the GetSystemCapability request is now only sent once and all subsequent requests use the cached value. This slight timing changed broke video streaming resumption when switching between navigation apps on SYNC 3.0. Video streams to the module but the screen is black. Adding a slight delay between getting the OnHMIStatus notification and starting the video stream fixed the issue.

Change Log

Bug Fixes
  • Fixed video not resuming when switching between navigation apps on legacy head units.
  • The state machine now always transitions to the SDLVideoStreamManagerStateReady when the video start service ACKs. Previously it could transition to the SDLVideoStreamManagerStateSuspended state, which was unnecessary as the logic of checking for the suspended state is already handled in the SDLVideoStreamManagerStateReady state.

Tasks Remaining:

  • Test against sdl_core
  • Test against SYNC 3.0

CLA

Signed-off-by: NicoleYarroch <nicole@livio.io>
Signed-off-by: NicoleYarroch <nicole@livio.io>
@NicoleYarroch NicoleYarroch self-assigned this Mar 19, 2021
@NicoleYarroch NicoleYarroch added bug A defect in the library manager-streaming-video Relating to the manager layer - video streaming labels Mar 19, 2021
Signed-off-by: NicoleYarroch <nicole@livio.io>
Signed-off-by: NicoleYarroch <nicole@livio.io>
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #1947 (b68e69d) into develop (1117c73) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head b68e69d differs from pull request most recent head 520dcf1. Consider uploading reports for the commit 520dcf1 to get more accurate results

@@             Coverage Diff             @@
##           develop    #1947      +/-   ##
===========================================
+ Coverage    83.63%   83.64%   +0.01%     
===========================================
  Files          441      441              
  Lines        22531    22533       +2     
===========================================
+ Hits         18844    18848       +4     
+ Misses        3687     3685       -2     

NicoleYarroch and others added 2 commits March 23, 2021 13:07
Co-authored-by: Joel Fischer <joeljfischer@gmail.com>
Signed-off-by: NicoleYarroch <nicole@livio.io>
@joeljfischer joeljfischer merged commit 46fda98 into develop Mar 23, 2021
@joeljfischer joeljfischer deleted the bugfix/issue_1944_video_resumption_broken_switching_nav_apps branch March 23, 2021 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library manager-streaming-video Relating to the manager layer - video streaming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants