Added support for Sequences (aka macros)

[riskable]

Demo: https://youtu.be/M2OXrNP3fgI

Example usage:

use keyberon::action::{k, l, m, kp, kr, tap, Action, Action::*, SequenceEvent};
use keyberon::key_code::KeyCode::{self, *};

const MACROTEST: Action = Sequence {
    delay: 10,
    actions: &[
        SequenceEvent::Press(LCtrl), SequenceEvent::Press(LShift), SequenceEvent::Press(U), // Long form
        tap(Kb1), tap(F), tap(Kb4), tap(A), tap(F), // Short form
        kr(LCtrl), kr(LShift), kr(U), // Still need to get ReleaseAll() working
    ],
};

I couldn't get working SequenceEvent::ReleaseAll() (see: https://github.com/riskable/keyberon/blob/master/src/layout.rs#L324) working. Probably a trivial task for you =)

Also, I'm not sure what the purpose is of that transform() method (it's not used anywhere) so I just guessed in my own implementation, haha. I also didn't implement Action.key_codes() for Action::Sequence because I'm not sure it's necessary. I have a little commented out line in place where it'd go.

Overall it wasn't too bad and I learned a ton. Thanks for the assistance and let me know what I have to change or if it's all bad or whatever =)

[TeXitoi]

Please fmt your code.

[riskable]

All fixed I hope! I followed your instructions as best I understood them and also made Delay() work! Though to use it you have to do something like this:

const MACROTEST2: Action = Sequence {
    events: &[
        Press(A), Release(A),
        Delay {since: 0, ticks: 5000},
        Press(F), Release(F),
    ],
};

Maybe it could use a shortcut function?

[riskable]

Just want to mention #10 so this PR gets tracked in that issue.

[TeXitoi]

Please run fmt and fix clippy warnings.

[riskable]

I made the changes necessary to make Clippy happy but they weren't to my code... That's why I left them alone previously (because I didn't want to mess up your stuff--in case you intentionally wanted the code to be as it was).

Anyways, all set!

[riskable]

I have decided to become the hero we all deserve! Just pushed something awesome: Not only do we now support sequences but they can be of nearly limitless length! Muwahahaha!

Clippy is still complaining about a "never loop" and I tried a whole bunch of ways to fix it and none of them worked. It's annoying the heck out of me right now.

I've been pestering various Rust chats asking for help but I'm getting a surprising lack of answers.

Tried this but the unit tests fail:

let chunk_length = self.sequenced.capacity() - 1; // -1: Keep a slot for Continue()
                for key_event in events.chunks(chunk_length).next() {
                    match *key_event {
                        [SequenceEvent::Press(keycode)] => {
                            self.sequenced.push_back(SequenceEvent::Press(keycode));
                        }
                        [SequenceEvent::Release(keycode)] => {
                            self.sequenced.push_back(SequenceEvent::Release(keycode));
                        }
                        [SequenceEvent::Delay { since, ticks }] => {
                            self.sequenced
                                .push_back(SequenceEvent::Delay { since, ticks });
                        }
                        _ => {} // Should never reach a continue here
                    }
                }
                // Add a continuation
                self.sequenced.push_back(SequenceEvent::Continue {
                    index: 0,
                    events,
                });

[TeXitoi]

You still have a lot of not fmted code.

For the clippy warning,

for e in iter {
    // something
    break;
}

is equivalent to

if let Some(e) = iter.next() {
    // something
}

[riskable]

Those two are not equivalents. I get the same error as I get when I changed it to for key_event in events.chunks(chunk_length).next(): I'm still trying to figure it out.

Also, I did run rustfmt on the code. What do you see that isn't formatted?

[riskable]

Aha! Thanks to some help in Discord I've successfully made Clippy happy. I was so confused on that one, haha.

I also re-ran both the rust-analyzer fmt and rustfmt on the code and as far as I can tell the only thing it changed is the rustfmt wants a space after elements in the unit test arrays (e.g. [LCtrl, U] instead of [LCtrl,U] (which rust-analyzer's fmt function doesn't care about).

[riskable]

In my testing I accidentally added a sequence with a ridiculously long delay (two extra 0s haha) and it occurred to me that there's no mechanism to cancel a running sequence without hitting the reset switch. To solve that problem I just added a CancelSequence action (which can be bound to a key like anything else) and SequenceEvent::Complete which can be used instead of several Release() actions. Complete is basically the same thing as the ReleaseAll() concept we discussed previously except it will work on any type of SequenceEvent in case we support stuff like emulated mouse/joystick events in the future.

[riskable]

Anything else you need me to change? The code is passing all tests and has had rustfmt run. I'll change whatever else you want just need some direction :+1:

[TeXitoi]

Sorry, not so much time to pause and carefully read this PR. Hopefully soon, I will not forget.

[riskable]

I know you said, "Let's keep it simple first, and only support Press and Release" but I added SequenceEvent::Tap just now because I was entering a large-ish sequence and it was annoying me to have to type out Press() and Release() over and over again for what could be a simple Tap().

Adding support for Tap() only required six lines so hopefully that passes the, "let's keep it simple" requirement :smile:

[riskable]

A (very) gentle reminder to review this PR (really, there's no rush just excitement!).

BTW: I'm working on another PR for Keyberon that adds TryFrom<u8> and TryFrom<char> support (and unit tests!) to key_codes::KeyCode. It's (my idea of) the first step in making it possible to convert a long string like, "The quick brown fox jumped over the lazy dog." to a Sequence of Tap() events. I've got all the alphas/numbers handled via a macro (instead of an enormous match block) and I'm slowly working my way through the other edge cases by calculating the enum distance from ASCII's codes (http://www.asciitable.com/).

[TeXitoi]

You don't want to do these transformations at runtime. It should be more for a crate of macro/procmacro with useful helpers.

[TeXitoi]

It will also not work for non american keyboard users as is.

[riskable]

If we can't do them at runtime how can we support updating the keyboard configuration on-the-fly? Your firmware may not desire that capability but in mine I want users to be able to change their keymap without having to re-flash the whole firmware. That includes adding/removing sequences. I know that (updating sequences) will likely require an allocator but merely translating strings into keycodes shouldn't.

Also: KeyCode currently only supports American English... How do you plan to change it to support other languages? I assumed you were planning on doing it like QMK with aliases (or perhaps macros) available for doing the translation of language-specific strings-to-ascii-keycode. I honestly have no idea how best to do that sort of thing in Rust.

[TeXitoi]

What is "update configuration on the fly"? If you think of something like QMK via, that's basically writing something to flash, so not really "at runtime".

KeyCode are just code that is interpreted by the OS. The official naming of the HID KeyCodes are based on the american layout. That's why the name of the key codes in keyberon reflect the american layout. You have some strangety as NonUsBackslash, and that's also the official name of the keycode. If we want to provide easy names corresponding to some other layout, we can have some module with const declaration of the keycode using the corresponding layout. It would be more modular to declare them in a dedicated crate for each layout, limiting the size of keyberon, easily adding layout without modifying keyberon and limiting features in keyberon.

[riskable]

Let's say I wanted to make a temporary macro that types yes\n 5 times with a 2-second delay between repeats. I only need this for like a day (just doing some installs) so I don't want to update my whole firmware for this. I send a string like {"seq": "yes\\n", "repeat": 5, "name": "yes", "row": 1, "col": 16} over the keyboard's USB serial port. If there's no way to convert a string (or char or u8) to KeyCodes how can I pull that off?

[TeXitoi]

For such a case, maybe that's even better to have some kind of recording directly in the keyboard.

As you described, that's a very complicated feature, needing json parsing and layout modification on the fly.

[riskable]

For such a case, maybe that's even better to have some kind of recording directly in the keyboard.

Shall I get started on that then? :smile:

[TeXitoi]

Any problem with the revîew? Do you need some help to fix it?

I hope I didn't discourage you with this review :innocent:

[riskable]

You haven't discouraged me... It just took a while for the initial review that I started work on other things :smile:

I'm finishing up those other things (tease: https://imgur.com/a/IFsvBGh) and will be returning to this in the next few days. I did start work on it though... Already implemented a decrementing counter instead of using the since/ticks method :+1:

[riskable]

I'm done making my keyboard PCB and was able to get back to work on this! I implemented all of your suggested changes and added some general improvements (can handle up to four simultaneous running sequences now!). Had a hell of a time with the merge from upstream though haha. Hopefully I didn't clobber anything! Double-check that please :smile:

I've got complete unit tests for everything except multiple simultaneous sequences running at the same time because I'm not sure how to handle that (in test cases).

[riskable]

Just checking in... In case you missed Github's notification that I've submitted a new update :grin:

[TeXitoi]

Seen, just not so much time to work on this. I also have to implement tap hod, and release v0.2

[riskable]

I just merged upstream (everything's up to date again). Any chance you can take a look?

[TeXitoi]

I really want a clean public interface to merge it. Implementation details can be changed later, so OK to merge without the other proposed change.

[riskable]

OK I think I've resolved the "clean public interface" issue which was the last remaining hurdle (I think). @TeXitoi What do you think?