Single instance

AGENTS.md

@@ -48,7 +48,7 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
 - The daemon is split into: `device_tables` (raw HID report tables per model plus
   `decode_gkey_state`, the report → pressed-bitmap decoder), `g910_device` (libusb
   open/detach/claim/read), `uinput_keyboard` (virtual keyboard), `action`
-  (dispatch `key`/`command`/`disabled`), `config_watch` (inotify auto-reload of
+  (dispatch `key`/`command`/`disabled`/`keyhold`), `config_watch` (inotify auto-reload of
   the mappings file) and `ipc_dbus` (the GDBus service). `shared/keymap` does the
   layout lookups for both sides. The hot path never touches the GUI: D-Bus is
   control plane only and is pumped once per loop.
@@ -58,6 +58,20 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   a chord fire every key in it. Media/memory keys and command/LED answers are
   rejected by report length and header; keep it that way instead of
   re-introducing exact-report matching.
+- `keyhold` uses the **falling** edge too: `run_mapping(..., hold=true)` calls
+  `ActionRunner::begin_hold()`, and the 1 → 0 transition of that bit calls
+  `end_hold(index)` (a no-op for every other type). The held codes are stored per
+  G key in `ActionRunner::held_`, so a code that two G keys hold is released only
+  when the *last* one comes up. `release_held()` runs on device loss and on
+  shutdown, otherwise a key whose release report never arrived would stay down in
+  the whole session. `ActionRunner::run()` (the `TestMapping` path) taps a hold
+  mapping, because a D-Bus call cannot wait for a release. Only a single step can
+  be held; a sequence is replayed as a tap with a log line, and `keyhold:` on an
+  unresolvable name does nothing.
+- Auto-repeat of a held key comes from the **X server / compositor**, not from the
+  virtual device: it sets no `EV_REP` bits, so the kernel repeats nothing, but X
+  does (measured: release+press pairs every ~40 ms after 660 ms). That matches a
+  physical key, so do not "fix" it by filtering.
 - The daemon runs **without** a keyboard: `open()` failing (or a read error) only
   logs the absence once, reports `DeviceChanged(false)` and the loop retries every
   2 s while D-Bus and the config watch keep running. Never make it exit on a
@@ -87,9 +101,26 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   id — `GtkApplication` owns that itself to detect a second GUI instance, so a
   daemon on the same name would make every GUI start look like a duplicate and
   exit. All names live in `shared/ipc_ids.h`.
-- The GUI never does a blocking D-Bus call: `DaemonClient` uses async calls and
-  signal subscriptions, which arrive on the GTK main loop, and it follows
-  `NameOwnerChanged` so the footer reacts when the daemon starts or stops.
+- Exactly one GUI runs, and it is the one that was just started: `main()` calls
+  `replace_running_instance()` (`src/singleinstance.{h,cpp}`) **before** it
+  creates the `GtkApplication`. It asks the instance that owns `ipc::kAppId` to
+  quit over D-Bus (the `quit` action `GApplication` exports as
+  `org.gtk.Actions` at the app-id path, added in `on_activate`) and waits for the
+  name; an instance from an older build has no such action, so it is terminated
+  with `SIGTERM` after checking that the bus name really belongs to a
+  `gkeymapper` process. Only then does the new process become the primary,
+  because otherwise `GtkApplication` would hand over to the old process and its
+  window — possibly one from an older build — would stay on screen. Row edits
+  that were not saved are gone with the replaced window; there is no implicit
+  save.
+- `on_activate` presents the window that is already there
+  (`gtk_application_get_windows()`), it does not build a second one: a remote
+  activation — an older build starting next to a new one — would otherwise add a
+  second window to the running process.
+- Except for that startup takeover, the GUI never does a blocking D-Bus call:
+  `DaemonClient` uses async calls and signal subscriptions, which arrive on the
+  GTK main loop, and it follows `NameOwnerChanged` so the footer reacts when the
+  daemon starts or stops.
 - `DaemonService::pump()` dispatches messages once per daemon loop iteration, so
   a control call is answered within one read timeout (≤500 ms) and outgoing
   signals are flushed then too. Keep it off the keypress path.
@@ -102,8 +133,10 @@ live in the daemon, not the GUI. Code is split across `src/` (GUI), `shared/`
   over AT-SPI as a generic `filler`, not as a "statusbar" role, so it buys
   nothing here.
 - The Type dropdown index is meaningful everywhere: `0=Key`, `1=Command`,
-  `2=Disabled`, matching `ActionType` and the `key`/`command`/`disabled` tokens
-  in the file. Keep that order.
+  `2=Disabled`, `3=Key (hold)`, matching the `key`/`command`/`disabled`/`keyhold`
+  tokens in the file. Keep that order (`ActionType` appends new values at the end
+  — `Disabled, Key, Command, KeyHold` — so an existing value never changes
+  meaning; the GUI maps indices explicitly).
 - The capture dialog's `CaptureSession` is heap-allocated and freed only by the
   window's `"destroy"` handler. Always close the dialog with
   `gtk_window_destroy` so the session is released.
@@ -126,11 +159,13 @@ the same format:
 G1=key:ctrl+c,ctrl+v
 G2=command:/usr/bin/firefox
 G3=disabled:
+G4=keyhold:ctrl+w
 ```
 
 `<type>:<target>`, one line per key. `key` is a captured sequence (lowercase
 `gdk_keyval_name` output; modifiers joined with `+`, steps joined with `,`),
-`command` is a program path or command line, `disabled` is inactive. The
+`command` is a program path or command line, `disabled` is inactive, and
+`keyhold` presses one step and keeps it down while the G key is held. The
 canonical modifier order is `ctrl`, `alt`, `shift`, `super`, `altgr`
 (`format_shortcut_step()`; parsing accepts any order).
 
@@ -226,6 +261,28 @@ over any installed file; the file lookup remains only as a safety net.
     script that dumps its environment is also how you check that a `command`
     action sees `DISPLAY`); with `key:` mappings the fake would inject keys into
     whatever window is focused.
+    Two details when the fake has to exercise `key:`/`keyhold:` mappings:
+    - **Copy `main.cpp` next to the fake**, e.g. `/tmp/…/fakesrc/main.cpp`, and put
+      the fake `g910_device.h` in an include directory. `#include "g910_device.h"`
+      searches the *including file's* directory first, so compiling the real
+      `daemon/main.cpp` would always pick up the real header (its inline
+      `is_open()` needs libusb).
+    - **Give the fake its own session bus** (`dbus-daemon --session --address=
+      unix:path=/tmp/…/fake_bus --fork --print-pid=1`, then
+      `DBUS_SESSION_BUS_ADDRESS=unix:path=…` for the fake). Otherwise it refuses
+      to start while the real daemon is running, and stopping the user service for
+      a test loses the media keys and the GUI footer.
+    - **Read the events back over X**, not from `/dev/input`: the event node of an
+      own uinput device has **no ACL** for the user (`/dev/uinput` does, its
+      children do not), so `open()` fails with `EACCES`. `xinput test <id>` works
+      without root; identify the new device by diffing `xinput list` ids before and
+      after (the name is the same as the installed daemon's virtual keyboard). X
+      keycodes are evdev + 8, and `xinput test` shows no `EV_SYN`, so
+      simultaneity of a chord cannot be read that way — the press/release order
+      and the durations can.
+    - Use inert keys: `f13`–`f24` are **not** in this layout (`Keymap::key_code`
+      returns -1), `pause` and `kp_equal` are. A held Shift for a few 100 ms is
+      harmless, a held Ctrl is not.
 - D-Bus activation is only testable in a real setup: the installed
   `share/dbus-1/services/dev.rager.gkeymapper.Daemon.service` needs
   `sudo make install`. For a quick check without root, drop a file with
@@ -245,7 +302,10 @@ over any installed file; the file lookup remains only as a safety net.
   mute/volume/play only on that very interface (`…/input1`, measured — see below),
   so the daemon replays those reports unchanged through its virtual keyboard.
   Deleting that passthrough in `daemon/main.cpp` silently kills the media keys
-  while the daemon runs.
+  while the daemon runs. Note that they are deliberately *not* a mapping and not
+  going to become one: making them remappable is decided against (plan §11), so do
+  not turn `mediaKeys` into config rows or GUI fields without that decision being
+  revisited first.
 - Media keys cannot be observed over the keyboard's usual event node: with the
   daemon stopped they appear on `event11` (`Phys …/input1`, created when `usbhid`
   owns the interface), not on `event10` (`…/input0`); *both* nodes advertise the
@@ -273,11 +333,22 @@ a child `label` node, so target the button by its child label. `pyatspi`
 and inject keys. Warn the user before synthesizing keystrokes or stealing window
 focus.
 
+The one-instance behavior needs no hardware: start the GUI twice and check that
+one process (`pgrep -x gkeymapper`) and one window (`wmctrl -l`) are left. The
+replaced process exits `0` when it was closed through the D-Bus `quit` action, or
+`143` (`SIGTERM`) when it came from a build without that action — build such an
+instance from an older commit (`git worktree add /tmp/legacy <commit>`), start it
+and then start the current build next to it. Starting the GUI twice also covers
+the `activate` guard: without it the second start adds a second window to the
+running process.
+
 The footer's daemon line is a plain `GtkLabel`, so its text can be read back over
 AT-SPI: `<app> → frame → panel → label` whose name starts with the translated
 `Daemon:` string. To pick the right window, filter the application whose tree
-contains that label — a stale GUI from an earlier build may be running from the
-same binary name. Three notes for tests:
+contains that label. A stale instance is not a problem any more: starting the
+binary under test closes whatever is running (see the single-instance notes
+above) before it shows its window, so it is safe to start it while an older GUI
+is up. Three notes for tests:
 - Clicking `Save mappings` writes `~/.config/gKeyMapper/mappings.conf` for real.
   Since M5 the window loads that file at startup, so the rows show the saved
   mappings and a save rewrites the same content; back the file up anyway (or skip