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

Document SemVer hazards of spi::Operation::DelayNs #552

Closed
GrantM11235 opened this issue Dec 22, 2023 · 1 comment · Fixed by #558
Closed

Document SemVer hazards of spi::Operation::DelayNs #552

GrantM11235 opened this issue Dec 22, 2023 · 1 comment · Fixed by #558
Milestone

Comments

@GrantM11235
Copy link
Contributor

Imagine this timeline:

  • A driver author releases cool-driver 1.0.0 which uses SpiDevice but doesn't use DelayNs.
  • A user uses a SpiDevice impl from embedded_hal_bus with NoDelay. The user tests it and it works fine.
  • The driver author notices a timing problem and adds a DelayNs and releases it as cool-driver 1.0.1
  • The user updates cool-driver (maybe even by accident). It still compiles with no warnings.
  • The program panics at runtime.

Aside from making actual changes to the SpiDevice trait, I can only think of two possible "solutions":

  1. Tell the author of cool-driver that by adding a DelayNs they have made a breaking change and they must release it as version 2.0.0 instead.
  2. Tell the user that they shouldn't really use NoDelay, but if they do they must pin the version of any dependency that they use it with.

I don't really like either of these options, but I don't have any better ideas.

@eldruin eldruin added this to the v1.0.0 milestone Dec 23, 2023
Dirbaio added a commit to Dirbaio/embedded-hal that referenced this issue Dec 27, 2023
Dirbaio added a commit to Dirbaio/embedded-hal that referenced this issue Dec 27, 2023
@Dirbaio
Copy link
Member

Dirbaio commented Dec 27, 2023

opened #558 to address this. It picks solution 2, which is sort of what we agreed on when we added the "no delay" versions: it's theoretically semver-breaking but in practice it's going to be rare that a driver starts using delays if it wasn't before.

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 a pull request may close this issue.

3 participants