Skip to content

Initial port to async/await with smol#146

Merged
max-baz merged 12 commits into
max-baz:mainfrom
LordMZTE:smol
Jun 12, 2025
Merged

Initial port to async/await with smol#146
max-baz merged 12 commits into
max-baz:mainfrom
LordMZTE:smol

Conversation

@LordMZTE
Copy link
Copy Markdown
Contributor

@LordMZTE LordMZTE commented Jun 9, 2025

This acts an an initial port to async/await using the smol runtime. The only parts of the codebase that still use regular threads and blocking IO should be the webcam ALS and the wayland capturer, as the v4l and wayland crates don't yet have support for async IO. No sleep loops have been removed yet, but this should make it easier to do so as we've discussed in #143.

Work done

  • Replaced the threads spawned "green threads" using smol::spawn
    • The 2 places this isn't done yet use smol's thread pool using smol::unblock
  • Ported IO operations to non-blocking variants (please check if I've missed any!)
    • The config reading code hasn't been ported (yet), it shouldn't be significant as it only does a little work on startup.
  • Replaced std::mpsc channels with smol channels.
  • Converted several traits to enums. Traits with async functions are a problem, because the return type of these functions are some compiler-generated future type that's different for each implementation of the type. This causes the problem that the trait would have to be generic over this type, preventing us from Box-ing it.
    • As a consequence of this, mockall can no longer generate a mock implementation for Brightness. Tests that use mocking have been commented out for now. I think this might be best left to another PR.
  • The main function as well as tests have been given a macro to allow them to be async.
  • Several pieces of code that previously used "monadic" functions with Result/Option has been rewritten with plain logic as the closures given to these functions cannot support async code.
  • std::sync::Mutex has been replaced with smol::lock::Mutex
  • A type alias ErrorBox for boxed errors has been created as the type got a few more trait bounds now. We already transitively depend on anyhow, perhaps we should use that for error handling in the future.
  • Some clippy lints have been fixed on the way, mostly assert_eq!(true, x) -> assert!(x)
  • Maybe other stuff I've missed :D

All tests pass and wluma has been tested with the IIO sensor ALS on wayland/River.

Copy link
Copy Markdown
Owner

@max-baz max-baz left a comment

Choose a reason for hiding this comment

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

This looks very interesting and promising, leaving some initial feedback - thank you so much for working on this!

Comment thread src/als/controller.rs Outdated
Comment thread src/main.rs
Comment thread src/als/iio.rs Outdated
Comment thread src/predictor/controller/manual.rs Outdated
Comment thread src/als/mod.rs
Comment thread src/brightness/controller.rs Outdated
@LordMZTE LordMZTE requested a review from max-baz June 10, 2025 19:42
@max-baz
Copy link
Copy Markdown
Owner

max-baz commented Jun 10, 2025

@LordMZTE thanks for the latest changes! Would you be able to review some of my latest commits? It seems to compile, but I can't e.g. actually test whether iio sensors still work as expected - just to be extra sure 🤞

( ...I have a soft spot for the monadic style in Rust.... 🙈 )

Copy link
Copy Markdown
Contributor Author

@LordMZTE LordMZTE left a comment

Choose a reason for hiding this comment

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

Thanks for you work! Here are some thoughts.

I can also confirm that everything still works with an IIO sensor.

Comment thread src/als/iio.rs Outdated
Comment thread src/main.rs Outdated
Comment thread src/main.rs Outdated
@max-baz
Copy link
Copy Markdown
Owner

max-baz commented Jun 12, 2025

Thanks for the continuous feedback :) By the way, I really didn't mean to steal all the fun, you are very welcome to push improvements directly 😁 Do you think this is in a good shape to merge, are there more things you would like to do here, or in another PR?

@LordMZTE
Copy link
Copy Markdown
Contributor Author

No worries, I really appreciate the help! I can't think of anything else to do in this PR, so I'd be alright with merging. The next PR will probably be de-sleep-looping some of the controllers :D

@max-baz max-baz merged commit 18b7929 into max-baz:main Jun 12, 2025
2 checks passed
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