search

pop-os / system76-dkms

System76 DKMS driver
GNU General Public License v2.0
37 stars 20 forks source link

Better Keyboard Color Schemes For Oryx Pro (2018 version) #8

Closed viridian1138 closed 5 years ago

viridian1138 commented 6 years ago

Better Keyboard Color Schemes For Oryx Pro (2018 hardware version). Tested on Oryx Pro.

zaufi commented 6 years ago

It'll be nice to have a module parameter(s) to set initial color(s) after (re)boot %)

viridian1138 commented 6 years ago

It'll be nice to have a module parameter(s) to set initial color(s) after (re)boot %)

That doesn't already exist?

   echo a8383b > /sys/class/leds/system76::kbd_backlight/color_left

   etc.
ebobby commented 6 years ago

I think he meant to pick one of your particular schemes as opposed to have to look for it every time you boot.

zaufi commented 6 years ago

I meant to have parameters for the module e.g. /etc/modprobe.d/blah.conf:

options system76 color_left=xxxxxx color_center=xxxxxx color_right=xxxxxx

(or maybe just one color)... so, on start or reboot (module load), I can have smth different than just (initial) white...

viridian1138 commented 6 years ago

I changed the reboot logic. I think the simplest UI from the ordinary user's perspective is one where the user's color choice persists across a reboot, which is what I decided to implement.

jackpot51 commented 5 years ago

I think it is really cool what you have done here. That being said, it is unlikely we would merge this.

I strongly disagree with having color restore logic in the kernel driver. It could instead be done in a userspace daemon, by using the /sys/class/leds folder. This would reduce the amount of kernel code which is usually a good thing. Currently, gnome-settings-daemon restores the backlight intensity, but not color. Perhaps you would want to patch this in there, or have a simple daemon that only restores color.

I also disagree with changing the default colors. The ones as they are were picked for simplicity, and users who want more can also override the defaults either by modifying the kernel driver as you have done, or by scripting changes to the color by setting the hex values in the exposed /sys/class/leds folder.

If someone wants to pull your branch, build it with debuild -b -uc -us, and install it, I certainly support that. However, I do not believe that these are good defaults.