espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.77k stars 743 forks source link

I2C.readFrom() returns values for a non existing devices #855

Closed MaBecker closed 8 years ago

MaBecker commented 8 years ago

ESP8266 with 1v85 latest pull

testing with some i2c devices and using a scanner function publish on the forum i2cdetect

I2C.readFrom(address, quantity) return a value even without a device on the i2c bus

I2C1.setup({sda: D4, scl: D5} );

// 0x52 is not existing
>I2C1.readFrom(0x52,1);
ERROR: No ACK
=new Uint8Array([200])
> 

// 0x3c is existing
>I2C1.readFrom(0x3c,1);
=new Uint8Array([65])

Espruino/targets/esp8266/jshardware.c

https://github.com/espruino/Espruino/blob/master/targets/esp8266/jshardware.c#L925

gfwilliams commented 8 years ago

Is this a bug? As far as I can tell this happens on all devices. I guess maybe it should just throw an exception?

MaBecker commented 8 years ago

Yes - exception, so we can use try and catch

MaBecker commented 8 years ago

No bug, works as coded as far I can see.

No idea about side effects of jsExceptionHere() in this case, will try now.

@tve what do you think about that ?

MaBecker commented 8 years ago

changes in jshI2CWrite()

...
error:
  i2c_master_stop();
  //jsError("No ACK");
  jsExceptionHere(JSET_INTERNALERROR, "I2CWrite : No ACK %d\n", ack);
}

and in jshI2CRead()

...
error:
  i2c_master_stop();
  //jsError("No ACK");
  jsExceptionHere(JSET_INTERNALERROR, "I2CRead : No ACK %d\n", ack);
}

Now try{ } catch{} is working for I2C.readFrom() and I2C.writeTo()

a i2cdetect javascript function could look like this:

function isDeviceOnBus(i2c, addr) {
  try {  
    return i2c.readFrom(addr,1);
  } 
  catch(err) { 
    return -1;
  }
}

function i2cdetect( i2c, first, last ) {
  if (typeof first === "undefined") first = 0x03;
  if (typeof (last) === "undefined") last = 0x77;

  print( "     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f" );
  for (var upper = 0; upper < 8; ++upper) {
    var line = upper + "0: ";
    for (var lower = 0; lower < 16; ++lower) {
      var address = (upper << 4) + lower;
      // Skip unwanted addresses
      if ((address < first) || (address > last)) {
        line += "   ";
        continue;
      }
      //console.log(isDeviceOnBus(i2c,address));
      // if (isDeviceOnBus(i2c,address) instanceof Array ) {
      // if (Array.isArray(isDeviceOnBus(i2c,address))) {
      if ( ! isDeviceOnBus(i2c,address) ) 
        line += (address + 0x100).toString( 16 ).substr( -2 )+" ";
      else 
        line += "-- ";
    }
    print( line );
  }
}

setTimeout(function(){i2cdetect( I2C1, 0x03, 0x77);},1000);

/* output - 

attached one  MLX90614

     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:          -- -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- 5a -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --

*/

not happy about this if ( ! isDeviceOnBus(i2c,address) )

@gfwilliams can you please suggest a elegant way, because this is confusing ?

gfwilliams commented 8 years ago

Thanks for the suggested changes...

I'm not sure what you expect instead of !isDeviceOnBus(i2c,address) - as far as I know that's basically what you'd have to do to see if a device is there.

MaBecker commented 8 years ago

sorry for not describing it clear.

Found a better readable solution: if ( isDeviceOnBus(i2c,address) !== -1 )

and

gfwilliams commented 8 years ago

Ahh - why not just:

function isDeviceOnBus(i2c, addr) {
  try {  
    i2c.readFrom(addr,1);
    return true;
  } catch(err) { 
    return false;
  }
}
MaBecker commented 8 years ago

That's it - thanks !

so now I use this simple if :

if (isDeviceOnBus(i2c,address))

tve commented 8 years ago

Is there something to fix here or can this ticket be closed?

MaBecker commented 8 years ago

@tve: something to fix

change: jsError(...) to jsExceptionHere(..) in jshI2CRead() and jshI2CWrite() reason: to be able to implement something like i2cdetect - see .js code above

Or is there a better way to accomplish this ?

gfwilliams commented 8 years ago

Sounds good to me.

MaBecker commented 8 years ago

created pull request #868