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

I2C: support 10-bit addressing #2507

Open
bugadani opened this issue Nov 8, 2024 · 4 comments
Open

I2C: support 10-bit addressing #2507

bugadani opened this issue Nov 8, 2024 · 4 comments
Labels
1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. api peripheral:i2c I2C peripheral

Comments

@bugadani
Copy link
Contributor

bugadani commented Nov 8, 2024

As noted by #2493 (comment)

We have a few options to support 10-bit addressing in inherent methods:

  • Create separate _10b methods
  • Change the address parameter into:
    • enum Address {Seven(u8), Ten(u16)} - too verbose?
    • impl TryInto<Address> and let the user pass either u8 or u16 - too implicit?
  • A separate AddressMode enum, and always take address: u16

Any other, perhaps better ideas?

@bugadani bugadani added status:needs-attention This should be prioritized peripheral:i2c I2C peripheral labels Nov 8, 2024
@github-project-automation github-project-automation bot moved this to Todo in esp-rs Nov 8, 2024
@Dominaezzz
Copy link
Collaborator

  • too implicit?

There's some precedence for this in the I8080 driver. Just want to note that the decision made here could also be made there as well, for consistency sake.

  • enum Address {Seven(u8), Ten(u16)}

Might be worth considering whether this can also be reused for the slave driver in future, as that may influence the decision.

@bugadani
Copy link
Contributor Author

bugadani commented Nov 8, 2024

There's some precedence for this in the I8080 driver

I'm just not entirely in love with the idea that users will need to add _u8 or _u16 at the end of their addresses, always. Though I guess we can implement TryFrom<i32> as well and pick one option (or "narrowest that fits") for a bare numeric literal.

@bugadani bugadani added the api label Nov 9, 2024
@MabezDev MabezDev removed the status:needs-attention This should be prioritized label Nov 22, 2024
@tom-borcin tom-borcin added the investigation Not needed for 1.0, but don't know if we can support without breaking the driver. label Dec 18, 2024
@MabezDev MabezDev added this to the 1.0.0-beta.0 milestone Jan 6, 2025
@bjoernQ
Copy link
Contributor

bjoernQ commented Jan 7, 2025

In theory after #2864 this is not strictly a blocker since we should be able to add 10-bit addressing support in a non-breaking way later

@MabezDev MabezDev added beta-blocker and removed 1.0-blocker investigation Not needed for 1.0, but don't know if we can support without breaking the driver. labels Jan 10, 2025
@MabezDev MabezDev modified the milestones: 0.23, 1.0.0-beta.0 Jan 13, 2025
@JurajSadel JurajSadel assigned JurajSadel and unassigned JurajSadel Jan 14, 2025
@MabezDev
Copy link
Member

In theory after #2864 this is not strictly a blocker since we should be able to add 10-bit addressing support in a non-breaking way later

I agree, I think that fact we can support this post 1.0 is enough now. 10bit support is rare enough that I don't think we need this for 1.0.

@MabezDev MabezDev added 1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. and removed beta-blocker labels Jan 14, 2025
@MabezDev MabezDev removed this from the 1.0.0-beta.0 milestone Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 non-breaking Not needed for 1.0 and can be supported without breaking the driver. api peripheral:i2c I2C peripheral
Projects
Status: Todo
Development

No branches or pull requests

6 participants