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

Bugfix/issue 1802 Unregister apps when audio output becomes unavailable #1827

Merged
merged 3 commits into from
Sep 7, 2022

Conversation

joeygrover
Copy link
Member

Fixes #1802

This PR is ready for review.

Risk

This PR makes no API changes.
Note: method added to library scoped class and therefore no API was added.

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).
  • I have tested Android, Java SE, and Java EE

Unit Tests

N/A These tests need to be ran manually.

Core IVI Tests

  1. Set test app to Media using multiplexing (new HelloSdl build flavor: requiresAudioSupport)
  2. Turn off bluetooth adapter on phone
  3. Connect AOA through multiplexing, observe test app did not register
  4. Turn on bluetooth and allow phone to connect to IVI
  5. If app does not register automatically, open the manually and observe it did register
  6. Turn off bluetooth adapter, observe app unregisters and is removed on the IVI apps screen

IVI Info: Sync Gen3

Summary

This PR addresses the issue of apps not unregistering when they require audio support, it is initially given, and then is removed. Previously it would stop the session but not unregister which on some IVIs means the app icon will still be visible to the user. These changes are the simplest in terms of scope and lowest of risk. The alternatives were either a little more messy or created a much larger risk across the entire Java Suite.

Alternatives:
  1. Create a new constructor that takes in an audio streaming status callback
  2. Add a new method in the ISdlSessionListener for a required shutdown to the LCM or only that an UnregisterAppInterface message was sent. (This would affect Java SE/EE libs)

Changelog

Bug Fixes
  • Apps will now unregister from the IVI when the previously available audio output is no longer available.

CLA

Add to Andoroid’s SdlSession since it is only an Android feature
Closes LCM if audio outpput is required and no longer available. Properly clears apps off IVI.
@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #1827 (903024b) into develop (4f8e483) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 903024b differs from pull request most recent head 832a182. Consider uploading reports for the commit 832a182 to get more accurate results

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1827      +/-   ##
=============================================
- Coverage      54.04%   54.03%   -0.01%     
  Complexity      5532     5532              
=============================================
  Files            562      562              
  Lines          25821    25821              
  Branches        3398     3398              
=============================================
- Hits           13954    13953       -1     
- Misses         10596    10597       +1     
  Partials        1271     1271              
Impacted Files Coverage Δ
...rtdevicelink/streaming/video/SdlRemoteDisplay.java 50.00% <0.00%> (-2.44%) ⬇️
...smartdevicelink/encoder/VirtualDisplayEncoder.java 46.02% <0.00%> (+0.33%) ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants