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

Range lock mode #3

Open
ghost opened this issue Nov 11, 2022 · 9 comments
Open

Range lock mode #3

ghost opened this issue Nov 11, 2022 · 9 comments

Comments

@ghost
Copy link

ghost commented Nov 11, 2022

Hi dev i suggest to add volume range lock mode this will lock volume only able to changed between X number
For example i lock volume media to only changeable between 25% and 50%

Thanks for consideration 🙏😄

@nomelif
Copy link

nomelif commented Apr 5, 2023

Hi. I needed this feature or at least the ability to set a cap on maximum volume. Hacking the behaviour of the app to cap volume without changing the UI was surprisingly simple.

My android dev experience is very limited (haven't touched Kotlin before, for instance), but I'd probably be motivated enough by scratching my own itch here to make a PR. Would you (@jonathanklee) be willing to accept that, if the code quality was good enough?

The simplest UI change I could think of would be to have a RangeSlider (which seems to be a built-in widget) to set the volume range. Would this make pinning a specific volume hard though?

@jonathanklee
Copy link
Owner

@nomelif Hey. Sure you can make a PR (:

@nomelif
Copy link

nomelif commented Apr 7, 2023

I've made a quick sketch of potential changes here: https://github.com/nomelif/VolumeLockr/tree/range-mode

I don't think it's PR-quality yet though, because:

  • It saves broken preferences (probably because I changed the volume state to handle ranges). Has to be investigated. In general, I went the route of not modifying the Volume class and instead having the mVolumeLock object in VolumeService.kt be HashMap<Int, Pair<Int, Int>>(). I'm not sure this is the most elegant way to go about it.
  • I paid no attention to rounding, and as the RangeSlider is a Float [0, 1] and the volumes seem to be integers in version-dependent ranges, that's kinda ugly.
  • The original app has the sliders track volume if the lock isn't engaged. I did the same thing, but also made it so changing volume while the slider is in range mode doesn't affect the slider. It's kinda hacky and could probably be improved.
  • Oh and I think I probably messed up the theming on the sliders, though that's a very small thing.

Anyway, @jonathanklee, can you have a look at whether the general approach taken makes sense to you? (Also, this is the first time I ever touch Kotlin so if it's super unidiomatic or I fell into some obvious pitfall, please tell me.)

@jonathanklee
Copy link
Owner

@nomelif Hey. I had a look at it. It looks good. Have you made exploratory tests with it ? You can open a PR if you want.

@nomelif
Copy link

nomelif commented Apr 12, 2023

I have in the sense that I have built it and run it on a device. It seems to mostly work, but I feel that it's still somwhat janky (I feel like I have gotten it into an inconsistent state but don't really know how to reproduce it yet), particularly with the app being closed and re-opened. There's one somewhat serious bug that has to do with saving the json-format config (leads to a crash). I believe that the type annotations on the loader are wrong (=I didn't update them and because there is a re-load from the json, Kotlin doesn't type error at compile time), but I haven't had a serious look at it yet.

I also kinda feel that having the two ends of the range completely collapsed is maybe a bit misleading, as it looks exactly like a non-rangey slider. (So it's surprising when it doesn't behave as one when touched) I was wondering whether there should be some math to have them stay as a narrow range when tracking the volume being changed from the OS.

Oh and there's an unpleasant behaviour (but I don't know if it's really a bug) with the app as it stands when changing the volume by dragging the OS popup (as opposed to physical volume keys): the locking only kicks in after the fact, so you can max out the volume for a couple tenths of a second. I don't know if something can be meaningfully done about that.

@jonathanklee
Copy link
Owner

@nomelif Alright, I'll make some tests and try to debug when I have some time.

@owenfromcanada
Copy link
Contributor

Hey folks, I went ahead and implemented a max limit (I didn't notice this discussion until after I submitted a pull request). Not sure if anyone is still looking at this, but it might at least be a half-way point.

@BurnyBoi
Copy link

BurnyBoi commented Apr 22, 2024

@owenfromcanada Thank you for submitting that change! I know I said it was working in your PR but I did find one small bug: The setting you made for allowing lower than the limit doesn't seem to work if I stop the service and restart the app. I then have to toggle the new setting off than on for it to work again. I am looking into adding another change so I can see if I can fix this as well but figured I'd mention it.

@owenfromcanada
Copy link
Contributor

owenfromcanada commented Apr 22, 2024

My changes read the new preference during the onStartCommand callback, which apparently isn't run in that condition. I fixed it by also reading it when loadPreferences is called.

I submitted an additional PR.

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

No branches or pull requests

4 participants