stickbreaker / arduino-esp32

Arduino core for the ESP32
38 stars 24 forks source link

proposal to i2c #19

Closed ESP32DE closed 6 years ago

ESP32DE commented 6 years ago

see talk: https://github.com/espressif/arduino-esp32/issues/1061 PR on origin master : https://github.com/espressif/arduino-esp32/pull/1073

stickbreaker commented 6 years ago

@ESP32DE You are on to something. I spent some time with an oscilloscope. Your suggested modification reduced the spikes from: esp32de_17

to this: esp32de_16

Using your idea, I tried a changing the pinMode() options and reordering the digitalWrite(). This is what I achieved:

esp32de_18

No MORE SPIKES!, YEA!!

Change your code from:

pinMode(pin, OUTPUT_OPEN_DRAIN | PULLUP);
digitalWrite(pin, HIGH);

to:

digitalWrite(pin, HIGH);
pinMode(pin, OPEN_DRAIN | PULLUP);

Then I will merge it into my repo.

Chuck

ESP32DE commented 6 years ago

@stickbreaker :) you are right, i tested with an earier version of arduino esp32 and i tested with the actually version optional

// digitalWrite(scl, HIGH); // optional before
     pinMode(scl, OUTPUT_OPEN_DRAIN | PULLUP);
    digitalWrite(scl, HIGH); // tested

..

// digitalWrite(sda, HIGH); // optional before
     pinMode(sda, OUTPUT_OPEN_DRAIN | PULLUP);
    digitalWrite(sda, HIGH); // tested

cause not sure which version you use, i did optional code in first line instead. you must then use the optional suggestion like you found i did the same in Uart and have change the position to first line and setup the pin HIGH also i did in actually version the PR like you sayed

digitalWrite(scl, HIGH); // optional, tested successful in actually version 
pinMode(scl, OUTPUT_OPEN_DRAIN | PULLUP);
// digitalWrite(scl, HIGH); // successful tested in a prev version, but fails in the actually so use optional!

...

digitalWrite(sda, HIGH); // optional, tested successful in actually version
pinMode(sda, OUTPUT_OPEN_DRAIN | PULLUP);
// digitalWrite(sda, HIGH); // successful tested in a prev version, but fails in the actually so use optional!

Ok i did not check with OSC just in time, i see there is a different between using OPEN_DRAIN or OUTPUT_OPEN_DRAIN

#define OPEN_DRAIN        0x10
#define OUTPUT_OPEN_DRAIN 0x12

i have not tested "OPEN_DRAIN" instead "OUTPUT_OPEN_DRAIN" but if you have successfull tested this second option too i believe you, so yes will change the position of this code lines in the PR to yours and using "OPEN_DRAIN" instead "OUTPUT_OPEN_DRAIN"

best wishes rudi ;-)

edit: ( only for a reference info taken from esp idf )
GPIO mode : output only with open-drain mode GPIO_MODE_OUTPUT_OD = ((GPIO_MODE_DEF_OUTPUT)|(GPIO_MODE_DEF_OD)) GPIO mode : output and input with open-drain mode GPIO_MODE_INPUT_OUTPUT_OD = ((GPIO_MODE_DEF_INPUT)|(GPIO_MODE_DEF_OUTPUT)|(GPIO_MODE_DEF_OD))

ifrew commented 6 years ago

Glad you found this guys! I had started getting these timing errors again after syncing up to the latest ardunio esp32 core. I added manually in the change you stated but found it was still causing me issues. I had been using an earlier version of Chucks i2c recovery code (4files). I thought I would change the open_drain back to output_open_drain and all the timing issues disappeared! SO that made a difference. I then just saw now that chuck updated his i2c code in his master so I downloaded that. I noticed he used open_drain in there and thought it would fail but it doesn't! So between his updated code and using open_drain its now all seems to work!!! Just thought I'd post and let you know that.