Skip to content

Initial port to async/await with smol #146

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

Merged
merged 12 commits into from
Jun 12, 2025
Merged

Initial port to async/await with smol #146

merged 12 commits into from
Jun 12, 2025

Conversation

LordMZTE
Copy link
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
Owner

@maximbaz maximbaz 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!

@LordMZTE LordMZTE requested a review from maximbaz June 10, 2025 19:42
@maximbaz
Copy link
Owner

@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
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.

@maximbaz
Copy link
Owner

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
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

@maximbaz maximbaz merged commit 18b7929 into maximbaz: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