Update README to capture change to undefined light level value of 65535

[claws]

When reading a light value there is a possibility that the data read is not the expected 2 bytes. The implementation assumes that 2 bytes are read, even if they are not, and continues progressing through the function which could then use garbage data in the uninitialised level variable.

After applying pull request #44 the maximum light level value has been changed from NAN to 65535. This value is well above the maximum possible lux value (which is raw/1.2 = 54612).

The README documentation should be updated to inform the user that a value above 54612 indicates an error or undefined scenario.

References are #26 and #44.

[coelner]

i take care of it within the fix for HRes2 Mode.

[guillaume-dorczynski]

This is wrong, the result is still divided by 1.2 just before the return, so the returned value cannot be greater than 54612, so we cannot check for error. Also, 65535 is a valid light measurement.

I think, you should use a int32_t so that you can return -1 for error.

Another problem in this function, is that you don't check the return value of Wire.requestFrom. With multiple I2C devices, the actual code will read 2 bytes if available, no matter which I2C device they originate from. This means, if this sensor fail (when disconnected from the breadboard, for example), the actual code will read 2 bytes of another device and the return value will be randomized and look valid, when it should return -1 (or 65535, whatever you decide). You should change the code to something like

if ( Wire.requestFrom(BH1750_I2CADDR, 2) == 2 )
{
  level = __wire_read();
  level <<= 8;
  level |= __wire_read();
}

Similarly, you should also check the result of wire.write().

Another suggestion, add a bool parameter: readLightLevel( bool raw ), which return raw value if bool is true.

Edit: Sorry I wanted to post in https://github.com/claws/BH1750/issues/26

I have other suggestions, but I think I will just write my own library... :)

[coelner]

+1 for the suggestion that we extend the library to return a raw value In general we should decide between int32_t or float as this library get used on embedded systems which usually are bad at float calculation. Therefore we could implement a #ifdefine similiar to the debug setting where the user can decide between int32_t or float usage the Wire.write() does not return any useful information and we already check the Wire.endTransmission() return value @Guillaume57 or you fork and send pull requests ;-)

[bjensen]

+1 @coelner's suggestion sounds like a great idea

[claws]

46 updated readme to reflect the range float values being returned.