kareltucek / firmware

This is fork of UHK's firmware featuring extended macro engine.
Other
82 stars 9 forks source link

Time logic problem with secondary resolution in postponed macros #37

Closed orthoceros closed 4 years ago

orthoceros commented 5 years ago

I have encountered a time logic problem with postponed macro execution that I cannot solve. It occurs with both resolveSecondary and the 1-key virtual mod idea from the other thread. I use the latter below, as it makes the problem more clear in macro code:

I use the following macro on the ; key and the sequence ;->j maps to QWY.mod.j:=leftArrow, which works fine if this macro is executed in real-time:

$postponeKeys ifNotReleased ifNotPending 1 goTo @0
$postponeKeys ifPending 1 delayUntil 100
$ifNotReleased ifPending 1 final holdKeymapLayer QWY mod
$holdKey ;

I use another macro on the ' key to eleminate the default dead key behavior of the US international keyboard layout, as I do not need it on this key:

$postponeKeys tapKey '
$postponeKeys delayUntil 250 //necessary against lost quotes in Ubuntu HyperV-VM.
$postponeKeys tapKey space

In coding, I often enter '' or "" first, followed by leftArrow, before I write the actual string contents. If I now enter the sequence '->'->;->j fast enough, I do not end up with the intended '|', where | indicates cursor position, but I get '';j| instead. More precisely, this happens if I release the ; key before the line $ifNotReleased ifPending 1 final holdKeymapLayer QWY mod is executed. This problem can be made easily reproducible by using delayUntil 1000 instead of delayUntil 250.

The cause is that the execution of the macro on ; is postponed and when finally playing it back, ; has already been released. From a time logic point of view, what I really want to test for secondary resolution is that ; was released at least 100ms after the press event of j (independent of for how long these events were postponed). How could this be achieved if the macro is not executed in real-time, but postponed?

kareltucek commented 5 years ago

How could this be achieved if the macro is not executed in real-time, but postponed?

Well, you would have to extend the postponer to store times too, and either make it replay keys in "real time", or re-implement again all the needed features so that they could lookup these state conditions in the postponer's queue.

orthoceros commented 5 years ago

Sounds tedious... I hoped that the firmware would already internally keep two time point arrays for postponed keys: One for the press times and one for the release times.

All current ifs in the grammar could be left untouched (their logic is not wrong if you interpret them as ifReleasedNow at execution time).

With the two time point arrays, we could replace the critical line $ifNotReleased final holdKeymapLayer QWY mod above by something like the following (using local variable names instead of register numbers for legibility):

$postponeKeys setReg tTriggerRelease timeOf releaseOf 0              //the macro trigger key `;` is at queue index 0 now; when was it released?
$postponeKeys subReg tTriggerRelease 100                             //shift the release of the trigger key 100ms backwards in time
$postponeKeys setReg tActionPress    timeOf pressOf 1                //the action key `j` is at index 1 now; when was it pressed?
$ifReg tActionPress < tTriggerRelease final holdKeymapLayer QWY mod  //was the action key pressed before the (safety-shifted) trigger release?

Release time points of not-yet-occurred releases should return +Inf or intMax.

PS: I'm still at and happy with snappy 100ms time windows for 1-key virtual mods on a for mousing and on ; for arrow keys. :) I can provoke problems on purpose, but in practice I seem to always release keys fast enough during normal typing, i.e. I had no unwanted typing interference (except for the unrelated problem of this thread). I had no time yet to further test editing layer shortcuts/gestures on other alphabetic keys, though. I can only confirm that the 2-key virtual mods on c and v never caused any typing interference, either. But maybe I will give the new and faster 1-key mod logic another chance also for these editing purposes (later this month...).

kareltucek commented 5 years ago

Sounds tedious... I hoped that the firmware would already internally keep two time point arrays for postponed keys: One for the press times and one for the release times.

Actually, there is just one queue - for both presses and releases. It makes sense, because when you are replaying keys, you need to do that in order. Of course, it complicates some other queries which would be simpler in the two-array format... but you can't have a cake and eat it too. Times are not stored at all, since replay does not preserve delays between events - just replays them. Therefore, it did not make sense to waste memory on it.

If we actually wanted to support time-aware postponing, some way of distributing the event times would have to be written too, since currently there's no way to even pair an activation of a macro to the corresponding time event.

I am now thinking about creating an "alternative time measure" which would "globally" contain postponed "current" time, as an alternative to CurrentTime. It would most likely contain activation time of next event in the queue or (if there was nothing in the queue) actual current time. It might be an elegant and relatively simple-to-implement way of solving this problem... However, it might also introduce new bugs into the code, and would imply that a lot of code would have to be revised carefully.

orthoceros commented 5 years ago

If I understand your solution correctly, it would be able to playback postponed key events also during a delayUntil millis line in a postponed macro? I.e. it would be completely transparent to macros, as if the macro was always executed in real time and never postponed? This would be elegant, indeed! :)

Regarding the RAM footprint: As only relative times are ever needed (and not absolute microseconds since 1970), a uint16 should suffice for event time stamps in milliseconds (this gives you about 65 seconds of resolution for relative event times; more than enough). So, if the key event would contain two additional fields for press and release times, it would need 4 bytes of additional RAM. How deep is the postponer queue? I would say, postponing more than 10 key events will practically never happen. So, 4*10=40 bytes. I guess some of my macro comment lines have already caused higher RAM peaks... ;) So, assuming that the key events are some kind of structure, couldn't this structure be extended by the two time stamp fields to make them available everywhere in the existing code that has already access to queued key events, without having to change it?

[If you really need to optimize for every last byte of RAM usage, or if much more key events have to be stored for some reason, you could introduce a special number convention like 65535=+Inf, 65534=NaN and 65533=-Inf (and then already overflow/mod back to 0 millis on 65532). Then you could save RAM for all probably currently existing fields to store released states etc. by replacing them with pre-processor macros like isReleased(keyEvent):=keyEvent.releaseTime<specialNumbers.plusInf. But I would only recommend this bug-attracting special number hassle if it was really necessary due to hardware/RAM limits...]

kareltucek commented 5 years ago

If I understand your solution correctly, it would be able to playback postponed key events also during a delayUntil millis line in a postponed macro? I.e. it would be completely transparent to macros, as if the macro was always executed in real time and never postponed? This would be elegant, indeed! :)

Well, that's the idea... Unfortunately, it is not as simple as that.

E.g., if multiple delay actions of some macro fit within single such event, the counting would fail and only the first delay would get processed. There would have to be some system counting "consumption" of the time. That raises more problems - for instance, active wait loops...

I am afraid that attempting this would blow up the codebase. The code base actually could make use of a big refactor/rewrite of the macro-logic control structures, which would implement this and a whole bunch of other things (such as multiline macro actions, scoping, "synchronous" runtime and more), but that would took a lot of time.

Regarding the RAM footprint:

Indeed RAM is not a problem here.

So, assuming that the key events are some kind of structure, couldn't this structure be extended by the two time stamp fields to make them available everywhere in the existing code that has already access to queued key events, without having to change it?

Well, you view entire life-cycle of a keypress as a single and well organized structure. However it is not the case. From the view of the firmware, the lifetime consists of four independent phases, which are represented by four different code areas - debouncing phase, postponer's phase, key-activation phase and macro-processing phase. There is no structure which would carry state information throughout the entire lifecycle and therefore you cannot simply do that so that it "makes them available everywhere in the existing code".


Factual nitpicking:

GrauLab commented 4 years ago

Just as a quick info for others who might also want to use anti-dead-key macros (e.g. to counter some dead keys on OS level for the international US QWERTY keyboard layout):

The reason it did not work reliably without a time delay before was that the OS expects a vanilla tapKey space to release the dead key. A shift-space won't work, for example. But in the macro, tapKey reproduces pressed mods, of course... So essentially, only a suppressMods was missing above. My earlier line $postponeKeys delayUntil 250 //necessary against lost quotes in Ubuntu HyperV-VM is superfluous now. While this does not solve the macro timing logic problem, at least I do not have/see any other use case where this might be an issue...

For completeness, here is a ready-to-use version of my current anti-dead-key macro that also maintains OS key repetition behavior:

$postponeKeys holdKey `
$postponeKeys delayUntilRelease //allow for OS key repetition.
$postponeKeys ifNotInterrupted suppressMods tapKey space //anti-dead key (OS requires one vanilla tapSpace => use suppressMods; only applicable, if the user does not mean something else by pressing some combo before trigger release => use ifNotInterrupted.)