search

NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.06k stars 13.39k forks source link

`programs.ddcutil` should set up a working `ddcutil` #292049

Open l0b0 opened 5 months ago

l0b0 commented 5 months ago

Issue description

There seems to be manual steps necessary to make ddcutil work. According to @ElvishJerricco this should be solved by having a programs.ddcutil option which does the necessary steps rather than changing system settings when installing pkgs.ddcutil.

I'm uncertain whether the package should set services.udev.packages = [pkgs.ddcutil];, the literal string KERNEL=="i2c-[0-9]*", GROUP="i2c", MODE="0660" suggested by this post, or something else. I'm trying things out here.

Steps to reproduce

  1. Install pkgs.ddcutil
  2. Run ddcutil detect

Technical details

nixos-discourse commented 5 months ago

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-enable-ddc-brightness-control-i2c-permissions/20800/22

rnhmjoj commented 5 months ago

You're supposed to use hardware.i2c.enable. This gives local users access to all i2c devices, plus any other user in the i2c group.

l0b0 commented 5 months ago

You're supposed to use hardware.i2c.enable. This gives local users access to all i2c devices, plus any other user in the i2c group.

OK, then that's what programs.ddcutil.enable should do. I was also told I had to add pkgs.ddcutil to services.udev.packages. The point is that neither of these are obvious, so they should be behind a simple switch.

With all these settings I'm still running into an issue, but that might be unrelated.

rnhmjoj commented 5 months ago

OK, then that's what programs.ddcutil.enable should do.

It doesn't justify an ad-hoc option, IMHO: ddcutil is not the only program that wants i2c access and it doesn't require any other particular setup.

The point is that neither of these are obvious, so they should be behind a simple switch.

I don't like the idea of options just for the sake of explaining other options: this is what the manual is for.

rockowitz commented 5 months ago

Note that as of ddcutil 2.0, installation generally takes care of ensuring that driver i2c-dev is loaded and that the logged on uses has read/write access to /dev/i2c devices associated with displays. See the release notes for version 2.0.0.

l0b0 commented 5 months ago

OK, then that's what programs.ddcutil.enable should do.

It doesn't justify an ad-hoc option, IMHO: ddcutil is not the only program that wants i2c access and it doesn't require any other particular setup.

Any program which requires i2c access should have such a switch, since it's not done as part of package install. But that's another issue.

The point is that neither of these are obvious, so they should be behind a simple switch.

I don't like the idea of options just for the sake of explaining other options: this is what the manual is for.

It's not for explaining anything, it's a convenience since discovering the need for i2c is non-obvious.

l0b0 commented 5 months ago

Note that as of ddcutil 2.0, installation generally takes care of ensuring that driver i2c-dev is loaded and that the logged on uses has read/write access to /dev/i2c devices associated with displays. See the release notes for version 2.0.0.

Is that actually going to be done by the nixpkgs install, though? If I understand correctly, @ElvishJerricco and @rnhmjoj think that installing this package should not load i2c. I agree, with an additional caveat: enabling programs.foo should install programs.foo.package and apply all the necessary side effects to get foo working standalone, without any more manual work. Installing pkgs.foo, on the other hand, should not have any side effects other than installing the package files in the Nix store. That way, foo's files can be used by other packages which don't depend on the extra side effects of programs.foo.

rnhmjoj commented 5 months ago

Any program which requires i2c access should have such a switch, since it's not done as part of package install. But that's another issue.

By the same logic any game should have a programs.game.enable that does hardware.opengl.enable = true.

It's not for explaining anything, it's a convenience since discovering the need for i2c is non-obvious.

I think it's on the user to understand that this program requires the i2c module and devices permissions, it's also clearly stated in the project homepage. What may be non-obvious is that setting up i2c means hardware.i2c.enable on NixOS, because it's not mentioned anywhere in the manual.

rnhmjoj commented 5 months ago

Note that as of ddcutil 2.0, installation generally takes care of ensuring that driver i2c-dev is loaded and that the logged on uses has read/write access to /dev/i2c devices associated with displays. See

I don't think this works on NixOS.