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

bump embassy crate versions, make ipv6 opt-in #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MabezDev
Copy link

Thanks for the library!

This PR updates embassy to the latest released version. It also makes ipv6 opt in, on my target enabling ipv6 uses too much flash, so I need to support ipv4 only.

@vpetrigo
Copy link
Owner

Hey @MabezDev!
Thank you for the PR. Will review it as soon as possible.


[dependencies]
log = { version = "~0.4", optional = true }
chrono = { version = "~0.4", default-features = false, optional = true }
miniloop = { version = "~0.3", optional = true }
embassy-net = { version = ">=0.5", features = ["udp", "proto-ipv4", "proto-ipv6", "medium-ip"], optional = true }
embassy-net = { version = "0.6", features = ["udp", "proto-ipv4", "medium-ip"], optional = true }
Copy link
Owner

Choose a reason for hiding this comment

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

Would you please tell, why we need to bump embassy-net version to 0.6? I checked the embassy's UdpSocket implementation and there seems to be no breaking changes since 0.5+. So, I would prefer to leave more permissive requirements.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not that familiar with what >= actually does, but I'm guessing it will allow cargo to update to 0.6 or 0.7 etc in the future? If so, I don't think this is a good idea, just because the socket didn't change in 0.6 doesn't mean it won't later, and if it does this crate will break the second the end user runs cargo update, even if the end user hasn't updated to the newer embassy-net version.

Copy link
Owner

Choose a reason for hiding this comment

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

In this case it would be up to the user to specify a specific version in his app/library Cargo.toml. Currently, the sntpc should work properly with 0.5 and 0.6 versions of the embassy-net. Restricting it to only 0.6 breaks examples in this repository which cannot resolve some of their example specific dependencies with the updated embassy-net == 0.6 rule.

@MabezDev
Copy link
Author

I missed the Ci failures, will resolve them soon.

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