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

bitbang I2C running under Blinka did not work with the peripherals I tested. #18

Merged
merged 3 commits into from
Jun 4, 2021

Conversation

wolfmanjm
Copy link
Contributor

@wolfmanjm wolfmanjm commented Mar 18, 2021

I am running on an rpi zero w using Adafruit-Blinka 6.4.0 and adafruit-circuitpython-lc709203f 2.0.2 and adafruit-circuitpython-bitbangio 1.2.4 on raspbian buster python 3.7.

I am trying to use bitbangio as I also have an SSD1306 on the normal H/W I2C ports (and it won't work with the clock stretching needed with the lc7).

Initially when trying to use the bitbangio I always got a CRC error, no matter what frequency I set for it. (I tried everything from 10KHz down to 100Hz). Looking at a logic analyzer trace it was really confused by the I2C clock timing and was unable to decipher the I2C stream correctly. I then tried the bitbang I2C on a simpler chip (the MCP23017 using mcp230xx_simpletest.py). This also failed.

Examining the code and refering to the logic traces I cleaned up the clock generation to match the I2C specs, and it now worked with the MCP23017.

However it still did not work with the lc709203f. Again looking at the logic trace and comparing with a logic trace from the H/W I2C to the same chip I saw the difference was a stop after the write from the writeto_then_readfrom, which is incorrect according to the specs and the builtin bitbang in circuitpython. I modified writeto_then_readfrom() to disable the stop after the write by callinf _write() directly with the stop set to False.

The code now conforms very closely to the circuitpython I2C bitbang code.

The good news it now works with the lc709203f (which is a very finicky chip), I still get CRC errors on and off but that happens with the H/W I2C as well, and maybe a different bug which I will try to track down.
(It maybe that you need to do a proper _repeated_start after the write).
Comparing the logic trace with that generated by the H/W I2c now looks very similar (although a whole lot slower).

The caveats are on a pizero the maximum frequency is currently around 1KHz, setting it higher simply does nothing, this is not really surprising given it is a pi zero and bit twiddling in python is not going to be very fast. Rewriting this in C may speed it up a bit. (Non blinka versions of circuit python do have a builtin c based bitbang).

Changes made....

Made the pins simulate open drain (as RPI has no open drain), by setting to input when high.
Fix clock timing issues, more in line with the circuitpython i2c bitbang.
Replaced the time.sleep() calls with a wait() similar to the one in the bitbang SPI.
Stop writeto_then_readfrom() sending a stop at the end of the write (this upsets some chips) and is incorrect (https://github.com/adafruit/circuitpython/blob/11e510a06ab67b5b78349c91d38ea780f255a425/shared-bindings/bitbangio/I2C.c#L278)

Proposed future changes...

  1. Add detection of peripheral clock stretching
  2. speed it up a bit (1KHz is pretty dismal)
  3. The generated write clock is not symmetric, the low time being longer than the high time, this may not be an issue for most peripherals though, and may not be fixable.

Fix writeto_then_readfrom sending a stop at the end of the write (this upsets some chips)
 I notice that the stop was removed from writeto() for a reason, so instead of adding it back I just call _write() instead.
@jposada202020
Copy link

@makermelissa hello, I think the last check on this PR were successful could you take a look. I could test if needed with the SSD1306 display. Thanks :)

@jposada202020 jposada202020 added the needs retest Needs testing label May 13, 2021
@makermelissa
Copy link
Contributor

@jposada202020 yes, if you could test with the SSD1306, that would be great. I took a look over the code changes and they look good. @wolfmanjm thanks for submitting this!

Copy link
Contributor

@makermelissa makermelissa left a comment

Choose a reason for hiding this comment

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

Approving based on looking at code. We can merge after testing.

@jposada202020
Copy link

@makermelissa Will do Thanks

@jposada202020
Copy link

@makermelissa I tested this using a RP0 with Buster. and a Adafruit SSD1306

I did with the bouncing ball test in the library. Results are shown, The Speed of the videos is correct, as you will see in the video the speed is very very very slow. Not familiar with the purpose of the PR.

Try also the text, and it work, try the rectangle test, somehow the display did not show all the rectangles so I am wondering if the display gets a timeout as they draw very slow.

20210603_181506_2

@jposada202020 jposada202020 removed the needs retest Needs testing label Jun 3, 2021
@makermelissa
Copy link
Contributor

@jposada202020 yeah, bitbang is just going to be slow, especially on a pi zero. Is it slower than before this pr? If it either didn't work at all or it not slower, I'm fine with this PR as is.

@jposada202020
Copy link

Not sure... I could test that and compare, but tomorrow, I spent all my patience trying to edit code in a RP0, but tomorrow for sure I will compare :)

@jposada202020
Copy link

I test this with the unchanged library, speed about the same, a little bit (hard to measure) slower than this PR

20210603_234547_1

@ladyada
Copy link
Member

ladyada commented Jun 4, 2021

ok fyi bitbanging on linux in really really slow, like 20KHz or something. not surprising how pokey it is

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jun 5, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_BitbangIO to 1.3.0 from 1.2.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_BitbangIO#18 from wolfmanjm/fix/i2c-timing
  > Moved CI to Python 3.7
  > Added help text and problem matcher
  > Added pull request template
  > "Increase duplicate code check threshold "
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.

None yet

4 participants