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

Add toggle macro #427

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add toggle macro #427

wants to merge 2 commits into from

Conversation

sezanzeb
Copy link
Owner

@sezanzeb sezanzeb commented Jul 9, 2022

as suggested in #367 (comment)

closes #198

toggle(KEY_A)

utilizes the SharedDict object to remember if a key was held down

@sezanzeb
Copy link
Owner Author

sezanzeb commented Jul 9, 2022

@jonasBoss if you think my smaller PRs should be reviewed as well feel free to tell me, then I'll add you as a reviewer in the future. Otherwise I'll just keep merging them if I think they are fine, and only ask you to look at things if I'm not sure about something. It would definitely make sense once beta is merged anyway

@nullbyto
Copy link

nullbyto commented Jul 9, 2022

Hello, i just tried it as you asked #367. It works as it should but i found a problem when toggling, the toggle stays even after stopping the injection. For example:
I pressed a macro once with toggle(Super_L) and stopped injecting, the Super_L key stays pressed as it is still toggled until i apply and then toggle it back.

@nullbyto
Copy link

nullbyto commented Jul 9, 2022

One more thing to add to that, i'm not sure if that is related to the toggle function, but i think its more related of how the program works.
I toggled for example VoidSymbol and i have an already defined an entry as: VoidSymbol + j to call the function hold(KEY_L).
So when toggling VoidSymbol, what i expect it do is when pressing j now alone it should call that macro with hold(KEY_L), because VoidSymbol is been pressed.

I tried that with other combinations other than VoidSymbol like Control_L it doesn't recognize it as the same key in the combinations in the entries as i think the program doesn't really catch that before it gets sent to the OS from the macro.

Is it possible to manage to get that result?

@sezanzeb
Copy link
Owner Author

sezanzeb commented Jul 9, 2022

Thanks for testing!

when toggling, the toggle stays even after stopping the injection

expected. the toggle macro sends a key-down event and then stops. If you stop the injection the macro won't have a chance of injecting a key-up event. Do you think this is a huge problem?

So when toggling VoidSymbol, what i expect it do is when pressing j now alone it should call that macro with hold(KEY_L), because VoidSymbol is been pressed.

but i think its more related of how the program works.

correct

I tried that with other combinations other than VoidSymbol like Control_L it doesn't recognize it as the same key in the combinations in the entries as i think the program doesn't really catch that before it gets sent to the OS from the macro.

I didn't understand that, sorry

@nullbyto
Copy link

nullbyto commented Jul 9, 2022

expected. the toggle macro sends a key-down event and then stops. If you stop the injection the macro won't have a chance of injecting a key-up event. Do you think this is a huge problem?

Yes, in my case what happened is i had Control_L toggled and then stopped injecting then deleted the toggle entry then closed the app and couldn't do anything because i couldnt type, to toggle that back, until i restarted the pc.

I didn't understand that, sorry

I have some entries set to the arrows like so: (voidsymbol is my capslock which is a disabled capslock from KDE Plasma)

VoidSymbol + i -> `hold(KEY_UP)`
VoidSymbol + j -> `hold(KEY_LEFT)`
VoidSymbol + k -> `hold(KEY_DOWN)`
VoidSymbol + l -> `hold(KEY_RIGHT)`

I wanted to toggle(VoidSymbol) with any other combination to hold it so i can just press i, j, k, l as arrows without holding my CapsLock(VoidSymbol) until i toggle it back.

But it doesn't work as i expected.

@jonasBoss
Copy link
Collaborator

@sezanzeb It is your call how you think the code should be reviewed. Especially the macro system is almost untouched by my changes. so you won't break anything on the beta branch. If you think that your changes might impact the work I made on beta, I am happy to review them.

That said, I think we can change macros such that it will release any toggled keys as soon as the injection stops (on the beta branch). Currently each mapping handler has a method reset() which is called just before the injector event loop stops. If we implement such a method also on the Macro class we can safely reset all macros as well.

This is the current implementation of reset for the MacroHandler, which works for most macros but I don't think it will work for the toggle macro:

class MacroHandler(MappingHandler):
    def reset(self) -> None:
        self._active = False
        if self._macro.is_holding():
            self._macro.release_trigger()

It would be much cleaner, and we can guarantee it works if we implement the reset function directly on the Macro

Comment on lines +335 to +348
async def task(handler):
key = f"_toggle_{symbol}"

resolved_symbol = _resolve(symbol, [str])
code = _type_check_symbol(resolved_symbol)

if macro_variables[key]:
macro_variables[key] = False
handler(EV_KEY, code, 0)
await self._keycode_pause()
else:
macro_variables[key] = True
handler(EV_KEY, code, 1)
await self._keycode_pause()
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can avoid using the global variable dict and all the overhead, and the potential name conflict with user variables:

Suggested change
async def task(handler):
key = f"_toggle_{symbol}"
resolved_symbol = _resolve(symbol, [str])
code = _type_check_symbol(resolved_symbol)
if macro_variables[key]:
macro_variables[key] = False
handler(EV_KEY, code, 0)
await self._keycode_pause()
else:
macro_variables[key] = True
handler(EV_KEY, code, 1)
await self._keycode_pause()
def f():
while True:
resolved_symbol = _resolve(symbol, [str])
code = _type_check_symbol(resolved_symbol)
yield EV_KEY, code, 1
resolved_symbol = _resolve(symbol, [str])
code = _type_check_symbol(resolved_symbol)
yield EV_KEY, code, 0
toggle = f()
async def task(handler):
handler(*next(toggle))
await self._keycode_pause()

I didn't test this yet but it avoids the global dict altogether and the current state is stored inside the generator, which is initialized when parsing the macro.

Copy link
Owner Author

Choose a reason for hiding this comment

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

the generator would be unique for each call to toggle, wouldn't it?

I think pressing the key twice would correctly toggle it, but toggle(KEY_A).toggle(KEY_A) would inject two key-down events as far as I can tell

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah toggle(KEY_A).toggle(KEY_A) would send two key down events. and on the second press it would send two key up events.

Copy link
Collaborator

Choose a reason for hiding this comment

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

toggle(KEY_A).toggle(KEY_A) can also be written like key_down(KEY_A).key_up(KEY_A), not sure if there are any potential other issues. But I am fine with this either way.

@sezanzeb
Copy link
Owner Author

sezanzeb commented Oct 23, 2022

this might have stalled because I wanted to introduce local-only variables to reduce the overhead of toggle and such

EDIT: and maybe because input will freeze if input-remapper is stopped while the toggle is active

@sezanzeb sezanzeb marked this pull request as draft October 23, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Give support to button holding lock
3 participants