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

Support for the ESP-IDF framework #452

Merged
merged 4 commits into from
Jul 15, 2023

Conversation

jasta
Copy link
Contributor

@jasta jasta commented Jul 14, 2023

Smoke tested on esp32c3 dev board. I've also tested a similar patch backported to v0.4.9 with much greater functionality including tokio + mio with other patches I've been working on and it's fully working.

Closes #379

Smoke tested on esp32c3 dev board.  I've also tested a similar patch
backported to v0.4.9 with much greater functionality including tokio +
mio with other patches I've been working on and it's fully working.

Closes rust-lang#379
@jasta
Copy link
Contributor Author

jasta commented Jul 14, 2023

I'm not sure how to make the cargo check tests work either, pointers greatly appreciated.

@jasta
Copy link
Contributor Author

jasta commented Jul 14, 2023

The clippy check that's failing appears to be talking about a file I didn't touch (tests/socket.rs)? Weird...

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

You can ignore the Clippy warning

src/sys/unix.rs Outdated
self.0 & libc::MSG_EOR != 0
// TODO: Expose this constant in libc for the ESP-IDF/LwIP framework
#[cfg(target_os = "espidf")]
const MSG_EOR: libc::c_int = 8;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't want to maintain these constants so please add them to libc instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we might have alternatives here, let me do a little digging and get back to you with options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops yeah we can follow redox's example here and just conditionally remove the feature. I'll work on that soon and update. Thanks for the review!

src/sys/unix.rs Outdated
pub(crate) use libc::SO_OOBINLINE;
// TODO: Expose in libc for ESP-IDF/LwIP
#[cfg(target_os = "espidf")]
pub(crate) const MSG_TRUNC: libc::c_int = 4;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't want to maintain these constants so please add them to libc instead.

@jasta
Copy link
Contributor Author

jasta commented Jul 15, 2023

Much better, should be good to go now.

@Thomasdezeeuw Thomasdezeeuw merged commit e5a4d26 into rust-lang:master Jul 15, 2023
36 of 37 checks passed
@Thomasdezeeuw
Copy link
Collaborator

Thanks @jasta.

I've opened #454 to track CI support so we can ensure things keep working, any feedback on that would be welcome.

jasta added a commit to jasta/socket2 that referenced this pull request Jul 31, 2023
Not sure how this got missed in rust-lang#452, it was definitely compiling
before.

Properly closes rust-lang#379.
jasta added a commit to jasta/socket2 that referenced this pull request Jul 31, 2023
This was missed in rust-lang#452 because I wasn't testing with feature="all"
enabled for my small socket2 test.  For the full tokio integration I was
using v0.4.x which didn't need this fix.

Properly closes rust-lang#379.
Thomasdezeeuw pushed a commit that referenced this pull request Jul 31, 2023
This was missed in #452 because I wasn't testing with feature="all"
enabled for my small socket2 test.  For the full tokio integration I was
using v0.4.x which didn't need this fix.

Properly closes #379.
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.

Support ESP-IDF Rust Target (for ESP32-C3 especially)
2 participants