From d22415f07fbb3ce9de49d9ae960dc7cd2e3be649 Mon Sep 17 00:00:00 2001 From: yekingyan <529616@gmail.com> Date: Fri, 12 Jun 2026 22:27:00 +0800 Subject: [PATCH 1/3] fix(osm): keep OSM/OSL timeout alive across non-consuming events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OSM/OSL release path used a single select(timeout, next_event), so the first event arriving during the timeout cancelled the wait permanently, leaving the one-shot stuck in Single (never auto-released). A typical trigger is the release event of a layer key. Replace the single select with a deadline loop: non-consuming events are pushed to the queue and we keep waiting for the remaining time, while a consuming event (OSM: press in quick_release mode, release otherwise; OSL: press) breaks out. Additionally, check unprocessed_events before entering the await loop — if a consuming event was already dequeued from the channel, the subscriber would never see it, causing the OSM to hang indefinitely. --- rmk/src/keyboard/oneshot.rs | 88 +++++++++++++++++++++++++++---------- 1 file changed, 65 insertions(+), 23 deletions(-) diff --git a/rmk/src/keyboard/oneshot.rs b/rmk/src/keyboard/oneshot.rs index aa8b1b75f..df9fc51ed 100644 --- a/rmk/src/keyboard/oneshot.rs +++ b/rmk/src/keyboard/oneshot.rs @@ -1,5 +1,5 @@ use embassy_futures::select::{Either, select}; -use embassy_time::Timer; +use embassy_time::{Instant, Timer}; use rmk_types::modifier::ModifierCombination; use crate::event::KeyboardEvent; @@ -72,22 +72,48 @@ impl<'a> Keyboard<'a> { match self.osm_state { OneShotState::Initial(cur_modifiers) | OneShotState::Single(cur_modifiers) => { self.osm_state = OneShotState::Single(cur_modifiers); - let timeout = Timer::after(self.keymap.one_shot_timeout()); - match select(timeout, self.keyboard_event_subscriber.next_message_pure()).await { - Either::First(_) => { - // Timeout, release modifiers - self.update_osl(event); - self.osm_state = OneShotState::None; - - // Send release report because modifiers were held - if activate_on_keypress { - self.send_keyboard_report_with_resolved_modifiers(false).await; + let quick_release = self.keymap.one_shot_modifiers_config().quick_release; + + // If unprocessed_events already contains a consuming event, skip the + // await loop — waiting on the subscriber would miss it because events + // already dequeued from the channel live only in unprocessed_events. + let already_has_consumer = self + .unprocessed_events + .iter() + .any(|e| (quick_release && e.pressed) || (!quick_release && !e.pressed)); + + if !already_has_consumer { + let deadline = Instant::now() + self.keymap.one_shot_timeout(); + loop { + let now = Instant::now(); + if now >= deadline { + self.update_osl(event); + self.osm_state = OneShotState::None; + if activate_on_keypress { + self.send_keyboard_report_with_resolved_modifiers(false).await; + } + break; } - } - Either::Second(e) => { - // New event, send it to queue - if self.unprocessed_events.push(e).is_err() { - warn!("Unprocessed event queue is full, dropping event"); + let timeout = Timer::after(deadline - now); + match select(timeout, self.keyboard_event_subscriber.next_message_pure()).await { + Either::First(_) => { + self.update_osl(event); + self.osm_state = OneShotState::None; + if activate_on_keypress { + self.send_keyboard_report_with_resolved_modifiers(false).await; + } + break; + } + Either::Second(e) => { + if self.unprocessed_events.push(e).is_err() { + warn!("Unprocessed event queue is full, dropping event"); + } + // If this event would consume the OSM, stop waiting + if (quick_release && e.pressed) || (!quick_release && !e.pressed) { + break; + } + // Non-consuming event (e.g. layer key release), keep waiting + } } } } @@ -136,17 +162,33 @@ impl<'a> Keyboard<'a> { OneShotState::Initial(l) | OneShotState::Single(l) => { self.osl_state = OneShotState::Single(l); - let timeout = embassy_time::Timer::after(self.keymap.one_shot_timeout()); - match select(timeout, self.keyboard_event_subscriber.next_message_pure()).await { - Either::First(_) => { + let deadline = Instant::now() + self.keymap.one_shot_timeout(); + loop { + let now = Instant::now(); + if now >= deadline { // Timeout, deactivate layer self.keymap.deactivate_layer(layer_num); self.osl_state = OneShotState::None; + break; } - Either::Second(e) => { - // New event, send it to queue - if self.unprocessed_events.push(e).is_err() { - warn!("Unprocessed event queue is full, dropping event"); + let timeout = Timer::after(deadline - now); + match select(timeout, self.keyboard_event_subscriber.next_message_pure()).await { + Either::First(_) => { + // Timeout, deactivate layer + self.keymap.deactivate_layer(layer_num); + self.osl_state = OneShotState::None; + break; + } + Either::Second(e) => { + // New event, send it to queue + if self.unprocessed_events.push(e).is_err() { + warn!("Unprocessed event queue is full, dropping event"); + } + // A key press consumes the one-shot layer. + if e.pressed { + break; + } + // Release events (e.g. layer key release) don't consume, keep waiting } } } From 055fd993d27c0e9cab67db8a5010476d9be49023 Mon Sep 17 00:00:00 2001 From: yekingyan <529616@gmail.com> Date: Fri, 12 Jun 2026 22:27:05 +0800 Subject: [PATCH 2/3] test(osm): add regression test for timeout cancelled by non-consuming event --- rmk/tests/keyboard_one_shot_test.rs | 35 +++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/rmk/tests/keyboard_one_shot_test.rs b/rmk/tests/keyboard_one_shot_test.rs index b169d1381..63dff3f44 100644 --- a/rmk/tests/keyboard_one_shot_test.rs +++ b/rmk/tests/keyboard_one_shot_test.rs @@ -658,8 +658,39 @@ mod one_shot_test { }; } - // TODO: test_osm_quick_release_rolling removed — OSM + morse/tap-hold interaction - // has a known bug where the OSM deadline loop times out before the tap resolves. + // NOTE: test_osm_quick_release_rolling is intentionally omitted. With a + // tap-hold key the OSM modifier is cleared before the buffered morse tap + // resolves, so the tapped key is emitted without the modifier. That is a + // separate OSM + morse/tap-hold timing issue (not a timeout bug). + + /// Regression test: a non-consuming event arriving during the OSM release + /// timeout must NOT cancel that timeout. Previously the single + /// `select(timeout, next_event)` gave up the timeout on the first event, + /// leaving the OSM stuck in `Single` forever. Here an OSL press interrupts + /// the wait; after the timeout elapses the OSM must have auto-released, so + /// the later layer-1 key is sent WITHOUT the one-shot modifier. + #[test] + fn test_osm_timeout_not_cancelled_by_non_consuming_event() { + key_sequence_test! { + keyboard: create_test_keyboard_with_behavior_config(BehaviorConfig { + one_shot: OneShotConfig { + timeout: Duration::from_millis(100), + }, + ..BehaviorConfig::default() + }), + sequence: [ + [0, 0, true, 10], // Press OSM LShift + [0, 0, false, 10], // Release OSM LShift -> Single, start timeout + [0, 1, true, 10], // Press OSL(1): non-consuming event during timeout + [0, 2, true, 200], // After timeout elapses, press layer-1 key C + [0, 2, false, 10], // Release C + ], + expected_reports: [ + [0, [kc_to_u8!(C), 0, 0, 0, 0, 0]], // C WITHOUT LShift (OSM timed out) + [0, [0, 0, 0, 0, 0, 0]], // All released + ] + }; + } #[test] fn test_osm_quick_release_combined_modifiers() { From 0610cb4cdf86979ae3fc5ab1c080f0e83ea48576 Mon Sep 17 00:00:00 2001 From: yekingyan <529616@gmail.com> Date: Fri, 3 Jul 2026 20:25:02 +0800 Subject: [PATCH 3/3] fix(osl): use the layer bound in the match arm instead of the outer param In the OSL release path, the Initial(l) | Single(l) arm was calling self.keymap.deactivate_layer(layer_num) using the outer function parameter instead of the layer number l bound by the match arm. Currently these happen to be numerically equal on the only call path, but relying on that is fragile and semantically wrong: the deactivated layer should always be the one actually stored in osl_state, not the layer passed into this invocation of process_action_osl. Addresses review comment from @HaoboGu on PR #866. --- rmk/src/keyboard/oneshot.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rmk/src/keyboard/oneshot.rs b/rmk/src/keyboard/oneshot.rs index df9fc51ed..5583efec8 100644 --- a/rmk/src/keyboard/oneshot.rs +++ b/rmk/src/keyboard/oneshot.rs @@ -167,7 +167,7 @@ impl<'a> Keyboard<'a> { let now = Instant::now(); if now >= deadline { // Timeout, deactivate layer - self.keymap.deactivate_layer(layer_num); + self.keymap.deactivate_layer(l); self.osl_state = OneShotState::None; break; } @@ -175,7 +175,7 @@ impl<'a> Keyboard<'a> { match select(timeout, self.keyboard_event_subscriber.next_message_pure()).await { Either::First(_) => { // Timeout, deactivate layer - self.keymap.deactivate_layer(layer_num); + self.keymap.deactivate_layer(l); self.osl_state = OneShotState::None; break; }