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

make brightness control via hotkeys exponential #113

Closed
wants to merge 5 commits into from
Closed

make brightness control via hotkeys exponential #113

wants to merge 5 commits into from

Conversation

nova-r
Copy link

@nova-r nova-r commented May 3, 2023

fixes #112
As brightness is perceived exponentially, it is more intuitive to make the brightness change exponentially as well.

nova-r added 2 commits May 3, 2023 14:16
As brightness is perceived exponentially, it is more intuitive to make the brightness change exponentially as well.
the previous brightness calculation did not round to the nearest integer, this perl script does now do that
Copy link
Contributor

@denisok denisok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise doesn't work for me:

May 22 11:52:37 stdlen sway[4702]: Execution of -e aborted due to compilation errors.
May 22 11:52:37 stdlen sway[4702]: syntax error at -e line 1, near "%.0"
May 22 11:52:37 stdlen sway[4702]:         (Missing operator before f?)
May 22 11:52:37 stdlen sway[4702]: Bareword found where operator expected at -e line 1, near "0f"
May 22 11:52:37 stdlen sway[4702]:         (Missing operator before 0?)
May 22 11:52:37 stdlen sway[4702]: Number found where operator expected at -e line 1, near "%.0"

@denisok denisok added this to the 0.16 milestone May 22, 2023
@denisok denisok added the enhancement New feature or request label Jun 1, 2023
@denisok
Copy link
Contributor

denisok commented Jan 17, 2024

@nova-r could you please check suggestions?

@denisok denisok removed this from the 0.16.0 milestone Jan 18, 2024
@denisok
Copy link
Contributor

denisok commented Jan 23, 2024

@FilippoBonazziSUSE could you please check this one. When I checked it last time, it didn't work for me.

@denisok denisok self-requested a review January 23, 2024 11:10
Copy link
Collaborator

@FilippoBonazziSUSE FilippoBonazziSUSE left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commands work for me (with the necessary ' to " fix) in the sense that they succeed in changing brightness.

However, there are 3 things that I would like to see changed:

  1. The use of the Perl interpreter is not ideal, and I'd like to avoid it if at all possible. Any extra dependency must be carefully considered
  2. The brightness up and down commands are not symmetrical, i.e. increments on the way up are different from decrements on the way down. This makes brightness values not reproducible and arbitrary looking. For example, starting from 20%, going up to 100% takes N increments, but going back down N decrements arrives at 19%. This needs to be fixed before this can be merged.
  3. There is no progress on the discussion we had in make brightness control exponential #112 , and I'd like to see those points addressed too

bindsym XF86MonBrightnessDown exec brightnessctl -q set 5%- && ( echo $((`brightnessctl get` * 100 / `brightnessctl m`)) > $SWAYSOCK.wob )
bindsym XF86MonBrightnessUp exec brightnessctl -q set +5% && ( echo $((`brightnessctl get` * 100 / `brightnessctl m`)) > $SWAYSOCK.wob )
bindsym XF86MonBrightnessDown exec brightnessctl -q -e set 5%- && ( perl -e "use Math::Complex; printf \"%.0f\n\", 100 * ((`brightnessctl get` / `brightnessctl m`)**0.25)" > $SWAYSOCK.wob )
bindsym XF86MonBrightnessUp exec brightnessctl -q -e set +5% && ( perl -e 'use Math::Complex; printf \"%.0f\n\", 100 * ((`brightnessctl get` / `brightnessctl m`)**0.25)' > $SWAYSOCK.wob )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here the ' string delimiter needs to be changed to "

@FilippoBonazziSUSE
Copy link
Collaborator

@denisok This feature will take a bit more time, let's not wait for this for 0.16.0

@@ -37,8 +37,8 @@ bindsym --to-code {
# Media keys
bindsym XF86AudioMicMute exec pactl set-source-mute @DEFAULT_SOURCE@ toggle

bindsym XF86MonBrightnessDown exec brightnessctl -q set 5%- && ( echo $((`brightnessctl get` * 100 / `brightnessctl m`)) > $SWAYSOCK.wob )
bindsym XF86MonBrightnessUp exec brightnessctl -q set +5% && ( echo $((`brightnessctl get` * 100 / `brightnessctl m`)) > $SWAYSOCK.wob )
bindsym XF86MonBrightnessDown exec brightnessctl -q -e set 5%- && ( perl -e "use Math::Complex; printf \"%.0f\n\", 100 * ((`brightnessctl get` / `brightnessctl m`)**0.25)" > $SWAYSOCK.wob )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulling in yet another programming language (Perl) is not a good idea. Look at #126 as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we simply use awk for this task?

Consider the following, formatted across multiple lines for clarity:

brightness=$(brightnessctl get)
max_brightness=$(brightnessctl m)
percentage=$(awk -v br="$brightness" -v max="$max_brightness" 'BEGIN {printf "%.0f\n", 100 * ((br / max) ^ 0.25)}')
echo $percentage > $SWAYSOCK.wob

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@mcepl mcepl Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use just bc:

mitmanek~$  echo "scale=2 ; 100 * e(l("$(brightnessctl get)" / "$(brightnessctl m)") * 0.25)"|bc -l
78.00

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an additional dependency though

@nova-r nova-r closed this by deleting the head repository Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

make brightness control exponential
5 participants