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

fix: answer call from notification in foreground service [WPB-9648] #3797

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

saleniuk
Copy link
Contributor

@saleniuk saleniuk commented Jan 13, 2025

TaskWPB-9648 [Android] Ensure Compliance with Audio Focus Request Restrictions on Android 15


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

We need to ensure compliance with audio focus request restrictions on Android 15:
https://developer.android.com/about/versions/15/behavior-changes-15#audio-focus

Basically, when targeting Android 15, app must be in the foreground or have foreground service working to successfully request audio focus and it’s not the case right now.

Solutions

AVS requests audio focus when starting/answering a call and when receiving incoming call - this one is probably just for the ringing sound and we provide the ringing sound via the notification and do not play it ourselves.
For starting or answering a call, there's one case when it may happen when the app is in the background - when answering from the incoming call notification. To make it work, answering logic is moved to the call service, so when answering from the notification, first the CallService is started with proper action so that the call is answered and audio focus requested after the foreground service is started.

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

Right now it's not yet possible to test it manually as it requires the app to target Android 15 (API 35), but after we update target then it shouldn't crash when doing these steps:
-open the app
-put into background
-make a call to that user from another device
-answer from the notification


PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@echoes-hq echoes-hq bot added the echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt. label Jan 13, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 56.75676% with 16 lines in your changes missing coverage. Please review.

Project coverage is 45.79%. Comparing base (f45ed9e) to head (a00a2b3).
Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
...in/kotlin/com/wire/android/services/CallService.kt 42.85% 12 Missing ⚠️
...n/broadcastreceivers/IncomingCallActionReceiver.kt 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3797      +/-   ##
===========================================
- Coverage    45.80%   45.79%   -0.01%     
===========================================
  Files          483      485       +2     
  Lines        16513    16575      +62     
  Branches      2783     2784       +1     
===========================================
+ Hits          7563     7590      +27     
- Misses        8170     8205      +35     
  Partials       780      780              
Files with missing lines Coverage Δ
...otlin/com/wire/android/services/ServicesManager.kt 74.41% <100.00%> (+2.62%) ⬆️
...n/broadcastreceivers/IncomingCallActionReceiver.kt 3.70% <0.00%> (-0.30%) ⬇️
...in/kotlin/com/wire/android/services/CallService.kt 12.50% <42.85%> (+9.51%) ⬆️

... and 11 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f45ed9e...a00a2b3. Read the comment docs.

Copy link
Contributor

Built wire-android-staging-compat-pr-3797.apk is available for download

Copy link
Contributor

Built wire-android-dev-debug-pr-3797.apk is available for download

@saleniuk saleniuk requested review from a team, yamilmedina, alexandreferris, MohamadJaara, ohassine and emmaoke-w and removed request for a team January 14, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants