Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Sequences (aka macros) #30

Open
wants to merge 31 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3a83060
Fixed description and updated the either dependency to 1.6
riskable Oct 15, 2020
6e5e068
Added support for Sequence Actions (aka macros) whereby multiple key …
riskable Oct 16, 2020
b56de4f
Added support for Sequence Actions (aka macros) whereby multiple key …
riskable Oct 16, 2020
7df7519
Modified per TeXitoi's instructions (as best I understood them) and a…
riskable Oct 19, 2020
d65daac
Merge branch 'master' of github.com:riskable/keyberon
riskable Oct 19, 2020
6067648
Removed some leftover commented-out cruft
riskable Oct 19, 2020
95650fd
Made some changes to make Clippy happy.
riskable Oct 19, 2020
255c4d3
Unit tests for sequences and improved tick start
riskable Oct 19, 2020
18fb401
Minor fixes to make Clippy happy
riskable Oct 19, 2020
37e6b31
Support for sequences of nearly limitless length!
riskable Oct 20, 2020
b7a77d3
Make Clippy happy
riskable Oct 20, 2020
999b9f2
Changed do_action() to make Clippy happy
riskable Oct 20, 2020
ab0bfe1
Fixed a bug where Delay() wasn't working properly
riskable Oct 20, 2020
0785151
Added the ability to cancel running sequences
riskable Oct 20, 2020
60b08b9
Fixed an edge case with CancelSequence
riskable Oct 20, 2020
0d89989
Added `SequenceEvent::Tap`
riskable Oct 23, 2020
0660d2b
CHANGELOG and more doc for custom actionmagnet:?xt=urn:btih:a6721ad2e…
TeXitoi Dec 11, 2020
f3ad3ff
Merge remote-tracking branch 'upstream/master'
riskable Dec 14, 2020
3fe71e5
Changed how sequences are processed (MUCH simpler).
riskable Dec 15, 2020
43ed3e5
Merge remote-tracking branch 'upstream/master'
riskable May 3, 2021
f93d8ec
Added SequenceEvent back to layout.rs (got removed with last merge of…
riskable May 3, 2021
f0e7d65
Merge remote-tracking branch 'upstream/master'
riskable Sep 22, 2021
a26a3b6
Merged upstream
riskable Dec 25, 2021
4604ec7
Added some Sequence test cases
riskable May 2, 2022
2585845
Merge remote-tracking branch 'upstream/master'
riskable May 2, 2022
c990d9d
Fixed Sequence tests to use the new syntax
riskable May 2, 2022
d93436d
Removed unused enum variant and modified to use a const generic va…
riskable May 2, 2022
8e2dfa9
Merged upstream
riskable Oct 4, 2022
883a2d1
Merge remote-tracking branch 'upstream/master' into sequences
riskable Oct 4, 2022
5e67102
Added SequenceEvent back to action.rs
riskable Oct 4, 2022
16be78a
Fixed duplicate HoldTapConfig lines
riskable Oct 4, 2022
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
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "keyberon"
version = "0.1.1"
authors = ["Guillaume Pinot <texitoi@texitoi.eu>", "Robin Krahl <robin.krahl@ireas.org>"]
edition = "2018"
description = "Parse command line argument by defining a struct."
description = "Pure Rust keyboard firmware."
riskable marked this conversation as resolved.
Show resolved Hide resolved
documentation = "https://docs.rs/keyberon"
repository = "https://github.com/TeXitoi/keyberon"
keywords = ["keyboard", "usb-device", "firmware"]
Expand All @@ -12,7 +12,7 @@ license = "MIT"
readme = "README.md"

[dependencies]
either = { version = "1.5", default-features = false }
either = { version = "1.6", default-features = false }
generic-array = "0.13"
embedded-hal = { version = "0.2", features = ["unproven"] }
usb-device = "0.2.0"
Expand Down
51 changes: 49 additions & 2 deletions src/action.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
//! The different actions that can be done.
//! The different actions that can be executed via any given key.

use crate::key_code::KeyCode;

/// The different types of actions we support for key macros
#[non_exhaustive]
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum SequenceEvent {
/// A keypress/keydown
Press(KeyCode),
/// Key release/keyup
Release(KeyCode),
/// Combination quick keydown followed by keyrelease
Tap(KeyCode),
/// Release all (currently) pressed keys
ReleaseAll(),
riskable marked this conversation as resolved.
Show resolved Hide resolved
}

/// The different actions that can be done.
#[non_exhaustive]
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
Expand Down Expand Up @@ -41,6 +55,13 @@ pub enum Action {
/// The tap action.
tap: &'static Action,
},
/// A sequence of KeyEvents
Sequence {
/// How long to delay between events
delay: u16, // NOTE: Currently unused
/// The sequence of KeyEvents that will be triggered
actions: &'static [SequenceEvent],
riskable marked this conversation as resolved.
Show resolved Hide resolved
},
}
impl Action {
/// Gets the layer number if the action is the `Layer` action.
Expand All @@ -55,6 +76,7 @@ impl Action {
match self {
Action::KeyCode(kc) => core::slice::from_ref(kc).iter().cloned(),
Action::MultipleKeyCodes(kcs) => kcs.iter().cloned(),
// Action::Sequence { delay, actions } => actions.iter().cloned(), // TODO (maybe unnecessary)
_ => [].iter().cloned(),
}
}
Expand All @@ -78,8 +100,33 @@ pub const fn d(layer: usize) -> Action {
Action::DefaultLayer(layer)
}

/// A shortcut to create a `Action::KeyCode`, useful to create compact
/// A shortcut to create `Action::MultipleKeyCodes`, useful to create compact
/// layout.
pub const fn m(kcs: &'static [KeyCode]) -> Action {
Action::MultipleKeyCodes(kcs)
}

/// A shortcut to create `KeyEvent::Tap`, useful to create compact
/// layout.
pub const fn tap(kc: KeyCode) -> SequenceEvent {
SequenceEvent::Tap(kc)
riskable marked this conversation as resolved.
Show resolved Hide resolved
}

/// A shortcut to create `KeyEvent::Press`, useful to create compact
/// layout.
pub const fn kp(kc: KeyCode) -> SequenceEvent {
SequenceEvent::Press(kc)
}

/// A shortcut to create `KeyEvent::Release`, useful to create compact
/// layout.
pub const fn kr(kc: KeyCode) -> SequenceEvent {
SequenceEvent::Release(kc)
}

// NOTE: This doesn't work yet:
/// A shortcut to create `KeyEvent::Release`, useful to create compact
/// layout.
pub const fn ra() -> SequenceEvent {
SequenceEvent::ReleaseAll()
}
62 changes: 61 additions & 1 deletion src/layout.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Layout management.

use crate::action::Action;
use crate::action::{Action, SequenceEvent};
use crate::key_code::KeyCode;
use arraydeque::ArrayDeque;
use heapless::consts::U64;
Expand Down Expand Up @@ -32,13 +32,18 @@ pub enum Event {
Press(u8, u8),
/// Release event with coordinates (i, j).
Release(u8, u8),
/// Press event with just a keycode (for sequences)
SequencePress(KeyCode),
/// Release event with just a keycode (for sequences)
SequenceRelease(KeyCode),
Copy link
Owner

Choose a reason for hiding this comment

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

You can't modify this enum. It's the public contract of events entering a layout. This enum is not non_exhaustive, and thus adding variants is a breaking change.

Choose a reason for hiding this comment

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

are breaking changes bad for keyboard firmware - I'm build and flash it all in one go so from my side I wouldn't be affected by a breaking change? - maybe we should break now, up the semver and make it non exhaustive at the same time?

Copy link
Owner

Choose a reason for hiding this comment

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

Breaking change will say that you don't to just do cargo update to update keyberon. I don't care of making breaking changes, but it must add something to the user.

But here, the problem is more that implementation details leak in the public interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gilescope I think TeXitoi was just making a point (in a nice, technical way) that how I implemented this particular feature sucks, haha. He's right though! I've got a much better solution that I'm currently working on that should allow us to incorporate specific delays between keystrokes instead of just one universal delay that's the same for all Press() and Release() events (in a given macro).

I'm still new to Rust and his wisdom is appreciated 😄 . I'm currently struggling with writing code (a map(), filter() or filiter_map()) that figures out which keycodes need to remain down during the execution of any given sequence. It's not so simple because if you have a Press() but no corresponding Release() until the end of the sequence you need to A) figure out where in the sequence you are and B) cancel out matching pairs of Press() and Release() events but only if they're within the same window of the sequence (that you're currently at).

I actually got the code working to execute simple sequences but it currently doesn't work with modifiers or anything that needs to remain held during the sequence. Hopefully not much longer now!

Aside: In the process of writing this I've learned a lot about how HID keyboards work, haha. Like how you can only have a maximum of six keys down at a time--even though there's no real technical reason behind this limitation. It's just that when folks were defining the USB HID spec they didn't say how many keycodes could be sent at once so vendors (e.g. Apple and Microsoft) limited it to six simultaneous keycodes in their implementations (or at least, that's how it was explained to me). No idea what happens if you try to send more but it's in my TODO list of things to play with =)

Copy link
Owner

Choose a reason for hiding this comment

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

The "usb keyboard boot protocol" has this 6 keys (+8 modifiers) limitation. There is alternative USB descriptor, but the one used in keyberon has this limitation (that I didn't find limiting, so I didn't try the "NKRO" descriptor support).

}
impl Event {
/// Returns the coordinates (i, j) of the event.
pub fn coord(self) -> (u8, u8) {
match self {
Event::Press(i, j) => (i, j),
Event::Release(i, j) => (i, j),
_ => (0, 0), // Need a NaN version of this or something
}
}

Expand All @@ -63,6 +68,13 @@ impl Event {
let (i, j) = f(i, j);
Event::Release(i, j)
}
// Not really sure what (if anything) this needs to be. Just returning as-is
Event::SequencePress(k) => {
Event::SequencePress(k)
}
Event::SequenceRelease(k) => {
Event::SequenceRelease(k)
}
}
}
}
Expand All @@ -71,11 +83,13 @@ impl Event {
enum State {
NormalKey { keycode: KeyCode, coord: (u8, u8) },
LayerModifier { value: usize, coord: (u8, u8) },
SequenceKey { keycode: KeyCode },
riskable marked this conversation as resolved.
Show resolved Hide resolved
}
impl State {
fn keycode(&self) -> Option<KeyCode> {
match self {
NormalKey { keycode, .. } => Some(*keycode),
SequenceKey { keycode } => Some(*keycode),
_ => None,
}
}
Expand All @@ -90,6 +104,12 @@ impl State {
_ => Some(*self),
}
}
fn seq_release(&self, k: KeyCode) -> Option<Self> {
match *self {
SequenceKey { keycode, .. } if keycode == k => None,
_ => Some(*self),
}
}
fn get_layer(&self) -> Option<usize> {
match self {
LayerModifier { value, .. } => Some(*value),
Expand Down Expand Up @@ -201,6 +221,16 @@ impl Layout {
let action = self.press_as_action((i, j), self.current_layer());
self.do_action(action, (i, j), stacked.since);
}
SequenceRelease(k) => {
self.states = self
.states
.iter()
.filter_map(|s| s.seq_release(k))
.collect()
}
SequencePress(k) => {
let _ = self.states.push(SequenceKey { keycode: k });
}
}
}
/// A key event.
Expand Down Expand Up @@ -278,6 +308,36 @@ impl Layout {
self.do_action(action, coord, delay);
}
}
Sequence { delay, actions } => {
for key_event in actions {
match *key_event {
SequenceEvent::Press(keycode) => {
self.stacked.push_back(Event::SequencePress(keycode).into());
riskable marked this conversation as resolved.
Show resolved Hide resolved
}
SequenceEvent::Release(keycode) => {
self.stacked.push_back(Event::SequenceRelease(keycode).into());
}
SequenceEvent::Tap(keycode) => {
self.stacked.push_back(Event::SequencePress(keycode).into());
self.stacked.push_back(Event::SequenceRelease(keycode).into());
}
SequenceEvent::ReleaseAll() => {
// Can't figure out a good way to handle iterating over self.stacked
// ...without running into borrow checker problems.
// I basically don't know what I'm doing (yet) hehe
// TODO:
// for s in self.stacked.into_iter().collect() {
// match s.event {
// Event::SequencePress(keycode) => {
// self.stacked.push_back(Event::SequenceRelease(keycode.clone()).into());
// }
// _ => { () }
// }
// }
}
}
}
}
Layer(value) => {
let _ = self.states.push(LayerModifier { value, coord });
}
Expand Down