BH1750 MTreg min is 31 in the datasheet

[beta-tester]

a value of 31 should not be rejected. in the datasheet min is 31:

[claws]

Yeah, looks like you are right. Thanks for the fix.

The continuous integration step is failing due to code formatting not complying. I'm going to guess that it is because you have brackets around the new min and max mtreg constant values. You can ensure the changes are format compliant by running clang-format locally to check. Instructions can be found here

The example file examples/BH1750autoadjust/BH1750autoadjust.ino should also be updated to accomodate your proposed changes. In code and comments it references the mtreg value 32 but should make use of the new constants you are adding.

[beta-tester]

i think it failed because of the spaces i inserted to line up the valuse with the one of the previous line.

[coelner]

@beta-tester As far as I remember this was added by intention. https://github.com/claws/BH1750/blob/37068ca0984ff60f29c2550c7cd674c1c6247bb9/src/BH1750.cpp#L146

Did you tried the value 31 with your hardware?

[beta-tester]

yes, i did. and setMTreg(31); returned true and readLightLevel(); returned a value >=0. like with BH1750_MTREG_MAX and BH1750_DEFAULT_MTREG.

for MTreg = 31 at CONTINUOUS_HIGH_RES_MODE_2, i expect a resolution of about 0.93lx. 11.13 lx, 12.06 lx. 242.98 lx i think the values are close enough to the expected resolution.

all my BH1750 has the batch/lot number AAO 261 and sits on a GY-302 mdule.

a stripped down sketch i used on my ESP32-CAM:

// AI Thinker ESP32-CAM

#include <BH1750.h>
#include <Wire.h>

#define I2C_SDA 14 // SDA Connected to GPIO 14
#define I2C_SCL 15 // SCL Connected to GPIO 15

#define DEBUG_MODE true

#define BH1750_MODE       (BH1750::CONTINUOUS_HIGH_RES_MODE_2)
#define BH1750_MTREG_INIT (31)
#define BH1750_DELAY      (180)

BH1750 bh;
bool   bh_enabled = false;

void I2C_init() {
  Wire.begin(I2C_SDA, I2C_SCL);
}

void BH1750_init() {
  bh_enabled = bh.begin(BH1750_MODE);
  if (!bh_enabled) {
    Serial.println("Couldn't Find BH1750 Sensor");
  }
  else {
    Serial.println("BH1750 Sensor Found");
    delay(BH1750_DELAY);
    if(!bh.setMTreg(BH1750_MTREG_INIT)) {
      Serial.println("BH1750 setMTreg Error");
    }
    delay(BH1750_DELAY);
  }
}

void BH1750_get_data() {
  float lx;
  if (bh_enabled) {
    if(!bh.measurementReady(true))
      return;
    lx = bh.readLightLevel();
    Serial.printf("BH1750 readLightLevel: %.2f\n", lx);
    if(lx < 0)
      Serial.println("BH1750 readLightLevel Error");
  }
}

void setup() {
  Serial.begin(115200);
  Serial.setDebugOutput(DEBUG_MODE);
  Serial.println();

  I2C_init();
  BH1750_init();
}

void loop() {
  delay(10000);
  BH1750_get_data();
}

[beta-tester]

i took a look to another library for BH1750 Starmbi/hp_BH1750/src/hp_BH1750.h at line 42, there is a comment that the minimum values may depend on the used sensor device/module/chip.

when other user have problems with 31 as minimal MTreg value and you prefere to keep it at 32, then reject & close my pull request. it is not a problem for me - i fully understand it.

[claws]

The proposed change is consistent with the data sheet so I'm inclined to accept the merge request. The default value is still used so the change is really just expanding the valid range slightly. Do you see any issues with that @coelner?

[coelner]

No, I think this is totally fine. Mine are different, but I didn't test it yet since back then.

AA7 457 (GY-302)
AA7 447 (lolin ambient light v1.0.0)

To the fact that someone has got an existing BH1750 which works fine with this value, the datasheet is correct. My guess was that this was a typo. Astonishingly you can set even lower values.

[beta-tester]

thank you for merging.