input-linux-rs icon indicating copy to clipboard operation
input-linux-rs copied to clipboard

Convenience high-level interface to read a single evdev event?

Open bluenote10 opened this issue 2 years ago • 1 comments

In C++ a evdev poll loop can be written relatively easily, by simply pulling events one-by-one.

input_event event{};
while (true) {
  bool event_available = read(fd, &event, sizeof(input_event)) == sizeof(input_event);
  if (event_available) {
    // ...
  }
}

Reading a single event in Rust is currently rather tedious. So far, EvdevHandle.read requires a pre-initialized buffer. Initializing this buffer is not that straightforward, because input_linux::sys::input_event doesn't seem to implement Default, so it has to be initialized explicitly. Also, when pulling events one-by-one the returned usize is not super convenient, because one is interested in a single event anyway. There are other challenges, like not treating the WouldBlock error as an error, and there seems to be an unnecessary clone required when converting from input_linux::sys::input_event => InputEvent => Event.

It took me a while to figure it all out. What about offering something like the following in the library?

fn read_event<F>(evdev_handle: &EvdevHandle<F>) -> io::Result<Option<Event>>
where
    F: AsRawFd,
{
    let mut events = [input_linux::sys::input_event {
        time: input_linux::sys::timeval {
            tv_sec: 0,
            tv_usec: 0,
        },
        type_: 0,
        code: 0,
        value: 0,
    }];
    let result = evdev_handle.read(&mut events);
    match result {
        Ok(count) => {
            // We should have read exactly 1 event here.
            assert_eq!(count, 1);
            Ok(Some(
                Event::new(InputEvent::from_raw(&events[0]).unwrap().clone()).unwrap(),
            ))
        }
        // The 'WouldBlock' error is entirely expected when polling a file description in O_NONBLOCK
        // mode, so it makes more sense to map it to `None` (no event available).
        Err(err) if err.kind() == ErrorKind::WouldBlock => Ok(None),
        Err(err) => Err(err),
    }
}

bluenote10 avatar Oct 22 '23 11:10 bluenote10

So there are a few distinct points of discussion here:

  1. The sys type is just an alias of libc::input_event, which prevents the crate from impl'ing Default or similar. Perhaps something like InputEvent::zeroed().into_raw() would be the most convenient way to do this.
  2. A InputEvent::with_raw constructor would be convenient too, to avoid the explicit .clone(). Matching TryFrom sys impls also seem to be missing.
  3. A higher-level read might also make sense, something that looks roughly like... fn read(&EvdevHandle, events: &mut [MaybeUninit<InputEvent>]) -> io::Result<&mut [InputEvent]> Not really a "convenience" method, but it would provide a way to work with InputEvent directly rather than need to mess with the sys types at all. Though you also potentially lose multiple events if only a single one fails to parse...
  4. The suggested helper function seems useful, however I'm not sure whether it should catch EWOULDBLOCK, it may be more consistent for that to be the caller's responsibility...
    1. That implementation would return Result<Event> if not for catching EWOULDBLOCK, so if someone does set the fd to non-blocking, they really should just match over the result or read_event().map(Some).or_else(|e| if e.kind == WouldBlock { Ok(None) } else { Err(e) }) themselves.
    2. I don't recall, is Ok(0) not a possible result in any reasonable scenario (for example does UI_DEV_DESTROY result in eof or EIO or what)? If so, would it be mapped to None or ErrorKind::UnexpectedEof or?
    3. It otherwise wouldn't work as expected if used with AsyncFd::async_io to wrap it into an async fn

arcnmx avatar Oct 25 '23 16:10 arcnmx