rambo / TinyWire

My modifications to TinyWire Arduino libs
284 stars 121 forks source link

Strange address comparison line #32

Closed neilyoung closed 7 years ago

neilyoung commented 7 years ago

Hi,

I have some doubts regarding this line in usiTwiSlave.c

if ((USIDR == 0) || ((USIDR >> 1) == slaveAddress)) { // Slave address comparison??

In fact, the code is not working except I'm using slave address 0. I'm especially astonished about the right shift of USIDR. Isn't that register supposed to hold the original slave address at this stage?

rambo commented 7 years ago

I'm fairly sure the register holds whatever was sent on the wire, being the 8-bit address including the rw-bit so the shift to get rid of that bit is ok.

Something else might be weird here. Are you absolutely sure that ESP2866 has proper hardware I2C master that implements all of the optional features (most crucially clock-stretching) ?

You can try with an ATMega based arduino and the https://github.com/rambo/I2C/blob/master/examples/i2crepl/i2crepl.ino sketch in the improved I2C library

I don't think I have ever tested tinywire at all with 2313 so there definitely might be bugs as well

neilyoung commented 7 years ago

@rambo No, not really sure. Need to make sure with an oscilloscope tomorrow. Shift to get rid of the bit is ok, but what about the comparison? Say I use a slaveAddress of 0x20, and 0x20 or 0x21 is in the register. I might be completely dumb here, but how can an USIDR of 0x20 >> 1 ( 0x10) be equal my slaveAddress (= 0x20) then?

As a matter of fact I can only use address 0 from the ESP. The stack used on ESP is the stuff provided by Wire.cpp (https://github.com/esp8266/Arduino/blob/master/libraries/Wire/Wire.cpp).

BTW: The lib works great (at least the .c stuff, which I tested) on 2313. Absolutely, once the address problem is solved. Will keep you posted regarding the address, if I can find something with oscilloscope.

rambo commented 7 years ago

Your 7-bit slave address is 0x20, but the 8-bit address that the master MUST send on the wire is 0x40 or 0x41 depending on read vs write.

Now depending on the libraries used on the master side you either give it the 7-bit address and it figures out the rest from the API calls you're using, or you give it the correct 8-bit address depending on what you're doing (if using low-level commands to send raw data on the wire).

neilyoung commented 7 years ago

@rambo OK, makes sense. Is this documented somewhere?

neilyoung commented 7 years ago

@rambo But it doesn't work here.

Here is the ESP test code running in a loop:

    #define I2C_TINY_ADDRESS     0x40

    Wire.beginTransmission(I2C_TINY_ADDRESS);
    Wire.write(0xAA);
    Wire.write(0x55);
    Wire.requestFrom(I2C_TINY_ADDRESS, 1);  
    uint8_t buff[1];
    int i = 0;
    while (Wire.available() && i < 1)  {
        buff[i++] = Wire.read(); 
    }
    Wire.endTransmission(); 

And this on the Tiny:

    usiTwiSlaveInit(0x20); 

    while (1) {
        int nrOfBytesReceived = usiTwiAmountDataInReceiveBuffer();
        if (nrOfBytesReceived > 1) {
            // get it
            uint8_t b1 = usiTwiReceiveByte();
            uint8_t b2 = usiTwiReceiveByte();
            if (nrOfBytesReceived == 2 && b1 == 0xAA && b2 == 0x55) {
                LED_PORT &= ~_BV(LED);  // LED on
                _delay_ms(50);
                LED_PORT |= _BV(LED);   // LED off
            }

            // echo current time in seconds back
            usiTwiTransmitByte(seconds);
        }
    }

They just understand each other if I configure the slave address to be zero on both sides.

neilyoung commented 7 years ago

According to the implementation in core_esp8266_si2c.c (https://github.com/esp8266/Arduino/blob/4897e0006b5b0123a2fa31f67b14a3fff65ce561/cores/esp8266/core_esp8266_si2c.c) I would suspect to have to configure slave address 0x20 on the ESP as well, because the left shift is done inside. But this doesn't work as said.

unsigned char twi_readFrom(unsigned char address, unsigned char* buf, unsigned int len, unsigned char sendStop){
  unsigned int i;
  if(!twi_write_start()) return 4;//line busy
  if(!twi_write_byte(((address << 1) | 1) & 0xFF)) {
    if (sendStop) twi_write_stop();
    return 2;//received NACK on transmit of address
  }
  for(i=0; i<(len-1); i++) buf[i] = twi_read_byte(false);
  buf[len-1] = twi_read_byte(true);
  if(sendStop) twi_write_stop();
  i = 0;
  while(SDA_READ() == 0 && (i++) < 10){
    SCL_LOW();
    twi_delay(twi_dcount);
    SCL_HIGH();
    twi_delay(twi_dcount);
  }
  return 0;
}
neilyoung commented 7 years ago

@rambo Don't know what's different now, but changed to 0x20 on both sides, and look Ma: It works.

Sorry for the hassle.