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

Auto-reload on config change #5

Open
maurges opened this issue Jun 19, 2022 · 8 comments
Open

Auto-reload on config change #5

maurges opened this issue Jun 19, 2022 · 8 comments

Comments

@maurges
Copy link
Owner

maurges commented Jun 19, 2022

Deleted original message, since I wrote it to myself and to an outside observer it might have sounded passive-aggressive.

It would be nice to automatically reload changed config. My first thought to do it was like in haskell: have an observer thread and an executor thread, and upon detecting changes the observer kills and restarts the executor. The problem is that without haskell runtime this needs to be done with forks and linux signals, which is not great and sort of unsafe.

My second thought was a canonical unrolling of async exceptions into flags. The executor thread is a pipeline from an endless gesture producer. So if it stops producing upon a boolean flag, the thread dies, which is easy. The boolean doesn't even need to be atomic or correctly read-write ordered (unsafe in rust, but safe in reality). The problem with that is that it's kind of spaghetti, and the fact that the pipeline is badly documented. It's spaghetti and undocumented because I was thinking of haskell-conduit when I wrote it and it made a lot of sense in my head. Do I need to refactor it?

@spikespaz
Copy link

I don't think a daemon would be necessary here. I like that you avoided using the server/client model that touchegg uses. Perhaps we should introduce an asynchronous executor and use a ready-made file watcher crate? I don't think it would be too complicated; but I expect to eat my words.

@maurges
Copy link
Owner Author

maurges commented Jun 19, 2022

It is a simple thing to do (like four lines), but a hard thing to do so that I won't hate the code

@spikespaz
Copy link

Four lines in haskell, but this is Rust. I find that code you linked hard to read, I don't like the look of haskell. Would you mind explaining how it works? How can a file watcher work just slapped in the middle of a synchronous program like that?

I would think something like this would have to go through a few stages:

  1. Parse command line
  2. Check or config changes, load config
  3. Watch input events
  4. Process events according to config
  5. Send key events to device accordingly
  6. Back to stage 2

Unless my understanding is flawed.

@NICHOLAS85
Copy link

If you end up creating a user unit for this program it's pretty easy to add the auto reloading feature using systemd.

Here's an example:
https://github.com/NICHOLAS85/gebaar-libinput/tree/orientationawareness/assets

gebaard.service auto starts the daemon
gebaard-watcher.path watches for changes at the configuration location and triggers the following unit
gebaard-watcher.service restarts gebaard.service when the unit above triggers it

@spikespaz
Copy link

spikespaz commented Jun 19, 2022

Considering that Wzmach is supposed to work when started as a user, do you want to tie the auto-reloading functionality to systemd? It is an option, but it belies @d86leader's effort to lead by example when it comes to not running things as root.

@maurges
Copy link
Owner Author

maurges commented Jun 19, 2022

@spikespaz

  1. Check or config changes

It's undesirable to check for config every time. It's better to just use man 7 inotify and only reload config on the event itself. I also don't want the config to be behind a mutex, so upon re-reading the config the whole gesture-processing has to restart.

Well, I think the systemd way is ok. And it can be replaced in the future easily. So I will include it with the systemd unit files, until I write a proper restarting.

@spikespaz
Copy link

I was thinking of something like this: https://lib.rs/crates/notify

@spikespaz
Copy link

Hence why I talked about using an async executor. When I said "check" I really meant "respond to notification".

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

No branches or pull requests

3 participants