wf-shell icon indicating copy to clipboard operation
wf-shell copied to clipboard

battery: make scroll change brightness

Open carterprince opened this issue 1 year ago • 3 comments

battery_scroll_sensitivity is a new option that specifies the amount (in percentage points) that each scroll event will change the brightness. I found 1.8 to be a good default as a compromise between smooth (touchpad) scrolling and discrete (mouse-wheel) scrolling.

I wrote a method change_brightness() which reads and writes to /sys/class/backlight/ to change the brightness. This requires that the running user be in the video group. Personally I'm fine with this but I want to hear what everyone else thinks.

If I break some conventions here I apologize; I'm not very familiar with how you're supposed to do stuff in C++. Please let me know.

Also, I haven't tested this on any machine other than my laptop. So if it doesn't work for you also let me know.

Resolves https://github.com/WayfireWM/wf-shell/issues/207

carterprince avatar Jan 16 '24 02:01 carterprince

I wrote a method change_brightness() which reads and writes to /sys/class/backlight/ to change the brightness. This requires that the running user be in the video group. Personally I'm fine with this but I want to hear what everyone else thinks.

I think your original idea of using brightnessctl made more sense. I also think there should be an option to make the command configurable as @ammen99 suggested in #207. Maybe you can have two options, one defining the command for brighter and the other dimmer. Then you could possibly bind scroll 1-1 and eliminate the sensitivity option. Either way, you could call the options directly, as commands for the scroll events.

soreau avatar Jan 16 '24 06:01 soreau

I agree with @soreau, on some of my previous laptops brightness was rather hard to change and one had to use different files in sys. Also you get the problem of min/max brightness etc. It would be better to let the user provide a command themselves. If you care about smooth scrolling too much, you could make the command a format string. I.e option value is brightnessctl %.2f and then the widget replaces the %.2f with the scroll delta with sprintf. Though I suspect discrete steps should be fine enough.

ammen99 avatar Jan 16 '24 07:01 ammen99

Thanks for the feedback. I reverted to the original approach with the latest commit, added battery_scrollup_command and battery_scrolldown_command options, and took @ammen99's original suggestion of running commands every 3 steps of discrete scroll.

Let me know if you have any other suggestions.

carterprince avatar Jan 16 '24 19:01 carterprince