Skip to content

Commit d1c9225

Browse files
committed
fix(morse): resolve same-hand HRM tap on press in Normal mode
In Normal mode with unilateral_tap enabled, a same-hand key pressed while an HRM was held would fire immediately (no buffering), causing the roll to come out in the wrong order. The unilateral tap only fired on key release, by which point the plain key had already been reported first. Fix by checking unilateral_tap + same-hand on press in the Normal mode arm of make_decisions_for_keys, triggering UnilateralTap for the held HRM immediately and setting CleanBuffer so the incoming key fires after. Add test: test_normal_mode_same_hand_roll_order.
1 parent e2f2be6 commit d1c9225

3 files changed

Lines changed: 91 additions & 8 deletions

File tree

rmk/src/config/positional.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,14 @@ impl<const ROW: usize, const COL: usize> PositionalConfig<ROW, COL> {
3737
Self { hand }
3838
}
3939
}
40+
41+
impl Hand {
42+
/// Whether two keys sit on the same hand for unilateral-tap decisions.
43+
///
44+
/// Only `Left`/`Left` and `Right`/`Right` qualify. `Unknown` keys and
45+
/// `Bilateral` keys (which are deliberately exempt from same-hand rules)
46+
/// always return `false`, even when paired with themselves.
47+
pub fn is_same_side(self, other: Hand) -> bool {
48+
matches!((self, other), (Hand::Left, Hand::Left) | (Hand::Right, Hand::Right))
49+
}
50+
}

rmk/src/keyboard.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use rmk_types::mouse_button::MouseButtons;
1717
use usbd_hid::descriptor::{MediaKeyboardReport, SystemControlReport};
1818

1919
use crate::channel::send_hid_report;
20-
use crate::config::Hand;
2120
use crate::core_traits::Runnable;
2221
#[cfg(all(feature = "split", feature = "_ble"))]
2322
use crate::event::ClearPeerEvent;
@@ -725,7 +724,30 @@ impl<'a> Keyboard<'a> {
725724
let _ = decisions.push((held_key.event.pos, HeldKeyDecision::HoldOnOtherKeyPress));
726725
decision_for_current_key = KeyBehaviorDecision::CleanBuffer;
727726
}
728-
_ => {}
727+
MorseMode::Normal => {
728+
// Normal mode: resolve a same-hand HRM as tap on press when
729+
// unilateral_tap is enabled, so the roll fires in the correct
730+
// order (HRM tap first, then the new key).
731+
let unilateral_tap =
732+
Self::is_unilateral_tap_enabled(self.keymap, &held_key.action);
733+
if unilateral_tap
734+
&& matches!(held_key.state, KeyState::Pressed(_))
735+
&& let KeyboardEventPos::Key(pos1) = held_key.event.pos
736+
&& let KeyboardEventPos::Key(pos2) = event.pos
737+
{
738+
let hand1 =
739+
self.keymap.hand_at(pos1.row as usize, pos1.col as usize);
740+
let hand2 =
741+
self.keymap.hand_at(pos2.row as usize, pos2.col as usize);
742+
if hand1.is_same_side(hand2) {
743+
debug!("Unilateral tap on press (Normal mode): resolving HRM as tap for correct roll order");
744+
let _ = decisions
745+
.push((held_key.event.pos, HeldKeyDecision::UnilateralTap));
746+
decision_for_current_key = KeyBehaviorDecision::CleanBuffer;
747+
continue;
748+
}
749+
}
750+
}
729751
}
730752
} else {
731753
let unilateral_tap = Self::is_unilateral_tap_enabled(self.keymap, &held_key.action);
@@ -742,8 +764,7 @@ impl<'a> Keyboard<'a> {
742764
let hand1 = self.keymap.hand_at(pos1.row as usize, pos1.col as usize);
743765
let hand2 = self.keymap.hand_at(pos2.row as usize, pos2.col as usize);
744766

745-
if hand1 == hand2 && hand1 != Hand::Unknown && hand2 != Hand::Bilateral {
746-
//if same hand
767+
if hand1.is_same_side(hand2) {
747768
debug!("Unilateral tap triggered, resolve morse key as tapping");
748769
let _ = decisions.push((held_key.event.pos, HeldKeyDecision::UnilateralTap));
749770
continue;

rmk/tests/keyboard_morse_hrm_test.rs

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
/// Test cases for home row mod(HRM)
22
///
33
/// For HRM, `enable_flow_tap` and `unilateral_tap` is enabled, `prior-idle-time` will be considered.
4+
///
5+
/// Keyboard layout (1 row, 5 cols, 2 layers):
6+
/// Col: 0 1 2 3 4
7+
/// L0: [A, mt!(B, LShift), mt!(C, LGui), lt!(1, D), mt!(E, LAlt)]
8+
/// L1: [Kp1, Kp2, Kp3, Kp4, Kp5]
9+
///
10+
/// Hand config: [Left, Left, Right, Right, Right]
411
pub mod common;
512

613
use embassy_time::Duration;
@@ -27,8 +34,8 @@ fn create_hrm_keyboard() -> Keyboard<'static> {
2734
default_profile: MorseProfile::new(
2835
Some(true),
2936
Some(MorseMode::PermissiveHold),
30-
Some(250u16),
31-
Some(250u16),
37+
Some(250),
38+
Some(250),
3239
),
3340
..Default::default()
3441
},
@@ -66,8 +73,8 @@ fn create_hrm_keyboard_with_combo() -> Keyboard<'static> {
6673
default_profile: MorseProfile::new(
6774
Some(true),
6875
Some(MorseMode::PermissiveHold),
69-
Some(250u16),
70-
Some(250u16),
76+
Some(250),
77+
Some(250),
7178
),
7279
..Default::default()
7380
},
@@ -1758,3 +1765,47 @@ fn test_release_morse_keeps_pressed_layer_transparent_action_after_layer_off_hol
17581765
]
17591766
};
17601767
}
1768+
1769+
fn create_normal_unilateral_keyboard() -> Keyboard<'static> {
1770+
let hand = [[Hand::Left, Hand::Left, Hand::Right, Hand::Right, Hand::Right]];
1771+
create_morse_keyboard(
1772+
BehaviorConfig {
1773+
morse: MorsesConfig {
1774+
enable_flow_tap: false,
1775+
default_profile: MorseProfile::new(
1776+
Some(true), // unilateral_tap enabled
1777+
Some(MorseMode::Normal), // Normal (timeout-only) hold
1778+
Some(250),
1779+
Some(250),
1780+
),
1781+
..Default::default()
1782+
},
1783+
..Default::default()
1784+
},
1785+
hand,
1786+
)
1787+
}
1788+
1789+
/// Same-hand roll in Normal mode: mt!(B, LShift) (col 1, Left) then A (col 0, Left).
1790+
/// The HRM tap must fire BEFORE the plain key so the roll comes out in the pressed order.
1791+
/// Previously, Normal mode + unilateral_tap only resolved on key-release, causing the
1792+
/// plain key to fire first (wrong order).
1793+
#[test]
1794+
fn test_normal_mode_same_hand_roll_order() {
1795+
key_sequence_test! {
1796+
keyboard: create_normal_unilateral_keyboard(),
1797+
sequence: [
1798+
[0, 1, true, 10], // Press mt!(B, LShift) — HRM, Left hand
1799+
[0, 0, true, 10], // Press A — plain key, Left hand (same-hand roll)
1800+
[0, 0, false, 10], // Release A
1801+
[0, 1, false, 10], // Release mt!(B, LShift)
1802+
],
1803+
expected_reports: [
1804+
[0, [kc_to_u8!(B), 0, 0, 0, 0, 0]], // B fires first (unilateral tap on press)
1805+
[0, [kc_to_u8!(B), kc_to_u8!(A), 0, 0, 0, 0]], // A fires after
1806+
[0, [kc_to_u8!(B), 0, 0, 0, 0, 0]], // A released
1807+
[0, [0, 0, 0, 0, 0, 0]], // B released
1808+
]
1809+
};
1810+
}
1811+

0 commit comments

Comments
 (0)