Drive keyboard backlight from the ambient light sensor - #303
Conversation
scottjones
left a comment
There was a problem hiding this comment.
Verified this on hardware — an M2 Max MacBook Pro (apple,j414c) running Asahi, which has both the ambient sensor and a backlit keyboard.
It works, and the design is careful. The curve matched the documentation exactly when I exercised it through --map-lux (0 lux → 100%, 23 → 91%, 100 → 46%, 180+ → 0%), and --available correctly returns 0 here. Splitting the mapping out as a pure, testable subcommand that doesn't touch the LED is a nice touch.
Running the real loop and covering the sensor with my hand:
23-24 lux → 232 (91%, steady)
covered: 11 lux → 232
1 lux → 255 (100%)
uncovered: 24 lux → 232 (back down)
covered: 3 lux → 249
1 lux → 249 ← deadband holds; 249 vs 255 is inside 4%
It tracked the light within a sample or two, went full brightness in the dark, came back down when uncovered, and the deadband correctly declined to rewrite the LED over a 6-unit difference. The override handling is the best part: remembering last_set and treating any divergence as a human touch means this won't fight Shift+Brightness, and standing down while the session is locked or the lid is shut is exactly right.
Both test files pass, omarchy commands --check is clean at 454 commands, and it merges into quattro without conflicts.
One change before this lands: POLL_SECONDS=1 is too aggressive for a service that runs forever on a laptop.
Each tick spawns brightnessctl, loginctl, and omarchy-hw-laptop-closed — and that last one is itself a busctl call now. Measured on this machine:
full tick ≈ 19ms → ~68 seconds of CPU per hour, continuously, on battery
Ambient light doesn't change on a one-second timescale, and the trace above bears that out — I was sampling at 2s intervals and still caught every transition within a sample. At 3s the cost drops to ~23s/hour and I don't believe it would feel any different in use; 5s would likely still be fine and is worth considering, since the deadband already keeps small fluctuations from causing writes.
Happy to re-test on this hardware at whichever value you pick.
On laptops with an IIO illuminance sensor and a kbd_backlight LED, light the keys in the dark and dim them as the room brightens. Manual brightness keys pause auto until ambient light moves enough that the old choice no longer fits.
A 1s loop spends ~68s of CPU per hour on brightnessctl/loginctl/busctl. Ambient light does not move that fast; 5s still tracks room changes.
814aa9d to
33b1d7b
Compare
scottjones
left a comment
There was a problem hiding this comment.
Re-checked the branch: both test files pass, omarchy commands --check is clean at 454, and it still merges into quattro without conflicts. The design notes from my first review stand — the override handling is the best part of this.
bug: POLL_SECONDS=1 is still on the branch. We settled on 5 in the thread, but the head was never updated. That is the only blocker.
suggestion: the unit runs /usr/bin/omarchy-brightness-keyboard-auto, but /usr/bin/omarchy-* here comes from the omarchy package (pacman -Qo /usr/bin/omarchy-brightness-keyboard → omarchy 4.0.1-2), not the dev tree. Until that package is rebuilt with the new command, the migration enables a unit whose ExecCondition exits 203 and is silently skipped. Copying the unit into ~/.config/systemd/user does not help with the binary path. Worth a note in the PR so nobody thinks the migration is broken.
suggestion: the manual says "Lock and lid-close keep the keys off", but the code only stands down — tick returns early and nothing writes the LED. Please reword to match.
nit: on the --available path, [[ ... ]]; exit $? under set -e exits with 1 before reaching exit $?. It works, but by accident.
For the record, #330 (exact keyboard backlight CLI) touches different files, and its writes are exactly the manual override this service is designed to yield to. No conflict.
scottjones
left a comment
There was a problem hiding this comment.
Correction first: my last review's blocker was wrong. I said POLL_SECONDS=1 was still on the branch — it's been 5 since 33b1d7b ("Poll keyboard ALS every 5 seconds"), pushed about 17 hours before I wrote that. I had a stale ref shadowing the real head in my clone and reviewed the old snapshot. Sorry for the runaround; you'd already done exactly what we agreed.
Re-verified against the real head on the M2 Max (apple,j414c): POLL_SECONDS=5, both test files pass, omarchy commands --check clean at 454, still merges into quattro without conflicts.
One thing I'd still change before merge, and it's more than the note I made last time. Both the migration and enable-user-units.sh copy the unit into ~/.config/systemd/user/. That path is one quattro deliberately retired — bin/omarchy-upgrade-to-quattro:1717:
is_retired_config_file() {
local rel="$1" retired
[[ $rel == systemd/user/* ]] && return 0Every ~/.config/systemd/user/* is unconditionally retired so the package-owned /usr/lib/systemd/user/ copy takes effect — the header comment spells it out (systemd/user/bt-agent.service -> /usr/lib/systemd/user/bt-agent.service). The other seven units in that same enable --now list need no copy for exactly this reason. As written this leaves a user-local override that nothing ever refreshes.
Since the unit is inert until the omarchy package ships the binary anyway (my earlier ExecCondition 203 note), I'd drop both cp blocks and let the unit land via the settings PKGBUILD. The migration then just enables it.
suggestion: find_als() takes the first device with a readable in_illuminance_input, ignoring name. On this machine there are two AOP IIO devices sitting next to each other:
iio:device0 name=aop-sensors-las in_angl_raw <- lid angle
iio:device1 name=aop-sensors-als in_illuminance_input <- what you want
It picks device1 correctly today only because the lid-angle sensor happens not to expose illuminance. Matching name against *als* first would make that intentional rather than incidental — worth it given you also fall back to in_illuminance_raw.
Still open from last time, both small:
- The manual says "Lock and lid-close keep the keys off", but
tickonly returns early — nothing writes the LED. "Automatic control pauses while the screen is locked or the lid is closed" matches the code. [[ -n $als_path && -n $kbd_path ]]; exit $?— underset -euo pipefailthe failing[[ ]]exits with 1 beforeexit $?runs. Works by accident; an explicitif/elsewould say what you mean.
Drop the cp blocks and this is good to go from my side.
Do not copy it into ~/.config/systemd/user, which quattro retires. Prefer IIO devices whose name contains als, exit --available explicitly, and describe lock and lid-close as pausing automatic control.
|
Addressed in 533254d:
|
scottjones
left a comment
There was a problem hiding this comment.
Approving. Everything I raised is addressed, and re-verified on the M2 Max against 533254db9:
- Both
cpblocks into~/.config/systemd/user/are gone, and the migration now enables the package-owned unit with a comment recording exactly why — "quattro retires that path so /usr/lib/systemd/user/ stays authoritative." That is the right fix rather than the one I asked for narrowly. POLL_SECONDS=5.- The manual reads "pauses while the screen is locked or the lid is closed", which matches what
tickactually does. - The
[[ ]]; exit $?construct is gone. - You also took the
find_alsname-filter suggestion and covered both directions — "prefers a named ALS device over an earlier illuminance channel" and the fallback. On this machine that is the difference betweenaop-sensors-alsand the lid-angle sensoraop-sensors-lassitting next to it atiio:device0, so it is doing real work here.
Local run: the focused suite passes, systemd-test.sh passes, omarchy commands --check clean at 457, CI green, no unresolved threads.
One note for after this lands, not a blocker. On this machine neither runtime target exists yet:
/usr/lib/systemd/user/omarchy-brightness-keyboard-auto.service MISSING
/usr/bin/omarchy-brightness-keyboard-auto MISSING
Both arrive with the package rebuild, so until then the migration's systemctl --user enable fails and the fallback writes a symlink pointing at a unit that is not there yet. Harmless and self-healing — systemctl --user is-enabled reports not-found and nothing breaks — but worth knowing so a dangling wants-symlink in the interim does not look like a bug in this PR.
Nice work on this one, and thanks for your patience with my bad POLL_SECONDS call earlier in the thread.
…gurable The ambient light loop from omacom#303 lights the keys in the dark and dims them as the room brightens, but leaves them lit on an untouched laptop, and its thresholds are constants in the script. Three additions, all inside the existing command, which stays the one writer of the LED: - --idle and --active record the idle state under $XDG_RUNTIME_DIR and prod the loop with USR1, so the keys go dark the moment the compositor reports IDLE_SECONDS without input and come back at the first key press or trackpad touch. A new first-party shell service, omarchy.keyboard-backlight, relays the compositor's idle notifier into those two calls; a fast idle -> active flip while a call is still running is re-sent, not dropped. A level set by hand while idle survives the return from idle. - ~/.config/omarchy/keyboard-backlight.conf sets DARK_LUX, BRIGHT_LUX, IDLE_SECONDS, and POLL_SECONDS. Only whole-number assignments to those keys are read, so the file is settings rather than code, and the shell asks the command for IDLE_SECONDS rather than parsing the file a second time. - Hysteresis on the off edge: once bright light has turned the keys off, they stay off until the room is dark enough for a clearly visible level again instead of flickering at 1-2%. The band is a slice of the ramp, so it follows the configured thresholds. The test drives the real loop against a fake sensor and LED, with long polls so every change is the signal path doing the work. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015BSgNp56iTuhMy7XLHsRC2
Summary
Apple Silicon MacBooks expose a working
kbd_backlightLED and an AOP ambient light sensor, but Omarchy never connected them. The keys stay dark in a dark room unless you already know aboutShift+F1/F2.This adds a user service that reads IIO illuminance and drives the keyboard backlight the way macOS does: on in the dark, off in a bright room. It is not Apple-specific — any laptop with
in_illuminance_*and a*kbd_backlight*LED gets the same behavior.ExecConditionmakes the unit a no-op when either piece of hardware is missing.Behavior
Shift+Brightness(or any other write to the LED) pauses auto until ambient light moves enoughTest plan
bash test/shell.d/brightness-keyboard-auto-test.shbash test/shell.d/systemd-test.shExecCondition)Notes
omacom/omarchyquattro oromarchy-macquattro (checked). Related but different: Add a keyboard-backlight settings panel for Apple Silicon Macs #260 is a manual slider/setup panel./usr/lib/systemd/user/, same as the other first-run user units. It is inert untilomarchy-settingsships that unit and theomarchypackage shipsomarchy-brightness-keyboard-auto(ExecConditionexits 203 and systemd skips it until then).apple,j316c,aop-sensors-als+kbd_backlight).