Skip to content

Change to retry setting up I2C command rather than waiting indefinitely #590

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

Closed
wants to merge 1 commit into from

Conversation

lonerzzz
Copy link
Contributor

@lonerzzz lonerzzz commented Aug 22, 2017

With a large I2C bus, I encountered a situation where the following line in esp32-hal-i2c.c, method i2cInitFix hung indefinitely.

while(!i2c->dev->command[2].done);

Adding the included changes resolved the problem in my situation, by allowing a retry.

@me-no-dev
Copy link
Member

Can we please have this tested with a few devices? What comes to mind is OLEDs for the big transfer size, EEPROM for holding the SDA while operation is executed and some more general sensors.

@lonerzzz
Copy link
Contributor Author

I have tested with MCU to MCU communication so far. Will be testing to sensors and with 1K transfers shortly.

@everslick
Copy link
Contributor

I tested this patch and it breaks EEPROM communication for me.

unfortunately i cannot say more about the specific reason, at this point, other then that.

i tested it with one of these cheep RTC / EEPROM modules.

@me-no-dev
Copy link
Member

I think it should maybe use some sort of timeout ;) have to see what is the max time that regular eeproms can keep the line LOW though

@lonerzzz
Copy link
Contributor Author

I have a fix that has proven more stable for handling lockup so I am closing this issue in favor of my newer submission.

@lonerzzz lonerzzz closed this Sep 28, 2017
@me-no-dev me-no-dev deleted the Prevent_lockup_in_i2cInitFix branch June 28, 2018 23:03
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