diff --git a/rmk/src/keyboard/oneshot.rs b/rmk/src/keyboard/oneshot.rs index aa8b1b75f..5583efec8 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.keymap.deactivate_layer(l); 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(l); + 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 } } } 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() {