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

Merge with socketcan-rs? #9

Open
fpagliughi opened this issue May 4, 2023 · 8 comments
Open

Merge with socketcan-rs? #9

fpagliughi opened this issue May 4, 2023 · 8 comments

Comments

@fpagliughi
Copy link

socketcan-rs v2.0 is finally out, and I'm looking at what to do next for it. Adding async/await seems most likely. I want to add support for the three top executors that my customers are using: tokio, async-std, and smol. Merging this crate, tokio-socketcan, into that one would be a good first step to get it done.

Would it be OK to merge this library into socketcan-rs?

If so, I would try to pull in this whole repository with all the history and commits intact, and then put all this code in a module that could be enabled/disabled with a cargo feature flag, like async-tokio. Then I would write similar capabilities for async-std/smol.

@marcelbuesing
Copy link
Collaborator

Sounds like a good idea to me, I think decreasing fragmentation makes sense. @oefd what do you think?

@fpagliughi
Copy link
Author

That would be great. I'd also want to hear from @oefd as well, to be sure. This crate doesn't have a clear license, other than a line in Cargo.toml, and I want to make sure you are both OK with merging it into socketcan-rs with the MIT license that it has.

@fpagliughi
Copy link
Author

Oh, also @nnarain has already started a port of this library for socketcan v2. Discussions are here:
socketcan-rs/socketcan-rs#39

@marcelbuesing
Copy link
Collaborator

That would be great. I'd also want to hear from @oefd as well, to be sure. This crate doesn't have a clear license, other than a line in Cargo.toml, and I want to make sure you are both OK with merging it into socketcan-rs with the MIT license that it has.

I would be ok with the MIT license.

@idletea
Copy link
Owner

idletea commented May 9, 2023

Sorry for the delay, honestly @marcelbuesing is more an owner for this crate than I am at this point anyway!

I'm happy to see this merged in, and happy to see it under an MIT license.

@fpagliughi
Copy link
Author

Awesome. I did a test over the weekend and it seems easy enough to merge the whole repo with all the history into the socketcan-rs crate. Someone else already PR'ed an async-io implementation for async-std/smol, so things will be pretty complete.

Thanks, both, for getting back, and the work. Makes my life easy to just do some integration :-)

@nnarain
Copy link

nnarain commented May 9, 2023

Pull request for use socketcan v2 #10

@fpagliughi
Copy link
Author

It is done.

This whole repository, complete with all its commit history, plus @nnarain 's updates for v2.0 is now in the master branch of socketcan-rs .

There is also some initial support for async-io (async-std & smol) up there as well. It should hopefully all be cleaned up and released within a few weeks.

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

4 participants