battery: make scroll change brightness #209
Conversation
This reverts commit d53f915.
I think your original idea of using |
|
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 |
|
Thanks for the feedback. I reverted to the original approach with the latest commit, added Let me know if you have any other suggestions. |
| </option> | ||
| <option name="battery_scrolldown_command" type="string"> | ||
| <_short>Battery Scroll Down Command</_short> | ||
| <default>brightnessctl -q set 10%-</default> |
There was a problem hiding this comment.
Why not do it on every discrete event at 3% by default? It seems like you're losing some granularity and possibly introducing buggy behavior by doing nothing on some scroll events.
There was a problem hiding this comment.
Sorry, I was just doing what I thought ammen99 originally suggested. It just made sense to me because running so many commands at once makes the brightness update laggy on my system.
There was a problem hiding this comment.
Sounds like another thing which ought to be an option (granularity)
| @@ -174,6 +174,23 @@ void WayfireBatteryInfo::update_details() | |||
| } | |||
| } | |||
|
|
|||
| void WayfireBatteryInfo::on_battery_scroll(GdkEventScroll *event) | |||
| { | |||
| if (scroll_counter <= 0) { | |||
There was a problem hiding this comment.
How does GDK behave when we have smooth scroll, do we get separate events for discrete and smooth scrolling, or are they bunched here together? Can delta_* be a floating point number?
In general I am kinda hesitant to have a hard threshold like this (i.e update every 3 events). Wouldn't it be better to update on every X units of scroll, or even better, every milliseconds, where N is a configurable period of time?
There was a problem hiding this comment.
To complicate things further, there is also GdkEventScroll.GdkScrollDirection, which tells when the delta_x/y values are valid to use or which direction to scroll.
battery_scroll_sensitivityis a new option that specifies the amount (in percentage points) that each scroll event will change the brightness. I found1.8to 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 thevideogroup. 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 #207