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

support binding actions to switches #747

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Conversation

cmeissl
Copy link
Contributor

@cmeissl cmeissl commented Oct 17, 2024

Not sure this is the right approach, but it seems to work fine so far. Though the switch bindings should probably be also run during startup.

@YaLTeR
Copy link
Owner

YaLTeR commented Oct 17, 2024

Hmm, how does it work in other compositors? Also what happens if you e.g. suspend, then switch to tablet mode, then unsuspend?

@cmeissl
Copy link
Contributor Author

cmeissl commented Oct 17, 2024

afaik sway has something similar with

# Show the virtual keyboard when tablet mode is entered.
bindswitch tablet:on busctl call --user sm.puri.OSK0 /sm/puri/OSK0 sm.puri.OSK0 SetVisible b true

though there is also the option to react on toggle. If needed I can change the PR to split the actual switch and optional state (toggle), not sure this is really needed.

Will test the suspend behavior.

@cmeissl
Copy link
Contributor Author

cmeissl commented Oct 17, 2024

Okay, so it seems I can not directly un-suspend in tablet mode, even the power button is disabled. But I can with an external keyboard. Also I can suspend in Tablet Mode and unsuspend in non Tablet Mode. In both cases the switch does not seem to trigger. But tbh I am not sure handling these cases is important. This would probably require keeping some state somewhere and query the current state after resume.

@cmeissl cmeissl force-pushed the feature/switch_binds branch 2 times, most recently from 8cc55df to 60aaef2 Compare October 17, 2024 19:09
@YaLTeR
Copy link
Owner

YaLTeR commented Oct 17, 2024

I just had a shower thought, what if instead of putting this into the binds section with its intercept and repeat and all, this just was in its own section? Maybe even only allow a spawn action? With a comment mentioning that the command may be spawned extra times like on compositor start or w/e.

@cmeissl
Copy link
Contributor Author

cmeissl commented Oct 17, 2024

I just had a shower thought, what if instead of putting this into the binds section with its intercept and repeat and all, this just was in its own section? Maybe even only allow a spawn action?

Sure, you had something more static in mind or still keep it dynamic as with the binds?
For now I also expect the spawn action to be enough, though Lid maybe could use some other action in the future?

I tested lid with something like

binds {
    Switch+LidOn { spawn "bash" "-c" "niri msg output \"eDP-1\" on"; }
    Switch+LidOff { spawn "bash" "-c" "niri msg output \"eDP-1\" off"; }
}

which also works nicely (ignoring suspend and so on for now).

With a comment mentioning that the command may be spawned extra times like on compositor start or w/e.

Sure, as long as the command can be considered idempotent I don't see a reason against just re-run it on something like un-suspend or config reloads.

@cmeissl
Copy link
Contributor Author

cmeissl commented Oct 17, 2024

(just a side-note, I am still not really happy with the virtual keyboard in tablet mode. LibreOffice Calc just seems to always request input method and I don't see a way to hide the keyboard....)

@YaLTeR
Copy link
Owner

YaLTeR commented Oct 17, 2024

I think something more static would make sense here. E.g.

switch-events {
    lid-open { spawn ... }
    tablet-mode-on { spawn ... }
}

(names and structure up for discussion)

(just a side-note, I am still not really happy with the virtual keyboard in tablet mode. LibreOffice Calc just seems to always request input method and I don't see a way to hide the keyboard....)

I think they're proposing a Wayland request to enable/disable the osk, wonder if that'll help here..

@cmeissl
Copy link
Contributor Author

cmeissl commented Oct 17, 2024

I think something more static would make sense here. E.g.

switch-events {
    lid-open { spawn ... }
    tablet-mode-on { spawn ... }
}

k, sounds good, will do that change tomorrow. maybe we want to keep some options like allow-when-locked=true?

(just a side-note, I am still not really happy with the virtual keyboard in tablet mode. LibreOffice Calc just seems to always request input method and I don't see a way to hide the keyboard....)

I think they're proposing a Wayland request to enable/disable the osk, wonder if that'll help here..

Hm, let's see how this turns out. Current state seems quite annoying :/

@YaLTeR
Copy link
Owner

YaLTeR commented Oct 17, 2024

maybe we want to keep some options like allow-when-locked=true?

Hm, I thought it would make sense to allow those when locked by default (and document it I guess).

@cmeissl cmeissl force-pushed the feature/switch_binds branch 2 times, most recently from 9d1a47a to 0dbf6ab Compare October 18, 2024 10:45
this adds a new config section named `switch-events`
that allows to bind `spawn` action to certain switch
toggles.
@cmeissl
Copy link
Contributor Author

cmeissl commented Oct 18, 2024

Okay, added the new section and implemented the logic to just always run the configured actions.

Seems we can not query the state of the switches, at least not directly through libinput, so we can not easily run the actions other then in response to receiving a libinput toggle event. But so far I have not seen issues with this, even turning
the monitor back on after resume seems to work fine. If needed turning off could be limited to multi-monitor setup with something like [ $(niri msg --json outputs | jq length) -gt 1 ] && niri msg output "eDP-1" off.

We could store the current state in a map per switch and re-run on config changes, but I am not sure this is really worth implementing?

@cmeissl cmeissl marked this pull request as ready for review October 18, 2024 11:07
Copy link
Owner

@YaLTeR YaLTeR left a comment

Choose a reason for hiding this comment

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

Looks good.

Seems we can not query the state of the switches, at least not directly through libinput, so we can not easily run the actions other then in response to receiving a libinput toggle event. But so far I have not seen issues with this, even turning
the monitor back on after resume seems to work fine.

Interesting, I see.

If needed turning off could be limited to multi-monitor setup with something like [ $(niri msg --json outputs | jq length) -gt 1 ] && niri msg output "eDP-1" off.

Ideally that should happen automatically but it's out of scope for this PR. It's nice that you can do it manually at least.

We could store the current state in a map per switch and re-run on config changes, but I am not sure this is really worth implementing?

Nah, I think it's fine as is.

@YaLTeR YaLTeR enabled auto-merge (squash) October 18, 2024 13:53
@YaLTeR YaLTeR merged commit 79fd309 into YaLTeR:main Oct 18, 2024
9 checks passed
@YaLTeR
Copy link
Owner

YaLTeR commented Oct 18, 2024

Thanks!

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.

2 participants