Key History and Macros by firngrod · Pull Request #1507 · UltimateHackingKeyboard/firmware

firngrod · 2026-03-08T20:37:19Z

Did some more key history work and exposed it in macros:
ifDoubletap has support for detecting a series of taps any number long, within reason, such as triple and so on.
Added $previousKeyId which can be used to create a repeat key, which I have seen requested.
Added $previousKeyPressTime which can be used to detect idle time before the key was pressed. I know that has also been requested.

Also did a couple of small refactors where I ran across code which I thought could be optimized.

I want to do one more small refactor in macros: Macros started using call, fork and exec should inherit more variables from their parents than currentMacroKey and keyActivationId.

I am okay with any part of this being rejected, I mostly did this for fun.

firngrod · 2026-03-08T20:42:02Z

right/src/macros/commands.c

+    if (S->ms.oneShot == 1) {
+        return true;
+    }
    if (S->ms.currentMacroKey == NULL) {
-        return S->ms.oneShot == 1;
+        return false;
+    }
+    if (S->ms.currentMacroKey->activationId != S->ms.keyActivationId) {
+        return false;
+    }
+    if (!KeyState_Active(S->ms.currentMacroKey)) {
+        return false;
    }
    if (isCurrentMacroPostponing()) {
-        bool isSameActivation = (S->ms.currentMacroKey->activationId == S->ms.keyActivationId);
-        bool keyIsActive = (KeyState_Active(S->ms.currentMacroKey) && !PostponerQuery_IsKeyReleased(S->ms.currentMacroKey));
-        return  (isSameActivation && keyIsActive) || S->ms.oneShot == 1;
-    } else {
-        bool isSameActivation = (S->ms.currentMacroKey->activationId == S->ms.keyActivationId);
-        bool keyIsActive = KeyState_Active(S->ms.currentMacroKey);
-        return (isSameActivation && keyIsActive) || S->ms.oneShot == 1;
+        return !PostponerQuery_IsKeyReleased(S->ms.currentMacroKey);
    }
+    return true;


I think this spells out the logic more, and it should also be more performant to exit as soon as any logic allows it?

Well, I think that the refactor logic is correct, but I don't think that the result is more readable.

For me, this unrolled variant feels very alien since in order to understand how it behaves in real life, I have to hold the entire function in my mind and evaluate it, while in the original, I see the return on a single line: (isSameActivation && keyIsActive) || S->ms.oneShot == 1, and if needed, I can backtrack from there to see how keyIsActive is evaluated.

I have a strong preference to not include this Macros_CurrentMacroKeyIsActive refactor.

right/src/key_history.c

mhantsch · 2026-03-08T21:15:41Z

I like both $previousKeyId and $previousKeyPressTime. Appreciate!

mhantsch · 2026-03-08T21:19:15Z

But why do they have to live in the macrostate? Wouldn't this be global info? Assume a macro runs for a longer time, and other keys are pressed while it is running. Would $previousKeyId and $previousKeyPressTime change? You are capturing them when the macro starts? Curious what makes more sense...

firngrod · 2026-03-09T06:18:06Z

They are meant to be previous key from thisKey, not last key, which would generally be thisKey. So either I expand history enough to be reasonably sure that it still has the relevant event, which, fair enough, would be 4-8 events, or I do this caching and become certain I have the info.
It may well be better to expand history and move the source of truth to history for what values this can be done for. Will find out how much history I can get for the same RAM sometime.
It's a trade-off of where to put priorities. I will wait for Karel's input before I decide either way.

mhantsch · 2026-03-09T07:17:47Z

Yes, RAM concerns I have. @kareltucek made sure I don't store macroArgs in the macro state but use a global pool across all macros. Balance for keeping memory consumption compact.

But key history size: hold shift key and type A LONGER SENTENCE ALL IN CAPITALS. How would that hold up regarding key history and a macro bound to the shift key?

I guess macro may need to cache the info at start of macro to avoid running out of 4-8 key history, e.g. setVar prevKey $previousKeyId

Not sure I like this solution.

Lock certain history items as "don't delete" because they were active when a macro started and just refer to the history index of that item to retrieve keyId and time when needed by the macro? Release the lock when macro ends?

Exiting times.

pcooke9 · 2026-03-09T07:37:04Z

Exiting times.

"Exiting times" are always exciting.

I'll show myself out...
😉

firngrod · 2026-03-09T07:38:51Z

Well it depends on how we would feel about a world in which a macro may loose access to key press information if it lives long enough. I'm thinking we don't feel too good about it, even if long-lived macros are discouraged. Sure, individual macros can mitigate by using variables, but it would have to be communicated very clearly in documentation, and going from certainty to near-certainty is a big leap.

I could make key history persist as long as a macro has reference to it, but it adds a lot of complexity and increases RAM usage in that history now realistically needs to be long enough that each macro slot can hold a reference on an individual key event and it's predecessor. Finally, in a language like C without scoped deconstruction, I do not like such patterns.

Further, I don't currently don't see the value in extending key history except having lazy fetching of some information in some macro commands/variables.

firngrod · 2026-03-09T07:48:52Z

One thing I have really appreciated about the UHK over the years, and since I started working in the codebase, is the lack of jank-by-design. Even if it limits the available options, and bugs notwithstanding, one can be reasonably sure that things work as intended, robustly, even in edge cases.

mhantsch · 2026-03-09T08:46:32Z

make key history persist as long as a macro has reference to it

That's what I did with macroArgs. Each argument in the pool stores the macro state slot index as its owner, and when the macro ends, I clean up the pool and remove the arguments owned by that macro. Memory consumption is 8 bits * pool size (32), vs. storing pointers or even complete arguments in the macro state (size * maximum number of concurrent macros (16)).

Clean up is in core.c/endMacro().

firngrod · 2026-03-09T11:10:15Z

Having thought about it some more, this is where I'm at:

Either we accept that macros may loose access to information if they live long enough, or we don't. First, let's presume that we don't accept that:

The information has to live somewhere. Having it live in macro state allows history to be only 2 long but increases macro state size. Having it live in macro state allows history to be only 2 long but increases macro state size. It can live in macro state or in history, but it has to live. As such, there is no fundamental argument RAM-wise to be made for storing it in one place or the other.

Here are the arguments against having it live in history:

Since history stores more information that macros require, and macros require their activation event, AND the one before, AND we need to plan for every macro referencing events without overlap, AND we still need at least one event for history to function for the rest of the firmware, this is the more costly option in terms of RAM.
Implementing a history which allows for frozen events adds considerable complexity. It's fun, but also the costly option in terms of complexity, and in terms of processing on every keystroke, not just within macro scope.

And the arguments for having it live in history:

We have history. We currently do not have any requests for history beyond previous key, do we?

Now, let's say we accept the possibility that history leaves a macro behind and they may lose access to data:
We can make educated guesses and cost-benefit judgements on how long of a history we will allow before we loose data in macros. We can save on RAM by not keeping everything forever, and we don't incur the cost of a complex history implementation.
There could be a clear disclaimer in the reference manual about historical data, like "Some variables and commands rely on historical data which will become unavailable a number of key presses after macro start, after which default values will be used. These are marked as volatile."
And then "ifPlaytime - Volatile, defaults to true after X presses," and "$previousKeyId - Volatile, defaults to 255 after X-1 presses," and "$previousKeyPressTime - Volatile, defaults to 0 after ...". Something like that. Still something of a paradigm shift.

I will not start making that choice, but I would be happy to implement it if it was made.

mhantsch · 2026-03-09T12:12:05Z

I realise in the comparison with macroArg there are some differences. Not every macro will have macroArg, nor will they all have the maximum number of macroArg statements, but (almost) every macro will have an activating keypress, an activation time, and a previous key. (The few exceptions are macros like $onInit...) Because every macro has this information, it is probably suited to live in macro state. As you said, it has to be stored somewhere. Just referencing a place where it is stored, and managing the lifetime of that referenced storage is probably overkill, and will not offer a significant reduction of memory.

Thanks for the conversation!

kareltucek · 2026-03-09T15:51:28Z

I am mot sure we actually need these. Afaik, the only person that ever requested these was Max, and it was for his own macro-HRM implementation, which is a usecase that is kind of not supported on purpose.

But assume we actually want them. Then keeping the info in the macro state is indeed the only reasonable choice.

I could make key history persist as long as a macro has reference to it, but it adds a lot of complexity

Indeed. Let's not go that way.

That's what I did with macroArgs.

Macro arguments are a very different case.

My bottom line: I definitely pick the macro state. Current implementation I believe.

I will have to look the code over yet though.

firngrod · 2026-03-09T16:10:43Z

Don't yet.

kareltucek

A couple change requests, but otherwise, seems solid.

kareltucek · 2026-03-09T16:02:11Z

right/src/postponer.c

 static uint32_t lastPressTime;

-#define POS(idx) ((bufferPosition + POSTPONER_BUFFER_SIZE + (idx)) % POSTPONER_BUFFER_SIZE)
+#define POS(idx) ((bufferPosition + (idx)) % POSTPONER_BUFFER_SIZE)


This will fail with negative indexes. I am not sure how much illegal they are, but would prefer to leave the original version in place.

Aha, that's why it was there! Will it fail with negative indexes though? I guess that depends on what type the compiler chooses to calculate in, but if it calculates in unsigned integers, the buffer will underflow, and since the modulo is a power of two, it will all work out.
Will put it back the way it was, of course. :)

kareltucek · 2026-03-09T16:14:45Z

right/src/macros/commands.c

+    if (S->ms.oneShot == 1) {
+        return true;
+    }
    if (S->ms.currentMacroKey == NULL) {
-        return S->ms.oneShot == 1;
+        return false;
+    }
+    if (S->ms.currentMacroKey->activationId != S->ms.keyActivationId) {
+        return false;
+    }
+    if (!KeyState_Active(S->ms.currentMacroKey)) {
+        return false;
    }
    if (isCurrentMacroPostponing()) {
-        bool isSameActivation = (S->ms.currentMacroKey->activationId == S->ms.keyActivationId);
-        bool keyIsActive = (KeyState_Active(S->ms.currentMacroKey) && !PostponerQuery_IsKeyReleased(S->ms.currentMacroKey));
-        return  (isSameActivation && keyIsActive) || S->ms.oneShot == 1;
-    } else {
-        bool isSameActivation = (S->ms.currentMacroKey->activationId == S->ms.keyActivationId);
-        bool keyIsActive = KeyState_Active(S->ms.currentMacroKey);
-        return (isSameActivation && keyIsActive) || S->ms.oneShot == 1;
+        return !PostponerQuery_IsKeyReleased(S->ms.currentMacroKey);
    }
+    return true;


Well, I think that the refactor logic is correct, but I don't think that the result is more readable.

For me, this unrolled variant feels very alien since in order to understand how it behaves in real life, I have to hold the entire function in my mind and evaluate it, while in the original, I see the return on a single line: (isSameActivation && keyIsActive) || S->ms.oneShot == 1, and if needed, I can backtrack from there to see how keyIsActive is evaluated.

I have a strong preference to not include this Macros_CurrentMacroKeyIsActive refactor.

right/src/key_history.c

kareltucek · 2026-03-09T16:24:27Z

right/src/key_history.h

+const key_press_event_t * KeyHistory_GetPreceedingPress(const key_state_t *keyState, uint8_t activationId);
+
+// Querying specific info
+uint8_t KeyHistory_GetMultitapCount(const key_state_t *keyState, uint8_t activationId);


Is there any chance that any of these two functions gets called with any but the last pressed keyState?

If yes, then I suspect that history size 2 might be insufficient.

(Just thinking out loud. I don't think it is the case.)

Currently, there is not, no. I just put the interface in for good measure. I think you mentioned it when I originally made the history.

firngrod · 2026-03-09T20:38:32Z

Funny how preferences are different. I much prefer the other factorization of Macros_CurrentMacroKeyIsActive. I can easily see what each variable means for the output, and their hierarchy. I will put it back the way it was of course, this is your house I just walked into and started moving the furniture around. :)

Also, as I mentioned somewhere, there is one more thing I want to do before this PR is done: I want child macros to inherit more state.

Also also, there has been talk of a repeat key, which can be easily implemented with $lastKeyPressId. I have also seen that concept talked about elsewhere in keyboard blogs, so I thought that since we have a concept of key history now, might as well...

mhantsch · 2026-03-09T21:28:03Z

And even for the repeat key it may make sense to have timing information since that last keypress, e.g. only trigger repeat if that previous keypress was within the last x milliseconds...

Christian Dirksen added 6 commits March 6, 2026 21:28

Key History: Implement actual key history

6f0bc5e

Macros: If not more readable, it should be faster

5beefea

Doubletap: N-tap support, theoretically, with easier logic

85d2610

Macros: ifDoubletap can now test for any number of taps

09ed5b7

Key History and Macros: Expose some previous key info in macros

020e556

Docs: Added documentation of the new variables

fee2d62

cla-sign-bot bot added the cla:signed label Mar 8, 2026

firngrod commented Mar 8, 2026

View reviewed changes

right/src/key_history.c Show resolved Hide resolved

kareltucek requested changes Mar 9, 2026

View reviewed changes

Conversation

firngrod commented Mar 8, 2026

Uh oh!

firngrod Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

kareltucek Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhantsch commented Mar 8, 2026

Uh oh!

mhantsch commented Mar 8, 2026

Uh oh!

firngrod commented Mar 9, 2026

Uh oh!

mhantsch commented Mar 9, 2026

Uh oh!

pcooke9 commented Mar 9, 2026

Uh oh!

firngrod commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

firngrod commented Mar 9, 2026

Uh oh!

mhantsch commented Mar 9, 2026

Uh oh!

firngrod commented Mar 9, 2026

Uh oh!

mhantsch commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kareltucek commented Mar 9, 2026

Uh oh!

firngrod commented Mar 9, 2026

Uh oh!

kareltucek left a comment

Choose a reason for hiding this comment

Uh oh!

kareltucek Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

firngrod Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kareltucek Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kareltucek Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

firngrod Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

firngrod commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhantsch commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

firngrod commented Mar 9, 2026 •

edited

Loading

mhantsch commented Mar 9, 2026 •

edited

Loading

firngrod Mar 9, 2026 •

edited

Loading

firngrod commented Mar 9, 2026 •

edited

Loading