Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 56 additions & 5 deletions crates/openlogi-agent-core/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,30 @@ pub fn bindings_for(
pub fn gesture_bindings_for(
config: &Config,
config_key: Option<&str>,
) -> BTreeMap<GestureDirection, Action> {
gesture_bindings_for_app(config, config_key, None)
}

/// Effective gesture bindings for the dedicated HID++ gesture button, with a
/// foreground application's plain-click override applied when present.
#[must_use]
pub fn gesture_bindings_for_app(
config: &Config,
config_key: Option<&str>,
app_bundle: Option<&str>,
) -> BTreeMap<GestureDirection, Action> {
// The dedicated HID++ gesture button (CID 0x00c3) only gestures while it is the device's gesture
// owner. When the user moves the role to an OS-hook button (Middle/Back/
// Forward) or turns gestures off, return an empty map so the gesture watcher
// dispatches nothing — otherwise the always-seeded defaults would keep the
// HID++ gesture button firing regardless of the selection.
let owner = config_key.and_then(|key| config.gesture_owner(key));
if owner != Some(ButtonId::GestureButton) {
let Some(key) = config_key else {
return BTreeMap::new();
};
Comment on lines +73 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Unreachable early-return guard

The let Some(key) = config_key else { return BTreeMap::new() } branch can never execute: the owner check immediately above already calls config_key.and_then(|key| config.gesture_owner(key)), so when config_key is None the owner is None, None != Some(ButtonId::GestureButton) fires, and the function returns an empty map before reaching this guard. The let Some(key) arm is dead code that could confuse future readers into thinking there is a reachable None-key path distinct from the owner-check path.

Fix in Codex Fix in Claude Code

if config.gesture_owner(key) != Some(ButtonId::GestureButton) {
return BTreeMap::new();
}
let stored = config_key
.map(|key| config.gesture_bindings_for(key))
.unwrap_or_default();
let stored = config.gesture_bindings_for(key);
let mut bindings: BTreeMap<GestureDirection, Action> = GestureDirection::ALL
.iter()
.copied()
Expand All @@ -74,6 +85,17 @@ pub fn gesture_bindings_for(
for (k, v) in stored {
bindings.insert(k, v);
}
// Per-app bindings are deliberately single-action overrides. For the
// dedicated HID++ gesture button that action applies to a plain press,
// while the global directional gestures remain intact.
if let Some(app) = app_bundle
&& config.has_app_override(key, app)
&& let Some(Binding::Single(action)) = config
.effective_bindings(key, Some(app))
.get(&ButtonId::GestureButton)
{
bindings.insert(GestureDirection::Click, action.clone());
}
bindings
}

Expand Down Expand Up @@ -241,4 +263,33 @@ mod tests {
"HID++ gesture button must dispatch nothing once another button owns gestures"
);
}

#[test]
fn dedicated_gesture_button_applies_per_app_click_override() {
let mut cfg = Config::default();
cfg.set_gesture_owner("2b042", ButtonId::GestureButton);
cfg.set_per_app_binding(
"2b042",
"com.apple.Safari",
ButtonId::GestureButton,
Some(Action::MouseBack),
);

let global = gesture_bindings_for(&cfg, Some("2b042"));
let safari = gesture_bindings_for_app(&cfg, Some("2b042"), Some("com.apple.Safari"));

assert_eq!(
global.get(&GestureDirection::Click),
Some(&default_gesture_binding(GestureDirection::Click))
);
assert_eq!(
safari.get(&GestureDirection::Click),
Some(&Action::MouseBack)
);
assert_eq!(
safari.get(&GestureDirection::Up),
global.get(&GestureDirection::Up),
"the per-app click override must preserve directional gestures"
);
}
}
63 changes: 56 additions & 7 deletions crates/openlogi-agent-core/src/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use openlogi_hid::{CaptureChannel, DeviceRoute};
use tracing::warn;

use crate::DpiCycleState;
use crate::bindings::{bindings_for, gesture_bindings_for, oshook_gestures_for};
use crate::bindings::{bindings_for, gesture_bindings_for_app, oshook_gestures_for};
use crate::device_order::DeviceStableId;
use crate::hook_runtime::{HookMaps, SharedHookMaps};
use crate::ipc::InventoryHealth;
Expand Down Expand Up @@ -166,7 +166,7 @@ impl Orchestrator {
);
write_value(
&self.shared.gesture_bindings,
gesture_bindings_for(&self.config, key),
gesture_bindings_for_app(&self.config, key, self.current_app.as_deref()),
"gesture_bindings",
);
write_value(
Expand Down Expand Up @@ -338,11 +338,10 @@ impl Orchestrator {
self.config.app_settings.launch_at_login
}

/// Foreground-app change → re-overlay per-app bindings on the hook maps (DPI
/// and the dedicated HID++ gesture map are not app-scoped, so they're untouched).
/// Both hook maps are recomputed: a per-app override of the gesture owner
/// turns it into a single action for that app, dropping it from the OS-hook
/// gesture set — so the gesture map is app-scoped too.
/// Foreground-app change → re-overlay per-app bindings on the hook maps and
/// the dedicated HID++ gesture button's plain-click action. Both hook maps
/// are recomputed: a per-app override of the gesture owner turns it into a
/// single action for that app, dropping it from the OS-hook gesture set.
pub fn set_current_app(&mut self, bundle: Option<String>) {
if bundle == self.current_app {
return;
Expand All @@ -353,6 +352,15 @@ impl Orchestrator {
self.hook_maps_for(self.current_key(), self.current_app.as_deref()),
"hook_maps",
);
write_value(
&self.shared.gesture_bindings,
gesture_bindings_for_app(
&self.config,
self.current_key(),
self.current_app.as_deref(),
),
"gesture_bindings",
);
}

/// Replace the config (after `config.toml` changed) and rebuild everything.
Expand Down Expand Up @@ -521,6 +529,7 @@ mod tests {
AgentDevice, InventoryHealth, Orchestrator, configured_wheel_mode, plan_reapply,
reapply_targets,
};
use openlogi_core::binding::{Action, ButtonId, GestureDirection};
use openlogi_core::config::{Config, ScrollResolution};
use openlogi_core::device::Capabilities;
use openlogi_hid::DeviceRoute;
Expand All @@ -541,6 +550,14 @@ mod tests {
}
}

fn gesture_click(orch: &Orchestrator) -> Option<Action> {
orch.shared
.gesture_bindings
.read()
.ok()
.and_then(|bindings| bindings.get(&GestureDirection::Click).cloned())
}

#[test]
fn configured_wheel_mode_gates_resolution_and_inversion_independently() {
let mut config = Config::default();
Expand Down Expand Up @@ -705,4 +722,36 @@ mod tests {
orch.mark_inventory_unavailable();
assert_eq!(orch.inventory_health(), InventoryHealth::Ready);
}

#[test]
fn foreground_app_updates_dedicated_gesture_click() {
let mut config = Config::default();
config.set_gesture_owner("2b042", ButtonId::GestureButton);
config.set_per_app_binding(
"2b042",
"com.apple.Safari",
ButtonId::GestureButton,
Some(Action::BrowserBack),
);
let mut orch = Orchestrator::new(config);
orch.devices.push(AgentDevice {
config_key: "2b042".into(),
model_key: "2b042".into(),
route: None,
slot: 0,
serial: None,
unit_id: [0; 4],
capabilities: None,
online: true,
});
orch.rebuild();

assert_eq!(gesture_click(&orch), Some(Action::AppExpose));

orch.set_current_app(Some("com.apple.Safari".into()));
assert_eq!(gesture_click(&orch), Some(Action::BrowserBack));

orch.set_current_app(Some("com.other.App".into()));
assert_eq!(gesture_click(&orch), Some(Action::AppExpose));
}
}