-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Comments
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? |
@nomelif Hey. Sure you can make a PR (: |
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:
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.) |
@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. |
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. |
@nomelif Alright, I'll make some tests and try to debug when I have some time. |
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. |
@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. |
My changes read the new preference during the I submitted an additional PR. |
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 🙏😄
The text was updated successfully, but these errors were encountered: