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

Added asynchronous pwm functionality #25

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

lucavezoc
Copy link
Contributor

No description provided.

@lucavezoc lucavezoc requested a review from a team as a code owner January 5, 2024 20:06
Cargo.toml Outdated
@@ -19,3 +20,4 @@ See https://www.kernel.org/doc/Documentation/pwm.txt
readme = "README.md"

[dependencies]
tokio = { version = "1", features = ["full"] }
Copy link
Member

Choose a reason for hiding this comment

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

Using the full feature set is an antipattern in libraries per the tokio docs. Could you please reduce this to just the needed features?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Member

@nastevens nastevens left a comment

Choose a reason for hiding this comment

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

@rust-embedded/embedded-linux I think I'd like to see this published as a separate crate sysfs-pwm-async. We'd still add the code to this repo, just with its own Cargo.toml. Thoughts?

@eldruin
Copy link
Member

eldruin commented Jan 9, 2024

Putting it in a separate crate seems fine to me since at least the current version does not share anything with the blocking version other than the error type.
If doing so, I would convert this repository to a workspace and have two folders like sysfs-pwm and sys-pwm-async like we do over at embedded-hal.
If you need help with the restructuring, let me know.

@lucavezoc
Copy link
Contributor Author

Putting it in a separate crate seems fine to me since at least the current version does not share anything with the blocking version other than the error type.

How would I go about doing that?

@nastevens
Copy link
Member

How would I go about doing that?

Hey sorry for the delay @lucaVuitton, I had planned to make this change but there's been some things in my personal life that are taking priority. If you want to take a try at it, a good place to start would be looking at the structure of the Cargo.toml files in https://github.com/rust-embedded/embedded-hal. There is a top workspace Cargo.toml, and then each subcrate (sysfs-pwm and sysfs-pwm-async) would get their own subdirectories with Cargo.toml files of their own. If you're okay waiting I'll also get back to this as soon as I can. Thanks!

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.

3 participants