Please, provide more context. (Feel free to open a draft PR if showing code helps).
Or maybe, if thatâs what you are asking:
command1
command2
If command1 postpones in its last iteration (i.e., calls postponeCurrentCycle), then command2 is guaranteed to get the control while still postponed.
Previously, because there was primary and secondary only, one could write a macro like this:
ifPrimary {
do primary stuff
}
else {
do secondary stuff
}
But with a possible neither result, that would have to become
ifPrimary {
do primary stuff
}
else ifSecondary {
do secondary stuff
}
But that only works if we know that the queue does not unwind anything between the two evaluations. Would I have to extend the postponing one cycle upon resolution of ifPrimary to ensure that the ifSecondary evaluates on the same data?
I may also look at saving the result in macro scope. Thatâs probably better.
That scenario should be safe as is. I.e., you donât need to do anything.
I am wondering if double evaluation should actually happen though. Given that we have taken two bits in the key state, I think the ideal solution would be to extend that enum by a fourth state that would indicate that the secondary role wasnât evaluated yet.
I.e., secondary role driver would figure out that it is a new resolution without having be told by the isNewResolution parameter.
Iâll investigate that route.
1 Like
I have regrettably belatedly realized that a much better implementation of the same half logic would have been to not actively trigger primary on same half action keys, but rather to disallow same half action keys from triggering secondary. Keys on the same half are ignored when it comes to secondary role resolution. The primary role then triggers on key release instead. The net effect becomes that everything acts essentially the same way, only now we can do multi-mod combos with trigger-on-press.
This change would warrant renaming the settings and variables, along the lines of allow/disallowSecondaryFromSameHalf, so I want to make it before the next release to prevent invalidating too many user macros. I will try to do it tomorrow.
Anyone have any thoughts on this?
1 Like
Sounds solid reasoning to me.
No problem with that and no hurries. I plan to make template related changes so we will have to bump major macro version anyways.
In the meantime I have realized I may have been misunderstanding the feature all along 
.
Anyways, I trust your judgment.
I may have not properly explained the feature. At least, I seem to have failed to think it all the way through.
By the way, I see a distinct lack of const declarations on function parameter pointers the codebase. Is that a conscious choice or not? I generally find that thoroughly declaring const where applicable makes code easier to debug and generally work with. One can see from the function declaration whether it intends to modify the content of a pointer, showing which functions have side effects and which are query style functions.
EDIT: Maybe I donât see a lack of const. Anyway, itâs much more important in object oriented languages.
Is it just me or is master currently in a broken state with regards to modules on the 60v1?
Please elaborate. We have merged uart support, so it is possible that some bug got through.
It (the lack of consts) definitely is there.
Feel free to fill them in wherever their absence offends you.
In practice, I use them kind of idiomatically in this codebase - in case of some types (like key_state_t), I omit them to reduce clutter, since they are written from just one place and read everywhere else.
This is a small and resource-constrained codebase, due to which the demands on code style and structure are somewhat different to what they would be in a standard large software codebase. (I am clearly biased on this so I am not able to fairly assess how much this attitude makess sense and how much it is just an excuse for bad habits. The codebase would definitely benefit from actual code reviews.)
I guess my slave scheduler refactors were not as equivalent as I thought they were 
.
Will get to it in the evening or tomorrow. Thanks for letting me know!
First I flashed right half with the latest master with the rest on firmware 15.3.1, I think. That broke things, and it seems I could only use one, maybe two modules at once, including the left half. Then I played around with flashing all modules with that last version, but that didnât fix things. For a while, my key cluster seemed completely dead, but eventually, I got everything back to a branch branched from just before the uart modules branch.
Regarding the const discussion, I definitely see why some choices between code âaccessibilityâ vs resource efficiency would be made, and I am actually very impressed with how far the code leans towards code accessibility rather than efficiency.
I mention the const thing because itâs one of those cases where you get code quality without a cost to the compiled result. It just allows the compiler to help us writer safer code. The only cost to the compiled result is that it might deter some optimizations which would be (more easily) doable without thorough const tracking.
Another thing is that the syntax for pointers especially is rather opaque and will make the code considerably harder to read in a lot of places.
Another reason Iâm not just going to start going hard on it in everything I do is that it is hard to contain. Itâs sort of an all-or-nothing thing which, once started, will make everything more tedious until itâs permeated everything. I make one function take a const pointer, and suddenly I have to modify all itâs utility functions, and then the ones they use and so on.
Ad module weirdness: should be fixed in master I hope.
I am quite puzzled by it. If you want to take a glance, then I have written written down some comments in Fix i2c message padding (broken by uart_modules - it has to be 4 alig⌠¡ UltimateHackingKeyboard/firmware@6c20e35 ¡ GitHub
I have checked that enum length, and they are compressed. (Printing the value of sizeof(event_scheduler_event_t) on uhk60 gives 1.)
Today I ran into a new issue for the first time: I have a long timeout, and I pressed a key, then pressed a secondary role key, then released the first key while holding the secondary role key, postponing the first keyâs release. Naturally, I got a bunch of unwanted OS level repeats on the first key. Now Iâm idly wondering about selectively not postponing releases whose presses are not on the queue, and what that might lead to.
I also had same thoughts at some point in the past.
One solution candidate is to just state that it is userâs problem to increase their autorepeat delay in their OS.
Another is to abandon the #1420 PR and use the âactivate primaryâ samehalf strategy.
what that might lead to
It may happen that your modifiers get released before their intended letters are typed. This means that you would have to implement a complex modifier state tracking solution in the postponer. (You canât simply exclude modifier keys from logic, because in case of macros you donât know if they are modifier keys). Still, if software uses some non-modifier keys as modifiers internally, you will still break these usecases.
It may happen that your dual role keys is released before it is resolved.
I am not resolutely rejecting it, but I am very wary and will take a lot of persuading.
I doubt it would lead to any serious issues to allow keys which were pressed before postponing started to release. Itâs not like itâs a common issue either way. Like I said, it was the first time I have seen the issue, and itâs always been like that with trigger on release.
First I will implement the features I have in mind, then I might try that, just for fun, and if it works out well, I might try to persuade you to merge it.
2 Likes