Kore
Kore copied to clipboard
Control volume with hardware buttons when Kore is not focused (#157)
Since MediaSession was implemented, I thought it'd be a good point to add volume control via the VolumeProviderCompat. This way, playback volume can be adjusted even when Kore is not in focus and/or the screen is locked (but turned on). To my mind, it resolves Issue #157 as far as possible for now.
Thanks, and sorry for the delay in reviewing this.
Looks good, but a couple of comments:
- There's a setting that defines whether the hardware volume keys should control Kodi's volume, and that should be respected. Take a look at
Settings.java, look forKEY_PREF_USE_HARDWARE_VOLUME_KEYSand check how it's used inVolumeControllerDialogFragmentListener. I'll point out in the code where to check if the setting is active. - For consistency with the current code, please use spaces, with 4 spaces per level.
Hi, thanks for the feedback! I'll add the changes in the next days :)
Hi @SyncedSynapse I guess it's ready now :D I reindented the file and now support the user preference concerning hardware volume key usage
Ok, thanks.
Just one thing:
Note that in the constructor for RemoteVolumeProviderCompat you're calling setCurrentVolume, and you're constructing it, before checking if the hardware volume keys are enabled. I don't think it's serious, but can you please consider this. I'll point out in the code.
By the way, i'll probably be unavailable for the rest of the week, so i might not be able to merge this before next week.
Thanks for pointing that out. To my mind, it was not relevant to check the preference there since setCurrentVolume solely affects the RemoteVolumeProviderCompat. I thought it was necessary to get the currently set volume on Kodi and set it to the Compat so the volume used as reference for display (in the Android volume slider) is in sync. After doing some explorative testing, it seems that everything works just fine without initial synchronization, so I removed the call and method.
I didn't mind the syncing on the constructor, i just thought that you might only create remoteVolumePC after checking if the hardware volume keys were enabled, but this is ok also.
There's a slight bug: when you call any of the HostConnectionObserver.register* you need to call unregister somewhere (in this case in onDestroy), or it will keep a reference to the object, preventing GC, as hostConnectionObserver is a singleton.
I'll merge this as is and fix that later.
Thanks