Skip to content

feat(keyboard_locks): add Caps/Num/Scroll lock indicators - #755

Open
noirbizarre wants to merge 1 commit into
MalpenZibo:mainfrom
noirbizarre:feat/keyboard-locks
Open

feat(keyboard_locks): add Caps/Num/Scroll lock indicators#755
noirbizarre wants to merge 1 commit into
MalpenZibo:mainfrom
noirbizarre:feat/keyboard-locks

Conversation

@noirbizarre

@noirbizarre noirbizarre commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a new bar module displaying Caps/Num/Scroll Lock state, with click-to-toggle support.

  • Reads LED state from /dev/input/event* via evdev (compositor-agnostic — works on both Hyprland and Niri).
  • Hotplug-aware via a udev monitor on the input subsystem.
  • Clicking an indicator toggles the corresponding lock through a lazily-created /dev/uinput virtual keyboard. If /dev/uinput cannot be opened, the display still works and clicks become no-ops with a single warning.
  • Per-lock config: enabled, visibility (ActiveOnly default / AlwaysVisible), and an optional icon override (Nerd Font glyph or plain text). Default glyphs bundled (no extra font install required).
  • Module renders the three indicators flush inside a single rounded island (no inner pill rounding), each with a per-icon hover highlight that matches the rest of the bar.

Permissions

Both /dev/input/event* and /dev/uinput are typically owned by the input group, so a single sudo usermod -aG input \"\$USER\" is enough. The user docs cover the manual udev-rule fallback for systems where /dev/uinput is not in the input group by default.

Testing

  • make check (fmt + cargo check + clippy -D warnings) is clean.
  • Verified manually with bar_style = \"Islands\" and all three locks set to AlwaysVisible: glyphs render flush inside one island, click toggles the corresponding lock, LED state updates after the kernel synthesizes the press.
  • Verified that without /dev/uinput access the display still updates from external lock presses; clicks log a single warning and no-op.

Docs

website/docs/configuration/modules/keyboard.md adds a new "Keyboard Locks" section. Versioned docs under website/versioned_docs/ are untouched per the project policy.

Screenshots

Screenshot-2026-05-16-030719 Screenshot-2026-05-16-030747

Notes

  • I've been using it until now without any issue.
  • I've been using AI to code this, I prefer to be transparent on this (and hope this is not an issue)

@MalpenZibo MalpenZibo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sorry, I'm a bit late, but this PR is quite complex, so I decided to postpone it after the release.

Anyway, if I understand correctly, this PR can retrieve the current status of the lock indicators and react to a change by watching the lock indicator status.

For example, if ashell starts after the user clicks the Caps Lock, we're able to understand that the CAPS lock is active.

The only downside is the permissions that you have documented. A nice addition.

I will take some time to complete the review because the keyboard_locks is more complex than I thought!

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ok, the PR seems good. I only find the service part a little bit confusing.

If I understand correctly, we're spawning a tokio task for each keyboard, then we use an mpsc channel and track the join handle in a HashMap. Maybe we can handle everything with a simple SelectAll.

  fn open_lock_stream(path: &Path) -> Option<(KeyboardLocksData, EventStream)> {
      let device = match open_device_with_retry(path) {
          Ok(Some(device)) => device,
          Ok(None) => return None,
          Err(err) => {
              debug!("failed to open {}: {err}", path.display());
              return None;
          }
      };
      if !has_lock_leds(&device) {
          return None;
      }
      let initial = read_lock_state(&device)
          .map_err(|err| debug!("failed to read LED state for {}: {err}", path.display()))
          .ok()?;
      let stream = device
          .into_event_stream()
          .map_err(|err| debug!("failed to start event stream for {}: {err}", path.display()))
          .ok()?;
      Some((initial, stream))
  }

  fn device_lock_stream(
      path: PathBuf,
      initial: KeyboardLocksData,
      stream: EventStream,
  ) -> BoxStream<'static, (PathBuf, Option<KeyboardLocksData>)> {
      stream::unfold(
          (stream, initial, path, false),
          |(mut stream, mut current, path, ended)| async move {
              if ended {
                  return None;
              }
              loop {
                  match stream.next_event().await {
                      Ok(event) => {
                          let EventSummary::Led(_, code, value) = event.destructure() else {
                              continue;
                          };
                          let on = value != 0;
                          let changed = match code {
                              LedCode::LED_CAPSL if current.caps_lock != on => {
                                  current.caps_lock = on;
                                  true
                              }
                              LedCode::LED_NUML if current.num_lock != on => {
                                  current.num_lock = on;
                                  true
                              }
                              LedCode::LED_SCROLLL if current.scroll_lock != on => {
                                  current.scroll_lock = on;
                                  true
                              }
                              _ => false,
                          };
                          if changed {
                              return Some(((path.clone(), Some(current)), (stream, current, path, false)));
                          }
                      }
                      Err(err) => {
                          debug!("event stream error for {}: {err}; dropping device", path.display());
                          return Some(((path.clone(), None), (stream, current, path, true)));
                      }
                  }
              }
          },
      )
      .boxed()
  }

  fn add_device(
      path: PathBuf,
      per_device: &mut HashMap<PathBuf, KeyboardLocksData>,
      streams: &mut SelectAll<BoxStream<'static, (PathBuf, Option<KeyboardLocksData>)>>,
  ) {
      if per_device.contains_key(&path) {
          return;
      }
      if let Some((initial, stream)) = open_lock_stream(&path) {
          per_device.insert(path.clone(), initial);
          streams.push(device_lock_stream(path, initial, stream));
      }
  }

  async fn supervise(
      output: &mut Sender<ServiceEvent<KeyboardLocksService>>,
      mut commands: UnboundedReceiver<KeyboardLocksCommand>,
  ) {
      let mut monitor = match input_subsystem_monitor() {
          Ok(monitor) => monitor,
          Err(err) => {
              warn!("Failed to set up input udev monitor: {err}");
              return;
          }
      };

      let mut per_device: HashMap<PathBuf, KeyboardLocksData> = HashMap::new();
      let mut streams: SelectAll<BoxStream<'static, (PathBuf, Option<KeyboardLocksData>)>> =
          SelectAll::new();
      let mut current_aggregate = KeyboardLocksData::default();
      let mut emitter = ToggleEmitter::new();

      if let Ok(paths) = enumerate_event_devices() {
          for path in paths {
              add_device(path, &mut per_device, &mut streams);
          }
      }

      info!(
          "Keyboard locks service listening (tracking {} devices)",
          per_device.len()
      );

      loop {
          tokio::select! {
              Some((path, update)) = streams.next() => {
                  match update {
                      Some(state) => { per_device.insert(path, state); }
                      None => { per_device.remove(&path); }
                  }
                  let new_aggregate = aggregate(&per_device);
                  if new_aggregate != current_aggregate {
                      current_aggregate = new_aggregate;
                      if output
                          .send(ServiceEvent::Update(KeyboardLocksEvent(current_aggregate)))
                          .await
                          .is_err()
                      {
                          break;
                      }
                  }
              }
              cmd = commands.recv() => {
                  let Some(cmd) = cmd else { break };
                  match cmd {
                      KeyboardLocksCommand::Toggle(kind) => emitter.toggle(kind),
                  }
              }
              guard = monitor.writable_mut() => {
                  match guard {
                      Ok(mut guard) => {
                          let events: Vec<_> = guard.get_inner().iter().collect();
                          for evt in events {
                              handle_udev_event(&evt, &mut per_device, &mut streams);
                          }
                          guard.clear_ready();
                      }
                      Err(err) => {
                          warn!("input udev monitor failed: {err}");
                          break;
                      }
                  }
              }
          }
      }
  }

  fn handle_udev_event(
      evt: &udev::Event,
      per_device: &mut HashMap<PathBuf, KeyboardLocksData>,
      streams: &mut SelectAll<BoxStream<'static, (PathBuf, Option<KeyboardLocksData>)>>,
  ) {
      let Some(devnode) = evt.device().devnode().map(|p| p.to_path_buf()) else {
          return;
      };
      let Some(name) = devnode.file_name().and_then(|n| n.to_str()) else {
          return;
      };
      if !name.starts_with("event") {
          return;
      }
      match evt.event_type() {
          udev::EventType::Add => add_device(devnode, per_device, streams),
          // On Remove the device's stream surfaces a read error and self-removes,
          // emitting the `None` that clears `per_device` — nothing to abort here.
          udev::EventType::Remove => {}
          _ => {}
      }
  }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's a much better shape — applied in the latest push.

The service now keeps a single SelectAll<BoxStream<'static, (PathBuf, Option<KeyboardLocksData>)>> instead of one tokio task + mpsc + JoinHandle per keyboard:

  • open_lock_stream opens/filters the device and returns the initial state plus the EventStream.
  • device_lock_stream wraps it in stream::unfold, yielding only on an actual state change and a final None payload when the underlying stream errors out, after which the stream terminates and SelectAll drops it automatically.
  • add_device is shared between the initial enumeration and the udev Add path.
  • handle_udev_event no longer needs a Remove branch — the stream self-removes and emits the None that clears per_device, so the explicit abort/removal signalling is gone.

That also removed the tasks: HashMap<PathBuf, JoinHandle<()>>, the internal DeviceUpdate struct, and the best-effort abort loop at the end of supervise. Net -24 lines and no more tokio::spawn in the module.

One small deviation from your snippet: I kept the PermissionDenied arm in open_lock_stream so an inaccessible device is logged distinctly from a generic open failure — it's still just a debug! + None.

Also rebased on current main (the Message enum moved to src/app/message.rs in the meantime). make check is clean.

@noirbizarre
noirbizarre force-pushed the feat/keyboard-locks branch from 1c5752d to 9948874 Compare July 28, 2026 02:34
@noirbizarre

Copy link
Copy Markdown
Contributor Author

Hi 👋🏼
My turn to be sorry for the late response 😉.

I addressed the review comment.

To answer your question, yes, it displays the locks (CapsLock, NumLock) as a widget. It offload the state and event/update responsibility to udev, explaining the permissions requirement. And yes, udev allows us to properly catch-up the status on late startup

@MalpenZibo MalpenZibo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I have some security concerns about this


let mut per_device: HashMap<PathBuf, KeyboardLocksData> = HashMap::new();
let mut streams = DeviceStreams::new();
let mut current_aggregate = KeyboardLocksData::default();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

current_aggregate is define with default and per_device is seeded with real data. What happen if the Num lock is on at boot? Turning it off should produce an aggregate identical to the starting one and no Update is sent

return;
}

// On `Remove` the device's stream surfaces a read error and self-removes,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We are sure that the stream errors out before udev reports a re-Add on the same event?

Err(err) => {
last_err = Some(err);
if attempt < 2 {
std::thread::sleep(Duration::from_millis(50));

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's better to use tokio::time::sleep

KeyboardLocksCommand::Toggle(kind) => emitter.toggle(kind),
}
}
guard = monitor.writable_mut() => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you try with?

Suggested change
guard = monitor.writable_mut() => {
guard = monitor.readable_mut() => {

Comment on lines +60 to +61
Both nodes are owned by the `input` group on most distributions, so the user
running ashell just needs to be a member of that group:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Here we should explicitly state that this will give any process running as that user read access to all keystrokes, not just ashell.

Also, I have some concern about this. I'm not sure if I really want to suggest to people to add their user to the input group. It's not a trivial security concern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants