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

Put unregister volume receiver in try-catch block #54

Merged
merged 2 commits into from
Aug 20, 2018
Merged

Put unregister volume receiver in try-catch block #54

merged 2 commits into from
Aug 20, 2018

Conversation

manvi-ivnam
Copy link
Contributor

Hello!
It's a fix for issue #49 .
When back button is pressed, onHostDestroy method is called.
Inside the method we can see line
mContext.unregisterReceiver(volumeBR);
and it's ok, but if we want to launch an app again and try to call setVolume method, we get an IllegalArgumentException, because the first line of setVolume method is
mContext.unregisterReceiver(volumeBR);
and it's an attempt to unregister an already unregistered instance of the receiver.
I put the line in the try-catch block to fix the issue.

@c19354837
Copy link
Owner

Thanks for your PR.

And some people have meet this problem, while I cannot reproduce it by example.

When I press back, onHostDestroy will not be call, and app just turn into background. Any idea?

@manvi-ivnam
Copy link
Contributor Author

manvi-ivnam commented Aug 17, 2018

I looked at how a similar thing is implemented here and here, and I saw that operations with receivers are implemented in onHostPause and onHostResume methods.
If you agree, I can do something like that, so far only about the volume.

@c19354837
Copy link
Owner

That would be great!

@manvi-ivnam
Copy link
Contributor Author

Done. I also removed the mContext.removeLifecycleEventListener(this); line from the onHostDestroy (need to think about where to place it correctly), because on some devices the method works and after returning to the application it's impossible to listen lifecycle events. And this is necessary, because onHostResume and onHostPause method work on any android device unlike onHostDestroy. Do you have any questions?

@c19354837
Copy link
Owner

Thanks for your work. I'll test it later.

I haven't found a device which can reproduce this problem, so I just make sure it works in my phone.

@manvi-ivnam
Copy link
Contributor Author

I created a repo with a description of the algorithm for reproducing the error. Try it, maybe you can reproduce this way.

@c19354837 c19354837 merged commit 44d46a7 into c19354837:master Aug 20, 2018
@c19354837
Copy link
Owner

Thank you for such a wonderful contribution!

I've published V1.5.3.

I reproduce it by your repo, and when I'm ready to find out some differences between your repo and my example, the example reproduces too. What a strange thing!

@manvi-ivnam
Copy link
Contributor Author

I'm glad to help!

Yes, strange things happen...:)

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