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.

Sign up for GitHub

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

Jump to bottom

fix(android, events): properly intercept and fire remote playback events #1668

Merged
merged 4 commits into from
Aug 30, 2022
Merged

fix(android, events): properly intercept and fire remote playback events #1668

merged 4 commits into from
Aug 30, 2022
+38 −61

Conversation

mpivchev
Copy link
Contributor

@mpivchev mpivchev commented Aug 23, 2022 •
edited
Loading

This PR enables notification events to be intercepted. Conveniently, this also intercepts media session events without any additional code.

Followup of doublesymmetry/KotlinAudio#40

mpivchev
mpivchev commented Aug 23, 2022
View reviewed changes
android/src/main/java/com/doublesymmetry/trackplayer/service/MusicService.kt
@@ -381,7 +385,7 @@ class MusicService : HeadlessJsTaskService() {
}

scope.launch {
event.onMediaSessionCallbackTriggered.collect {
event.onPlayerActionTriggeredExternally.collect {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This observer should now be collecting events when media session OR notification triggers player events. Make sure to put some breakpoints here and test it.

@jspizziri jspizziri linked an issue Aug 23, 2022 that may be closed by this pull request
3.1.0 does not fire events in background service on Android when tapping play/pause buttons in notification #1657
Closed
@jspizziri
Copy link
Collaborator

@mpivchev (cc @doughsay) I just tested RNTP. I tested the following events:

  • RemotePlay
  • RemotePause
  • RemoteNext
  • RemotePrevious

All of them were working as I expected 🎊 . I tested both the notification player and via connected Bluetooth headphones.

Here are some things I noticed that did not seem to be working:

  • It seems the scrubbing/seeking the progress bar in the notification player isn't working (even with Capability.SeekTo enabled).
  • I'm receiving duplicate RemoteJumpForward and RemoteJumpBackward events (hit the button on the notification player once, and I get 2 events).
    • NOTE: the jump isn't being handled by KotlinAudio, which is consistent with iOS.
  • On the notification player, the Fast Forward & Rewind buttons (double arrows right and left) seem to be triggering the RemoteJump events, rather than RemoteSeek which I would expect

@mpivchev mpivchev changed the title Intercept events fix: intercept events Aug 29, 2022
@mpivchev
Copy link
Contributor Author

Here are some things I noticed that did not seem to be working:

  • It seems the scrubbing/seeking the progress bar in the notification player isn't working (even with Capability.SeekTo enabled).

Fixed :)

  • I'm receiving duplicate RemoteJumpForward and RemoteJumpBackward events (hit the button on the notification player once, and I get 2 events).

image

Can't reproduce it on my side, I only receive one event. Maybe it got fixed along with other things.

  • On the notification player, the Fast Forward & Rewind buttons (double arrows right and left) seem to be triggering the RemoteJump events, rather than RemoteSeek which I would expect

These are actually the right events, since we used them before removing the fork as well. The only issue was the RN side did not handle them. The new commits fix that.

@mpivchev
Copy link
Contributor Author

mpivchev commented Aug 30, 2022 •
edited
Loading

PR is updated with the fixes. New KA version is 1.2.1. Already uploaded to JitPack, but the PR is here: doublesymmetry/KotlinAudio#43

@jspizziri
Copy link
Collaborator

@mpivchev thanks for the work on this. I'll retest today and let you know if I have more feeback.

@mpivchev mpivchev mentioned this pull request Aug 30, 2022
Closed
@jspizziri
Copy link
Collaborator

@mpivchev I just finished running through all the same tests. I can confirm everything is now working as I expect.

Let's merge and release 🎊

@jspizziri jspizziri changed the title fix: intercept events fix(android, events): intercept events Aug 30, 2022
@jspizziri jspizziri changed the title fix(android, events): intercept events fix(android, events): properly intercept and fire remote playback events Aug 30, 2022
@dcvz dcvz merged commit 9ed308c into main Aug 30, 2022
@dcvz dcvz deleted the intercept-notification-events branch August 30, 2022 18:27
@jspizziri jspizziri mentioned this pull request Sep 6, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcvz dcvz dcvz approved these changes

@jspizziri jspizziri jspizziri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3.1.0 does not fire events in background service on Android when tapping play/pause buttons in notification
3 participants
@mpivchev @jspizziri @dcvz