swhkd does not process hotkeys of newly connected devices
#162
Comments
MWE for each implementationEach exemple prints events from the 'input' subsystem as they are received. Each log is prefixed by a basic timestamp to highlight the difference between the actual physical event and the time it is passed to the application. Busy loopuse std::error::Error;
use std::time::{SystemTime, UNIX_EPOCH};
use tokio::select;
use tokio_stream::StreamExt;
use udev::MonitorBuilder;
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
let mut udev = tokio_stream::iter(MonitorBuilder::new()?.match_subsystem("input")?.listen()?);
loop {
select! {
udev_item = udev.next() => {
if udev_item.is_none() {
continue;
}
let event = udev_item.unwrap();
println!("[{:?}] '{:?}' event - devnode: {:?}",
SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(),
event.event_type(),
event.devnode());
}
}
}
}On my computer, when disconnecting and reconnecting my keyboard, this gives:
Sync loopReplace the inner loop with: select! {
Some(event) = udev.next() => {
println!("[{:?}] '{:?}' event - devnode: {:?}",
SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(),
event.event_type(),
event.devnode());
}
else => {}
}This does work in this example, but events are ignored when trying the same thing in swhkd. I think this is because the stream is ignored by tokio on the first Asyncuse std::error::Error;
use std::time::{SystemTime, UNIX_EPOCH};
use tokio::select;
use tokio_stream::StreamExt;
use tokio_udev::{AsyncMonitorSocket, MonitorBuilder};
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
let mut udev = AsyncMonitorSocket::new(MonitorBuilder::new()?.match_subsystem("input")?.listen()?)?;
loop {
select! {
Some(Ok(event)) = udev.next() => {
println!("[{:?}] '{:?}' event - devnode: {:?}",
SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(),
event.event_type(),
event.devnode());
}
}
}
}For two disconnections/reconnections, this gives me (late events in bold):
Please note that here, the input which is the actual keyboard is I do not understand why this happens. The most I was able to understand was that the stream sometimes got 'stuck' on Running |
|
I looked into solving my issue with |
|
Wow! Impressive bug report and you have my respect for putting in the time to not only investigate the issue but also to propose possible changes ❤️. |
|
Apart from that, this looks good, I'll review it in depth once I'm done with relocating! |
|
I did some digging and found out that we don't need to create a new |
Maybe? Take a look at https://wiki.ubuntu.com/X/InputConfiguration#Device_classification: A basic implementation could be: // or tokio_udev
let mut udev_scan = udev::Enumerator::new()?;
udev_scan.match_subsystem("input")?;
udev_scan.match_property("ID_INPUT_KEYBOARD", "1")?;
let keyboard_devices = udev_scan.scan_devices()?
.map(|dev| dev.devnode().map(|path| path.to_owned()))
.flatten()
.map(evdev::Device::open)
.flatten()
.collect::<Vec<_>>();This works with some basic testing. I could implement a PR for that if you'd like! To be honest, I think it would be better to fix #131 rather than detecting keyboards: if |
Is it that bad? Isn't the socket only created once anyway? What does it change to create first the builder? |
|
> > I did some digging and found out that we don't need to create a new `MonitorSocket` from `MonitorBuilder::new()?....listen()`. We can create this builder once globally and then just poll the socket. Although this too is inefficient, it's...not as bad 🤣.
>
> Is it that bad? Isn't the socket only created once anyway? What does it change to create first the builder?
>
It's probably not too different but why perform extra computation when we can just poll the socket descriptor?
|
Okay! it's cool that udev can detect these things for me, intruiging. Also I've been meaning to ask, would you like to join us as a maintainer? I've noticed that you have great debugging skills along with good problem solving skills. |
|
I have time this weekend to work on the udev problem! I'll go ahead and start on a patch asap. PS: Cleared up some unnecessary messages. |
|
I have reported this issue to tokio-udev upstream: jeandudey/tokio-udev#17 |
Shinyzenith
added
Priority: high
|
The latest updates in the This is implemented in the I will be waiting for a release of |
|
@ajanon I tested the udev_async branch and it seems that sometimes, the code seems to panic. I am yet to find the triggering cause. I'll paste the logs below: |
|
Here are the steps I found to reliably reproduce the issue. |
|
When running from the main branch swhkd doesn't give any error but it doesn't seem to accept keyboard events any longer. When running from the |
|
Is this still an issue? I wanted to look into it because of the bounty but I can't reproduce the issue with the udev_async head. I can plug and unplug my USB keyboard freely and swhkd seems to cope just fine. (Incidentally I am doing this on a setup where no commands are actually run, on any hotkey swhkd prints "failed to send command through ipc". swhkd can't find the socket created by swhkd for reasons unclear to me. But that can't be the reason for the error not happening to me, can it?) |
|
I have detailed the steps to a crash issue two comments above. Can you reproduce that? |
I can connect and disconnect the USB keyboard, and pressing super in both states, on both keyboards (obvs on the external only when connected), makes swhkd log the key. Using any combinations involving super also are logged fine (though the commands not executed, as I mentioned above). The only combination in my config is a
Am I missing something? |
|
Fixed the socket issue with commands, it was just polkit being weird like always. still can't reproduce the crash. Wow, that's some |
|
Still don't face the issue, but I came up with some theories and made some fixes to obvious problems: @Shinyzenith and others: can you test and see if these fixes work for you? |
Nice, seems to be working now. Great job! |
Sorry for the late reply. I'm more than happy to do some more testing. Yeah the |
|
I'll review the PR and get that going. |
|
I have merged the pr sent by @ConcurrentCrab. Just to be sure @CluelessTechnologist would you mind testing the latest HEAD of udev branch once more? If all is good to go, I'll merge. |
Sure. Just tested the latest commit from udev branch. Happy to report that it's still working: |
|
@CluelessTechnologist I made some minor changes and force pushed. Would you mind testing the branch head one last time? My apologies. After that it'll be ready for merge. |
Okay I pulled the latest commit and recompiled and reinstalled. Still seems to be working: |
Awesome! Happy to be of help to y'all, and thanks for the bounty! @Shinyzenith I can't collect the bounty on bountysource.com until this issue is marked as closed, so I'd really appreciate it if you could close it and test miscelleneous changes afterwards :P |
|
@CluelessTechnologist I am not sure how you wish to split the bounty but I'll list out the people who have helped us achieve the goal: @ajanon For their excellent udev async patch and reporting upstream on fixing the epoll error. |
|
@ConcurrentCrab were you able to claim the bounty? |
I thought it best not to take any actions on my side yet in case you had actually intended to split it. |
|
@Shinyzenith I released the fixes for tokio-udev but who fixed it was @sjoerdsimons, so I don't actually deserve any bounty but him |
|
Thank you for the clarification! |
Unfortunately there's no way to split it. The one who creates the PR is eligible for the bounty. So you can go ahead and claim it since you were the one that created the PR.
@sjoerdsimons here's the bounty for tokio-udev: https://app.bountysource.com/issues/121368069-events-were-blocked-while-poll-next |
To be completely fair, alexis created the initial fix, just in a branch. I opened the PR. I obviously don't want any part of the bounty, I'm just stating the facts. |
I have no need for a bounty; I'm happy for @jeandudey to pick it up as thanks for maintaining tokio-udev or for the funding to be re-used for another issue that seems worthwhile. |
|
So a kinda update regarding bountysource: I had requested a cashout of a pair of bounties I did a couple weeks ago, and have been patiently waiting for the past few weeks for it to process, which I found weird as their FAQ claimed it should be a week at max. So I tried to look up some stuff and I kinda got my answer: https://github.com/bountysource/core/issues So it goes without saying: please stop giving more money to bountysource, or displaying their services on your projects. I think it;s pretty safe to say they're not going to pay any of it out now, even if they even exist anymore on any level. As for money that already exists on their platform, I saw somebody suggest in the issues asking paypal to chargeback any transactions that have happened less than 6 months ago; since bountysource is unresponsive on all levels, paypal should automatically resolve in favour of customers. @CluelessTechnologist Of course, That is what I'd recommend you do for the bounties you've posted for your issues as well. And, if I may ask this favour of you, I would also request you to paypal me for this bounty (only once the chargeback claim is successful, of course), because I sort of did actually need the pocket money. This would also allow you to split it with @ajanon as they wish. |
|
Sadly I was warned about BountySource before but there isn't that many sites that offer bounties with escrow. Rysolv is one that I have used before. I was debating switching to it but decided against it which was my mistake I guess. I'm really sorry about this @ConcurrentCrab. Please send me a DM with your PayPal email on Matrix (@CluelessTechnologist:matrix.org) or email me. (email in my GitHub profile) Another option is to enable Github Sponsors on your profile. |
|
@CluelessTechnologist I sent you an email (from the same address as my commits). |
|
Very unfortunate situation. I hope bountysource returns you your capital @CluelessTechnologist. |
Version Information:
swhkd-gitAUR package)Describe the bug:
swhkddoes not register hotkeys from a newly connected keyboard (or disconnected then reconnected).Expected behavior:
swhkdshould be able to register hotkeys on a newly connected keyboard.Actual behavior:
swhkddoes not grab newly connected devices and thus cannot handle their keypresses events.To Reproduce:
Start
swhkd. Connect a keyboard (or disconnect yours before reconnecting it). Hotkeys will not work.Additional information:
This can probably be solved using
udev, with either theudevcrate ortokio-udevcrate.My attempts at implementing this (the same content is also in the [wip] commits of each branch):
tokio-udevcrate (udev_asyncbranch):udevcrate iterators wrapped in a tokio stream (udev_syncbranch):udevcrate with a busy loop (udev_sync_busybranch):The busy loop is the only one which is working, but also the worst performance-wise. See this one more as a proof of concept.
The text was updated successfully, but these errors were encountered: