esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
15.98k stars 13.33k forks source link

i2c scanner not working correctly #254

Closed sanchosk closed 8 years ago

sanchosk commented 9 years ago

I am trying to run the i2c detect script from Arduino from http://playground.arduino.cc/Main/I2cScanner I modified the setup and started with Wire.pins(2, 0); Wire.setClock(100000); Unfortunately, the code returns error == 0 for all addresses. Am I doing something wrong? Did I not understood something?

wizard23 commented 9 years ago

I also have a problem with I2C communication when I build form current source.

I have an I2C accelerometer which I can sucessfully read out with the precompiled linux release.

But it does not work anymore in the current version built from source even when I call Wire.begin instead of the depricated Wire.begin.

I will gladly assist in any debugging if that helps.

sanchosk commented 9 years ago

Well, after a short digging within the source code I found the problem. Current endTransmission call returns number of sent bytes instead of the ACK status. I modified a single line within the i2c.cpp file - removing return size; replacing it with return ack; within the i2c_master_write_to function. All of the sudden, the i2c scanner works OK :)

igrr commented 9 years ago

@wizard23 can you capture the interaction using a logic analyzer or an oscilloscope?

wizard23 commented 9 years ago

I measured with a oscilloscope and it seems that in the new version the data pin remains floating while the clock pin seems fine.

In the old version and with the same oscilloscope both cloc and data seem fine.

I'm not 100% sure if it's really floating or stays high all the time with a lot of noise.

igrr commented 9 years ago

Can you please share the picture you see on the scope? Do you have a pull-up resistor on the SDA line?

wizard23 commented 9 years ago

i've reduced the code to a minimum and done some more research with an oscilloscope. The floating pin was a false alert. I forgot the ground connection so it was floating. Yes I have external 4.7k in addition to the internal pullups.

the first image is from the current (not working for me) version. Some notes: I had to use the i2c low level functions instead of the Wire lib because Wire checks the return codes and it chokes on some return value and the stops transmitting so you dont see much on the scope there. My guess is that it somehow generates one more clock low to high transition and that my MMA8452 accelerometer does not like that. Other i2c devices might accept it.: img_20150520_213244

the below image is from the precompiled 1.6.1 version. Here the last bits (that come from the accelerometer) have the correct value of 0x2A (in the top image its 0 which is wrong) : img_20150520_211330

I hacked together a version of the new i2c library that emulates the send bit style of the old lib in the new library and it works for me :) I should have made a picture of that too...I hope I find time tomorrow and test it again. I'm not 100% sure if the code is correct I2C and Wire still does not work for me. but it wortks for me so I copied it here. Sorry I am not yet experienced with pull requests but I'll try that tomorrow after more carefull testing.

changes in core_esp8266_si2c.c:

static bool twi_write_start(void) {
  SCL_HIGH();
  SDA_HIGH();
  if (SDA_READ() == 0) return false;
  twi_delay(twi_dcount);
  SDA_LOW();
  twi_delay(twi_dcount);

  // NEW: start condition
  SCL_LOW();
  twi_delay(twi_dcount);

  return true;
}

static bool twi_write_stop(void){
  unsigned int i = 0;

  // NEW: commented out next 2 lines
  //SCL_LOW();
  //SDA_LOW();

  twi_delay(twi_dcount);
  SCL_HIGH();
  while (SCL_READ() == 0 && (i++) < TWI_CLOCK_STRETCH);// Clock
stretching (up to 100us)
  twi_delay(twi_dcount);
  SDA_HIGH();
  twi_delay(twi_dcount);

  return true;
}

static bool twi_write_bit(bool bit) {
  unsigned int i = 0;

  // NEW: set clock low below
  //SCL_LOW();

  if (bit) SDA_HIGH();
  else SDA_LOW();
  twi_delay(twi_dcount+1);
  SCL_HIGH();
  while (SCL_READ() == 0 && (i++) < TWI_CLOCK_STRETCH);// Clock
stretching (up to 100us)

  // NEW: set clock low
  SCL_LOW();

  twi_delay(twi_dcount+1);
  return true;
}

static bool twi_read_bit(void) {
  unsigned int i = 0;

  // NEW: set cock low below
  // SCL_LOW();

  SDA_HIGH();
  twi_delay(twi_dcount+1);
  SCL_HIGH();
  while (SCL_READ() == 0 && (i++) < TWI_CLOCK_STRETCH);// Clock
stretching (up to 100us)
  bool bit = SDA_READ();
  twi_delay(twi_dcount);

  // NEW: set clock low
  SCL_LOW();

  return bit;
}
tytower commented 9 years ago

Have tried Wire.begin(sda,scl); using pins 4 and 5 and other combinations of begin functions including //Wire.setClock(40000); and many other speeds to no avail Cant get I2c working at all in 1.6.4 IDE

ghost commented 9 years ago

have to check your edits with my I2C things here, because some lines you removed deal with shitty slaves that do not want to release the data line on time. Scope image looks great BTW. Can you post images of both with and without your changes so I understand what is actually different?

ghost commented 9 years ago

I'm currently running some I2C things on my ESP without any modifications to the Wire lib tested 100000 and 400000 speeds

ghost commented 9 years ago

https://github.com/esp8266/Arduino/pull/303 the fix

ghost commented 9 years ago

I guess some devices need clock cycles to finish an operation. Had caught this in the read method, but never got it in write. What your changes did was exactly that... add a low clock on repeated start. But messed up the clock timing much. And broke other parts of the protocol.

tytower commented 9 years ago

So I wish to update so I can get this working . How do I do that now with the json setup? I have update at startup ticked . Will that do it automatically or do I have to do something else?

ghost commented 9 years ago

I'm not positive... I think that @igrr needs to update the package as well so you get the latest code. Then you need an Arduino 1.6+ info on how to add the json is on the forum as well :)

wizard23 commented 9 years ago

I just tested by building from current source and it works with Wire now :) I will make a scope picture in the next days. :) ktnx!

ghost commented 9 years ago

I'm so jealous that you have a 70 MHz digi scope... any picture is like porn to me...

wizard23 commented 9 years ago

I looked at the new code with the scope. To see the timing better I made two pictures:

below is a picture of a write operation (adress + 1 byte payload). It looks like the clock timing is strange because of the small spikes and I think clock should be high at the end of the write. But as I said: it works for my accelerometer. write_i2c

below is a picture of a read operation (adress + 1 byte reading). The clock timing of the reading looks different. and the clock is low at the beginning since the last write left it in that state read_12c

ghost commented 9 years ago

wow? this is which new code? looks like what you had changed. @igrr also sent me some images and this is what clock looked like with the commenting of the lines and moving the clock. With my source, they should be all about equal pulses?

igrr commented 9 years ago

I'll run a test on my side again a bit later to make sure what went into the commit is what we were testing.

On Tue, May 26, 2015 at 1:24 PM, ficeto notifications@github.com wrote:

wow? this is which new code? looks like what you had changed. @igrr https://github.com/igrr also sent me some images and this is what clock looked like with the commenting of the lines and moving the clock. With my source, they should be all about equal pulses?

— Reply to this email directly or view it on GitHub https://github.com/esp8266/Arduino/issues/254#issuecomment-105481732.

ghost commented 9 years ago

@wizard23 I imagine that you have that scaled so two of the big squares are 3.3V? The fluctuations at clock change are really bothering me... quite the spikes...

wizard23 commented 9 years ago

scaling is at 2V so yes the big squares are 3.3V. about the fluctuations: I think they coincide with the really narrow clock pulses. Also: is the stop signal in the read picture correct?

wizard23 commented 9 years ago

latest test was done with version built from: https://github.com/esp8266/Arduino/commit/49c25b9c1b38458b2e9508e4540519caaf052e04 I will recheck if I have used the correct version. pretty sure though that I did.

ghost commented 9 years ago

fluctuations seem to be on the falling clock, i guess the drain causes it. bello is a comparison between your changes and my code before the fix mentioned above. You can see how much the images from your scope look like the images from your code. My code is with the equal clock pulses and the "fix" just introduced extra clocks if the slave holds the SDA. screen shot 2015-05-22 at 14 18 58

wizard23 commented 9 years ago

strange. I must have messed up, sorry. I will check that again.

ghost commented 9 years ago

nothing to be sorry about, you are actually helping :) are you using pull-up resistors on the I2C lines? If so also can you take an image without? And if not, take one with? I wander if decoupling caps would fix those fluctuations

tytower commented 9 years ago

any progress on this?

ghost commented 9 years ago

ok, i got a bit too excited on @wizard23's DSO porn and got me one of those :) Here is an image of the I2C as it's in the github repo: i2c_porn

ghost commented 9 years ago

Here i'm using 1K resistors to pull the lines as they seem to get the best square looking signal

tytower commented 9 years ago

I'm curious . I'm trying to get a sparkfun breakout board of the BMP180 running correctly . It has 4.7K pullup resistors built in .

How about trying your new scope with 4.7 K and show us the pic ? I know when I get a new toy I grab every chance to use it .

tytower commented 9 years ago

I got I2C device (BMP180) working properly at this speed (80 Mhz) with this change in the BMP180 library

https://github.com/sparkfun/BMP180_Breakout/issues/1#issuecomment-106976786

tytower commented 9 years ago

I see on the datasheet from Bosh that 2.2K to 10K are fine . What does the scope look like with 10K ?

Note:2 (1) Pull-up resistors for I C bus, Rp = 2.2kΩ ... 10kΩ, typ. 4.7kΩ Figure 2: Typical application ci "

tytower commented 9 years ago

Little bit cliquey here are we , don't talk to anyone outside the circle?

wizard23 commented 9 years ago

sorry for taking so long. It works now and the timing looks correct. I must have mixed up the arduino ide i was using since i have so many now :) I think this can be closed :) at least from my point of view.

wizard23 commented 9 years ago

@tytower I cant give you a scope pic of the difference between 2.2k and 10k pullups because I dont want to solder around on my board. but from what I know I think the one with the 10k pullups will have less steep rising edges because the 10k resistor takes longer to charge the parasitic capacitance in the i2c wires.

So for high I2C speeds you need lower resistors but it also "wastes" more energy since current will flow through the resistors every time the wire gets driven low.

This is probably only a concern in battery powered low power apllications like a I2C realtime clock that should run for several months.

tytower commented 9 years ago

Thanks for the explanation I understand more nowOn Tue, 16 Jun 2015 04:37:21 -0700 wizard23 notifications@github.com wrote:

@tytower I cant give you a scope pic of the difference between 4.7k and 10k pullups because I dont want to solder around on my board. but from what I know I think the one with the 10k pullups will have less steep rising edges because the 10k resistor takes longer to charge the parasitic capacitance in the i2c wires.

So for high I2C speeds you need lower resistors but it also "wastes" more energy since current will flow through the resistors every time the wire gets driven low.

This is probably only a concern in battery powered low power apllications like a I2C realtime clock that should run for several months.


Reply to this email directly or view it on GitHub: https://github.com/esp8266/Arduino/issues/254#issuecomment-112392800

Yahoo tytower@yahoo.com

Yazzcat commented 9 years ago

Think I found a small bug in the implementation of the protocol. While using I2C scanner, it did find my I2C device only once. This device is actually an ATMega328P running a Slave receiver. It worked pretty well with other microcontrollers as a slave.

After analyzing the signals with a protocol analyzer, I found out that the STOP condition never was sent after a NACK was received, even after using Wire.endTransmission(1);

So I added the code for twi_writeTo in core_esp8266_si2c.c to:

    if (!twi_write_byte(((address << 1) | 0) & 0xFF)) {

->      if (sendStop) twi_write_stop();
        return 2;//received NACK on transmit of address
    }

This way, the STOP condition will be sent. Apparently, the ATMega hardware needs this to function. Now it works well.