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 #56

Merged
merged 23 commits into from
Jul 24, 2021
Merged

I2c #56

merged 23 commits into from
Jul 24, 2021

Conversation

JoNil
Copy link
Contributor

@JoNil JoNil commented Jul 7, 2021

This is the beginning of my i2c implementation, warning i have not tested it yet. Waiting for a pico order.

It is more or less a literal translation of the c code in the rp pico sdk.

TODO:

  • Test
  • Set the pin mode for i2c
  • Get the clock frequency from somewhere

Copy link
Contributor

@anall anall left a comment

Choose a reason for hiding this comment

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

I see this is a draft, but a few notes.

I'd probably base this off the v2 bits of atsamd-hal instead of the stm stuff, as this HAL's GPIO is heavily based on the atsamd-hal crate

It seems they haven't yet converted I2C to v2, but here's SPI: https://github.com/atsamd-rs/atsamd/blob/master/hal/src/thumbv7em/sercom/v2/spi.rs

rp2040-hal/src/i2c.rs Outdated Show resolved Hide resolved
rp2040-hal/src/i2c.rs Show resolved Hide resolved
rp2040-hal/src/i2c.rs Outdated Show resolved Hide resolved
rp2040-hal/src/i2c.rs Outdated Show resolved Hide resolved
rp2040-hal/src/i2c.rs Outdated Show resolved Hide resolved
@JoNil
Copy link
Contributor Author

JoNil commented Jul 8, 2021

@anall Can you check the type level shenanigans for the i2cX function.

What i think the code does is that it allows input of pins in any mode as long as they are convertible into FunctionI2C pins then it converts them into FunctionI2C pins and the struct only stores FunctionI2C pins.

Please tell me if you see any way to simplify it 😅

In the example pi skd i2c example https://github.com/raspberrypi/pico-examples/blob/146680d625bb2238a84c01dee118351efb5ede4e/i2c/lcd_1602_i2c/lcd_1602_i2c.c#L138
The pins are set into pull up mode. Will sda_pin.into_mode::() also change the pins into pull up mode?

Right now i have my test code here: https://github.com/JoNil/pico_plant/blob/9b4e5ac9c6ac0be6e51838c6f383f38e4657b1fc/src/main.rs#L28

I will add an example to this pr before it is ready :)

@anall
Copy link
Contributor

anall commented Jul 9, 2021

Can you check the type level shenanigans for the i2cX function.

I'll look deeper tomorrow.

What i think the code does is that it allows input of pins in any mode as long as they are convertible into FunctionI2C pins then it converts them into FunctionI2C pins and the struct only stores FunctionI2C pins.

It might just be easier to take a Pin<…> already in the mode, and rely on the caller calling .into_mode()?

The pins are set into pull up mode. Will sda_pin.into_mode::() also change the pins into pull up mode?

Not currently. Do we want that always done?

Also re. the Pin Pair constraint. Does the datasheet say these are paired? As far as I can tell you can use any valid SDA for that channel with any other valid SCL -- unless I missed something in the datasheet.

@JoNil
Copy link
Contributor Author

JoNil commented Jul 9, 2021

It might just be easier to take a Pin<…> already in the mode, and rely on the caller calling .into_mode()?

I changed it so that the code only accepts pin already in the FunctionI2C mode. How does the user choose if the pin should be a pull-up or not when the pin is in FunctionI2C mode? I can't seem to figure this out from the gpio code.

Also re. the Pin Pair constraint. Does the datasheet say these are paired? As far as I can tell you can use any valid SDA for that channel with any other valid SCL -- unless I missed something in the datasheet.

You are most likely correct! 😅 I will test this when i get my next batch of picos.

@JoNil JoNil marked this pull request as ready for review July 18, 2021 23:53
@JoNil
Copy link
Contributor Author

JoNil commented Jul 20, 2021

I have now successfully run a 1306 oled i2c display with this code :D

Copy link
Member

@9names 9names left a comment

Choose a reason for hiding this comment

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

LGTM

@9names 9names merged commit 077cba6 into rp-rs:main Jul 24, 2021
@9names 9names mentioned this pull request Jul 24, 2021
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.

3 participants