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

swayosd --brightness lower doesn't lower brightness #12

Closed
russelltg opened this issue May 3, 2023 · 13 comments · Fixed by #60 · May be fixed by #25
Closed

swayosd --brightness lower doesn't lower brightness #12

russelltg opened this issue May 3, 2023 · 13 comments · Fixed by #60 · May be fixed by #25
Labels
bug Something isn't working

Comments

@russelltg
Copy link

russelltg commented May 3, 2023

If I am reading the documentation correctly, it seems like swayosd is supposed to be able to change the brightness of my display. However, when I run swayosd --brightness lower|raise doesn't make any effect. brillo -A 10 or brillo -U 10 both work fine, so it doesn't seem to be a permissions issue.

Additionally, no OSD is displayed whatsoever, but other OSD's displayed by swayosd work great (volume, caps lock)

Version: swayosd-git from AUR r18.8ef76c1-1
Compositor: Hyprland (wlroots)
OS: Fully updated Arch Linux as of May 2 11:56 PM MST

swayosd strace

brillo strace

@ErikReider
Copy link
Owner

Have you added the udev rules that are in the readme?

@ErikReider ErikReider added the bug Something isn't working label May 3, 2023
@russelltg
Copy link
Author

Yes, and they seem to have worked:

$ ls -lah /sys/class/backlight/intel_backlight/brightness
.rw-rw-r-- root video 4.0 KB Tue May  9 19:34:29 2023 /sys/class/backlight/intel_backlight/brightness
$ groups
wireshark seat libvirt docker video kvm sudo russell

@hodgesds
Copy link

hodgesds commented May 19, 2023

I've played around with the brightness code a little bit and this patch seems to work (minus some bounds checks):

diff --git a/src/utils.rs b/src/utils.rs
index c8e2198..7dbd7c0 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -8,7 +8,7 @@ use std::{
 	sync::Mutex,
 };
 
-use blight::{change_bl, err::BlibError, Change, Device, Direction};
+use blight::{err::BlibError, Delay, Device, Direction};
 use pulse::volume::Volume;
 use pulsectl::controllers::{types::DeviceInfo, DeviceControl, SinkController, SourceController};
 
@@ -265,25 +265,20 @@ pub fn change_brightness(
 		.unwrap_or(String::new())
 		.parse::<u8>()
 		.unwrap_or(BRIGHTNESS_CHANGE_DELTA) as u16;
-	let direction = match change_type {
-		BrightnessChangeType::Raise => Direction::Inc,
-		BrightnessChangeType::Lower => {
-			let device = Device::new(None)?;
-			let change = device.calculate_change(brightness_delta, Direction::Dec) as f64;
-			let max = device.max() as f64;
-			// Limits the lowest brightness to 5%
-			if change / max < (brightness_delta as f64) * 0.01 {
-				return Ok(Some(device));
-			}
-			Direction::Dec
-		}
+	let mut device = Device::new(None)?;
+	let change = match change_type {
+		BrightnessChangeType::Raise => device.calculate_change(brightness_delta, Direction::Inc),
+		BrightnessChangeType::Lower => device.calculate_change(brightness_delta, Direction::Dec),
 	};
-	match change_bl(brightness_delta, Change::Regular, direction, None) {
+	match device.sweep_write(change, Delay::default()) {
 		Err(e) => {
 			eprintln!("Brightness Error: {}", e);
 			Err(e)
 		}
-		_ => Ok(Some(Device::new(None)?)),
+		_ => {
+			device.reload();
+			Ok(Some(device))
+		}
 	}
 }
 

A few things that stood out that are kind of weird about the existing code is the recreation of the Device as it is only used to be passed to the window.changed_brightness function where the Device is used to get the current and max value. It might be a better abstraction to just pass in the values directly rather than relying on the implementation of the device to return those values. The other change is to use the sweep_write method for smoother transitions.

Anyways, thanks for creating this project it's really nice!

@ErikReider
Copy link
Owner

Sorry for the massive delay, this got lost in my emails...

Can you open a PR with these changes? Makes it easier for me to keep track of stuff :)

@augustebaum
Copy link

Having the same issue on NixOS. Volume control works great, but exec swayosd --brightness raise shows the brightness bar at maximum, and exec swayosd --brightness lower doesn't show anything and doesn't lower the brightness.
brightnessctl does work on this machine btw.

@aacebedo
Copy link
Contributor

aacebedo commented Aug 1, 2023

Same issue here on nixos on an intel based machine. Changing it manually with an echo command or brightnessctl works correctly though.

Would it be possible to add some debug logs in the program?

@ErikReider
Copy link
Owner

I'll try to get this issue fixed within a couple of days. If I forget, please don't be afraid to ping me :)

@ErikReider
Copy link
Owner

@augustebaum @aacebedo have you installed the udev rules properly? If so, please provide the output of: cat /sys/class/backlight/*/brightness and cat /sys/class/backlight/*/max_brightness

@aacebedo
Copy link
Contributor

The udev rules were correctly set. I installed the latest version and it seems to work. Will check if it continues to work.

@nyabinary
Copy link

nyabinary commented Sep 18, 2023

@ErikReider Hmm did you forget? :3

@leon-erd
Copy link

For anyone having this problem on nixos adding the user to video group and manually setting the group and permissions of /sys/class/backlight/*/brightness fixed it for me.
I manually ran sudo chgrp video /sys/class/backlight/intel_backlight/brightness and sudo chmod g+w /sys/class/backlight/intel_backlight/brightness and put the following in the configuration.nix

  users.users.${username} = {
    isNormalUser = true;
    description = name;
    extraGroups = [ "networkmanager" "wheel" "video"];
  };

@Ziqi-Yang
Copy link

For anyone having this problem on nixos adding the user to video group and manually setting the group and permissions of /sys/class/backlight/*/brightness fixed it for me. I manually ran sudo chgrp video /sys/class/backlight/intel_backlight/brightness and sudo chmod g+w /sys/class/backlight/intel_backlight/brightness and put the following in the configuration.nix

  users.users.${username} = {
    isNormalUser = true;
    description = name;
    extraGroups = [ "networkmanager" "wheel" "video"];
  };

I find that NixOS will change file permission everytime we run nix rebuild, so I add these configuration to automatically change the file permission:

  system.activationScripts = {
    # swayosd cannot set brightness issue on NixOS
    # see https://github.com/ErikReider/SwayOSD/issues/12#issuecomment-1950581102
    fix-brightness-file-permission.text = ''
chgrp video /sys/class/backlight/intel_backlight/brightness
chmod g+w /sys/class/backlight/intel_backlight/brightness
'';
  };

@leon-erd
Copy link

leon-erd commented Apr 8, 2024

I discovered setting the udev rules provided by swayosd with adding this to my configuration.nix

services.udev.packages = [ pkgs.swayosd ];

I think this fixes the issue of reverting permissions after rebuild :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
8 participants