Issue calling readLightLevel immediately after setMTReg

[claws]

The readLightLevel function returns an inconsistent number if it's called right after calling setMTreg when the mode is ONE_SHOT. setMTreg has delay_ms when the mode is CONTINUOUS but not ONE_SHOT, and this inconsistent behavior doesn't happen in CONTINUOUS modes.

This issue was originally raised in #57.

[bjarnebuchmann]

The readLightLevel function also returns an inconsistent number right after switching between the two CONTINUOUS HIGH_RES modes, as there are no delay (to allow a new measurement) in configure(). In this case, the return 2-byte measurement value is based on the previous-mode measurement, but the interpretation (int 2 float) in readLightLevel is based on the new mode.

Example code:

BH1750 lightMeter((byte) 0x5C);
void setup() {
...
  lightMeter.begin(BH1750::CONTINUOUS_HIGH_RES_MODE);
  lightMeter.configure(BH1750::CONTINUOUS_HIGH_RES_MODE);
  lightMeter.setMTreg(BH1750_DEFAULT_MTREG );
  delay(180);
  plotlight();
  lightMeter.configure(BH1750::CONTINUOUS_HIGH_RES_MODE_2);
  plotlight();
  delay(180);
  plotlight();
}

void plotlight(){
  Serial.print(millis());
  Serial.print(" Light: ");
  Serial.print(lightMeter.readLightLevel());
  Serial.println(" lx");
}

Result (@~800lx light) - my comments after the hash marks

1536 Light: 800.00 lx # Correct HRES_MODE measurement
1547 Light: 400.00 lx # False. HRES_MODE interpreted as HRES_MODE_2
1728 Light: 799.17 lx # Correct HRES_MODE_2 measurement

Maybe configure() should add a delay also for CONTINUOUS modes (and not just for ONE_TIME modes)?

/Bjarne

[claws]

I'm happy to receive pull requests if you would like to propose a fix? With so many other things on the go it's hard for me to find the time.

[bjarnebuchmann]

I understand. Unfortunately, I do not presently have a git pull (nor a way to actually expose that to you).

In order to do this "right" I would suggest to keep a "reset_time" with the object, so that a new/updated measurement can be taken only whenever the module is ready to deliver it. This would impact the delay() (or _delay_ms) in setMTreg() and readLightLevel(). The advantage would be that "hard" delays would not be necessary, ie the module/code could be non-blocking. In addition, it might be an advantage to store the latest (read and compute) light measurement (as a float), so that could be returned until a new measurement is ready and could be read.

I would not actually implement changes on this magnitude without consulting with the original author (you) in order to assert that it is a way, which you would approve for the module.

[coelner]

1. Maybe we could do this like the BSEC library from bosch where they return a timestamp where the next measurement is ready. (It is somehow a little bit different from our case, because the bosch sensors contains a microcontroller, whereas we do not have this)

• store the millis() timestamp in the library instance -OR-
• return this value to the user for further processing -OR-
• boolean for the status 'new value possible' ?

2. an internal storage for the last value is not straight forward, as the sensor can deliver it. I think the user should be aware of holding the right value, as the user needs to set this specific value into a context. If we would use a thread based concept, I would understand that

[coelner]

a call of setMTReg should touch lastReadTimestamp because of the direct mode I2C call: https://github.com/claws/BH1750/blob/37068ca0984ff60f29c2550c7cd674c1c6247bb9/src/BH1750.cpp#L162

But maybe I missed something?