From 9b107e7f2a4c5a0d01d2fee3ab9a0b2445797aca Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 16 Nov 2025 22:05:33 -0600 Subject: [PATCH 1/4] touch ups --- esphome/components/bh1750/bh1750.cpp | 299 ++++++++++++++++++--------- esphome/components/bh1750/bh1750.h | 33 ++- 2 files changed, 226 insertions(+), 106 deletions(-) diff --git a/esphome/components/bh1750/bh1750.cpp b/esphome/components/bh1750/bh1750.cpp index 2fc476c17d..f2766e3c64 100644 --- a/esphome/components/bh1750/bh1750.cpp +++ b/esphome/components/bh1750/bh1750.cpp @@ -1,8 +1,8 @@ #include "bh1750.h" #include "esphome/core/log.h" +#include "esphome/core/application.h" -namespace esphome { -namespace bh1750 { +namespace esphome::bh1750 { static const char *const TAG = "bh1750.sensor"; @@ -13,6 +13,31 @@ static const uint8_t BH1750_COMMAND_ONE_TIME_L = 0b00100011; static const uint8_t BH1750_COMMAND_ONE_TIME_H = 0b00100000; static const uint8_t BH1750_COMMAND_ONE_TIME_H2 = 0b00100001; +static constexpr uint32_t MEASUREMENT_TIMEOUT_MS = 2000; +static constexpr float HIGH_LIGHT_THRESHOLD_LX = 7000.0f; + +// Measurement time constants (datasheet values) +static constexpr uint16_t MTREG_DEFAULT = 69; +static constexpr uint16_t MTREG_MIN = 31; +static constexpr uint16_t MTREG_MAX = 254; +static constexpr uint16_t MEAS_TIME_L_MS = 24; // L-resolution max measurement time @ mtreg=69 +static constexpr uint16_t MEAS_TIME_H_MS = 180; // H/H2-resolution max measurement time @ mtreg=69 + +// Conversion constants (datasheet formulas) +static constexpr float RESOLUTION_DIVISOR = 1.2f; // counts to lux conversion divisor +static constexpr float MODE_H2_DIVISOR = 2.0f; // H2 mode has 2x higher resolution + +// MTreg calculation constants +static constexpr int COUNTS_TARGET = 50000; // Target counts for optimal range (avoid saturation) +static constexpr int COUNTS_NUMERATOR = 10; +static constexpr int COUNTS_DENOMINATOR = 12; + +// MTreg register bit manipulation constants +static constexpr uint8_t MTREG_HI_SHIFT = 5; // High 3 bits start at bit 5 +static constexpr uint8_t MTREG_HI_MASK = 0b111; // 3-bit mask for high bits +static constexpr uint8_t MTREG_LO_SHIFT = 0; // Low 5 bits start at bit 0 +static constexpr uint8_t MTREG_LO_MASK = 0b11111; // 5-bit mask for low bits + /* bh1750 properties: @@ -43,74 +68,7 @@ void BH1750Sensor::setup() { this->mark_failed(); return; } -} - -void BH1750Sensor::read_lx_(BH1750Mode mode, uint8_t mtreg, const std::function &f) { - // turn on (after one-shot sensor automatically powers down) - uint8_t turn_on = BH1750_COMMAND_POWER_ON; - if (this->write(&turn_on, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Power on failed"); - f(NAN); - return; - } - - if (active_mtreg_ != mtreg) { - // set mtreg - uint8_t mtreg_hi = BH1750_COMMAND_MT_REG_HI | ((mtreg >> 5) & 0b111); - uint8_t mtreg_lo = BH1750_COMMAND_MT_REG_LO | ((mtreg >> 0) & 0b11111); - if (this->write(&mtreg_hi, 1) != i2c::ERROR_OK || this->write(&mtreg_lo, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Set measurement time failed"); - active_mtreg_ = 0; - f(NAN); - return; - } - active_mtreg_ = mtreg; - } - - uint8_t cmd; - uint16_t meas_time; - switch (mode) { - case BH1750_MODE_L: - cmd = BH1750_COMMAND_ONE_TIME_L; - meas_time = 24 * mtreg / 69; - break; - case BH1750_MODE_H: - cmd = BH1750_COMMAND_ONE_TIME_H; - meas_time = 180 * mtreg / 69; - break; - case BH1750_MODE_H2: - cmd = BH1750_COMMAND_ONE_TIME_H2; - meas_time = 180 * mtreg / 69; - break; - default: - f(NAN); - return; - } - if (this->write(&cmd, 1) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Start measurement failed"); - f(NAN); - return; - } - - // probably not needed, but adjust for rounding - meas_time++; - - this->set_timeout("read", meas_time, [this, mode, mtreg, f]() { - uint16_t raw_value; - if (this->read(reinterpret_cast(&raw_value), 2) != i2c::ERROR_OK) { - ESP_LOGW(TAG, "Read data failed"); - f(NAN); - return; - } - raw_value = i2c::i2ctohs(raw_value); - - float lx = float(raw_value) / 1.2f; - lx *= 69.0f / mtreg; - if (mode == BH1750_MODE_H2) - lx /= 2.0f; - - f(lx); - }); + this->state_ = IDLE; } void BH1750Sensor::dump_config() { @@ -124,45 +82,186 @@ void BH1750Sensor::dump_config() { } void BH1750Sensor::update() { - // first do a quick measurement in L-mode with full range - // to find right range - this->read_lx_(BH1750_MODE_L, 31, [this](float val) { - if (std::isnan(val)) { - this->status_set_warning(); - this->publish_state(NAN); + // Start coarse measurement to determine optimal mode/mtreg + if (this->state_ != IDLE) { + // Safety timeout: reset if stuck + if (millis() - this->measurement_start_time_ > MEASUREMENT_TIMEOUT_MS) { + ESP_LOGW(TAG, "Measurement timeout, resetting state"); + this->state_ = IDLE; + } else { + ESP_LOGW(TAG, "Previous measurement not complete, skipping update"); return; } + } - BH1750Mode use_mode; - uint8_t use_mtreg; - if (val <= 7000) { - use_mode = BH1750_MODE_H2; - use_mtreg = 254; - } else { - use_mode = BH1750_MODE_H; - // lx = counts / 1.2 * (69 / mtreg) - // -> mtreg = counts / 1.2 * (69 / lx) - // calculate for counts=50000 (allow some range to not saturate, but maximize mtreg) - // -> mtreg = 50000*(10/12)*(69/lx) - int ideal_mtreg = 50000 * 10 * 69 / (12 * (int) val); - use_mtreg = std::min(254, std::max(31, ideal_mtreg)); - } - ESP_LOGV(TAG, "L result: %f -> Calculated mode=%d, mtreg=%d", val, (int) use_mode, use_mtreg); + if (!this->start_measurement_(BH1750_MODE_L, 31)) { + this->status_set_warning(); + this->publish_state(NAN); + return; + } - this->read_lx_(use_mode, use_mtreg, [this](float val) { - if (std::isnan(val)) { + this->state_ = WAITING_COARSE_MEASUREMENT; + this->enable_loop(); // Enable loop while measurement in progress +} + +void BH1750Sensor::loop() { + const uint32_t now = App.get_loop_component_start_time(); + + switch (this->state_) { + case IDLE: + // Disable loop when idle to save cycles + this->disable_loop(); + break; + + case WAITING_COARSE_MEASUREMENT: + if (now - this->measurement_start_time_ >= this->measurement_duration_) { + this->state_ = READING_COARSE_RESULT; + } + break; + + case READING_COARSE_RESULT: { + float lx; + if (!this->read_measurement_(lx)) { this->status_set_warning(); this->publish_state(NAN); - return; + this->state_ = IDLE; + break; } - ESP_LOGD(TAG, "'%s': Illuminance=%.1flx", this->get_name().c_str(), val); + + this->process_coarse_result_(lx); + + // Start fine measurement with optimal settings + if (!this->start_measurement_(this->fine_mode_, this->fine_mtreg_)) { + this->status_set_warning(); + this->publish_state(NAN); + this->state_ = IDLE; + break; + } + + this->state_ = WAITING_FINE_MEASUREMENT; + break; + } + + case WAITING_FINE_MEASUREMENT: + if (now - this->measurement_start_time_ >= this->measurement_duration_) { + this->state_ = READING_FINE_RESULT; + } + break; + + case READING_FINE_RESULT: { + float lx; + if (!this->read_measurement_(lx)) { + this->status_set_warning(); + this->publish_state(NAN); + this->state_ = IDLE; + break; + } + + ESP_LOGD(TAG, "'%s': Illuminance=%.1flx", this->get_name().c_str(), lx); this->status_clear_warning(); - this->publish_state(val); - }); - }); + this->publish_state(lx); + this->state_ = IDLE; + break; + } + } +} + +bool BH1750Sensor::start_measurement_(BH1750Mode mode, uint8_t mtreg) { + // Power on + uint8_t turn_on = BH1750_COMMAND_POWER_ON; + if (this->write(&turn_on, 1) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Power on failed"); + return false; + } + + // Set MTreg if changed + if (this->active_mtreg_ != mtreg) { + uint8_t mtreg_hi = BH1750_COMMAND_MT_REG_HI | ((mtreg >> MTREG_HI_SHIFT) & MTREG_HI_MASK); + uint8_t mtreg_lo = BH1750_COMMAND_MT_REG_LO | ((mtreg >> MTREG_LO_SHIFT) & MTREG_LO_MASK); + if (this->write(&mtreg_hi, 1) != i2c::ERROR_OK || this->write(&mtreg_lo, 1) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Set measurement time failed"); + this->active_mtreg_ = 0; + return false; + } + this->active_mtreg_ = mtreg; + } + + // Start measurement + uint8_t cmd; + uint16_t meas_time; + switch (mode) { + case BH1750_MODE_L: + cmd = BH1750_COMMAND_ONE_TIME_L; + meas_time = MEAS_TIME_L_MS * mtreg / MTREG_DEFAULT; + break; + case BH1750_MODE_H: + cmd = BH1750_COMMAND_ONE_TIME_H; + meas_time = MEAS_TIME_H_MS * mtreg / MTREG_DEFAULT; + break; + case BH1750_MODE_H2: + cmd = BH1750_COMMAND_ONE_TIME_H2; + meas_time = MEAS_TIME_H_MS * mtreg / MTREG_DEFAULT; + break; + default: + return false; + } + + if (this->write(&cmd, 1) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Start measurement failed"); + return false; + } + + // Store current measurement parameters + this->current_mode_ = mode; + this->current_mtreg_ = mtreg; + this->measurement_start_time_ = millis(); + this->measurement_duration_ = meas_time + 1; // Add 1ms for safety + + return true; +} + +bool BH1750Sensor::read_measurement_(float &lx_out) { + uint16_t raw_value; + if (this->read(reinterpret_cast(&raw_value), 2) != i2c::ERROR_OK) { + ESP_LOGW(TAG, "Read data failed"); + return false; + } + raw_value = i2c::i2ctohs(raw_value); + + float lx = float(raw_value) / RESOLUTION_DIVISOR; + lx *= float(MTREG_DEFAULT) / this->current_mtreg_; + if (this->current_mode_ == BH1750_MODE_H2) { + lx /= MODE_H2_DIVISOR; + } + + lx_out = lx; + return true; +} + +void BH1750Sensor::process_coarse_result_(float lx) { + if (std::isnan(lx)) { + // Use defaults if coarse measurement failed + this->fine_mode_ = BH1750_MODE_H2; + this->fine_mtreg_ = MTREG_MAX; + return; + } + + if (lx <= HIGH_LIGHT_THRESHOLD_LX) { + this->fine_mode_ = BH1750_MODE_H2; + this->fine_mtreg_ = MTREG_MAX; + } else { + this->fine_mode_ = BH1750_MODE_H; + // lx = counts / 1.2 * (69 / mtreg) + // -> mtreg = counts / 1.2 * (69 / lx) + // calculate for counts=50000 (allow some range to not saturate, but maximize mtreg) + // -> mtreg = 50000*(10/12)*(69/lx) + int ideal_mtreg = COUNTS_TARGET * COUNTS_NUMERATOR * MTREG_DEFAULT / (COUNTS_DENOMINATOR * (int) lx); + this->fine_mtreg_ = std::min(MTREG_MAX, std::max(MTREG_MIN, ideal_mtreg)); + } + + ESP_LOGV(TAG, "L result: %.1f -> Calculated mode=%d, mtreg=%d", lx, (int) this->fine_mode_, this->fine_mtreg_); } float BH1750Sensor::get_setup_priority() const { return setup_priority::DATA; } -} // namespace bh1750 -} // namespace esphome +} // namespace esphome::bh1750 diff --git a/esphome/components/bh1750/bh1750.h b/esphome/components/bh1750/bh1750.h index a31eb33609..bd6b4c5b9a 100644 --- a/esphome/components/bh1750/bh1750.h +++ b/esphome/components/bh1750/bh1750.h @@ -4,10 +4,9 @@ #include "esphome/components/sensor/sensor.h" #include "esphome/components/i2c/i2c.h" -namespace esphome { -namespace bh1750 { +namespace esphome::bh1750 { -enum BH1750Mode { +enum BH1750Mode : uint8_t { BH1750_MODE_L, BH1750_MODE_H, BH1750_MODE_H2, @@ -21,13 +20,35 @@ class BH1750Sensor : public sensor::Sensor, public PollingComponent, public i2c: void setup() override; void dump_config() override; void update() override; + void loop() override; float get_setup_priority() const override; protected: - void read_lx_(BH1750Mode mode, uint8_t mtreg, const std::function &f); + // State machine states + enum State : uint8_t { + IDLE, + WAITING_COARSE_MEASUREMENT, + READING_COARSE_RESULT, + WAITING_FINE_MEASUREMENT, + READING_FINE_RESULT, + }; + // 4-byte aligned members + uint32_t measurement_start_time_{0}; + uint32_t measurement_duration_{0}; + + // 1-byte members grouped together to minimize padding + State state_{IDLE}; + BH1750Mode current_mode_{BH1750_MODE_L}; + uint8_t current_mtreg_{31}; + BH1750Mode fine_mode_{BH1750_MODE_H2}; + uint8_t fine_mtreg_{254}; uint8_t active_mtreg_{0}; + + // Helper methods + bool start_measurement_(BH1750Mode mode, uint8_t mtreg); + bool read_measurement_(float &lx_out); + void process_coarse_result_(float lx); }; -} // namespace bh1750 -} // namespace esphome +} // namespace esphome::bh1750 From 8934d4b498ec6f1ca02c2099bc87474984beaa18 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 16 Nov 2025 22:08:44 -0600 Subject: [PATCH 2/4] touch ups --- esphome/components/bh1750/bh1750.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/esphome/components/bh1750/bh1750.cpp b/esphome/components/bh1750/bh1750.cpp index f2766e3c64..16ef822b64 100644 --- a/esphome/components/bh1750/bh1750.cpp +++ b/esphome/components/bh1750/bh1750.cpp @@ -256,7 +256,7 @@ void BH1750Sensor::process_coarse_result_(float lx) { // calculate for counts=50000 (allow some range to not saturate, but maximize mtreg) // -> mtreg = 50000*(10/12)*(69/lx) int ideal_mtreg = COUNTS_TARGET * COUNTS_NUMERATOR * MTREG_DEFAULT / (COUNTS_DENOMINATOR * (int) lx); - this->fine_mtreg_ = std::min(MTREG_MAX, std::max(MTREG_MIN, ideal_mtreg)); + this->fine_mtreg_ = std::min((int) MTREG_MAX, std::max((int) MTREG_MIN, ideal_mtreg)); } ESP_LOGV(TAG, "L result: %.1f -> Calculated mode=%d, mtreg=%d", lx, (int) this->fine_mode_, this->fine_mtreg_); From 9b14444dad48e9bf3e19f83660051cfa1397ea09 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 16 Nov 2025 22:30:59 -0600 Subject: [PATCH 3/4] tidy --- esphome/components/bh1750/bh1750.cpp | 12 +++++++----- esphome/components/bh1750/bh1750.h | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/esphome/components/bh1750/bh1750.cpp b/esphome/components/bh1750/bh1750.cpp index 16ef822b64..eb91ee0789 100644 --- a/esphome/components/bh1750/bh1750.cpp +++ b/esphome/components/bh1750/bh1750.cpp @@ -82,10 +82,12 @@ void BH1750Sensor::dump_config() { } void BH1750Sensor::update() { + const uint32_t now = millis(); + // Start coarse measurement to determine optimal mode/mtreg if (this->state_ != IDLE) { // Safety timeout: reset if stuck - if (millis() - this->measurement_start_time_ > MEASUREMENT_TIMEOUT_MS) { + if (now - this->measurement_start_time_ > MEASUREMENT_TIMEOUT_MS) { ESP_LOGW(TAG, "Measurement timeout, resetting state"); this->state_ = IDLE; } else { @@ -94,7 +96,7 @@ void BH1750Sensor::update() { } } - if (!this->start_measurement_(BH1750_MODE_L, 31)) { + if (!this->start_measurement_(BH1750_MODE_L, MTREG_MIN, now)) { this->status_set_warning(); this->publish_state(NAN); return; @@ -131,7 +133,7 @@ void BH1750Sensor::loop() { this->process_coarse_result_(lx); // Start fine measurement with optimal settings - if (!this->start_measurement_(this->fine_mode_, this->fine_mtreg_)) { + if (!this->start_measurement_(this->fine_mode_, this->fine_mtreg_, now)) { this->status_set_warning(); this->publish_state(NAN); this->state_ = IDLE; @@ -166,7 +168,7 @@ void BH1750Sensor::loop() { } } -bool BH1750Sensor::start_measurement_(BH1750Mode mode, uint8_t mtreg) { +bool BH1750Sensor::start_measurement_(BH1750Mode mode, uint8_t mtreg, uint32_t now) { // Power on uint8_t turn_on = BH1750_COMMAND_POWER_ON; if (this->write(&turn_on, 1) != i2c::ERROR_OK) { @@ -214,7 +216,7 @@ bool BH1750Sensor::start_measurement_(BH1750Mode mode, uint8_t mtreg) { // Store current measurement parameters this->current_mode_ = mode; this->current_mtreg_ = mtreg; - this->measurement_start_time_ = millis(); + this->measurement_start_time_ = now; this->measurement_duration_ = meas_time + 1; // Add 1ms for safety return true; diff --git a/esphome/components/bh1750/bh1750.h b/esphome/components/bh1750/bh1750.h index bd6b4c5b9a..b9b3fa3f47 100644 --- a/esphome/components/bh1750/bh1750.h +++ b/esphome/components/bh1750/bh1750.h @@ -46,7 +46,7 @@ class BH1750Sensor : public sensor::Sensor, public PollingComponent, public i2c: uint8_t active_mtreg_{0}; // Helper methods - bool start_measurement_(BH1750Mode mode, uint8_t mtreg); + bool start_measurement_(BH1750Mode mode, uint8_t mtreg, uint32_t now); bool read_measurement_(float &lx_out); void process_coarse_result_(float lx); }; From 78a69cb744a265bcc74a2a86c1492e9683bae92b Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 16 Nov 2025 22:33:59 -0600 Subject: [PATCH 4/4] tidy --- esphome/components/bh1750/bh1750.cpp | 18 +++++++++--------- esphome/components/bh1750/bh1750.h | 1 + 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/esphome/components/bh1750/bh1750.cpp b/esphome/components/bh1750/bh1750.cpp index eb91ee0789..a2f94e1b4b 100644 --- a/esphome/components/bh1750/bh1750.cpp +++ b/esphome/components/bh1750/bh1750.cpp @@ -124,9 +124,7 @@ void BH1750Sensor::loop() { case READING_COARSE_RESULT: { float lx; if (!this->read_measurement_(lx)) { - this->status_set_warning(); - this->publish_state(NAN); - this->state_ = IDLE; + this->fail_and_reset_(); break; } @@ -134,9 +132,7 @@ void BH1750Sensor::loop() { // Start fine measurement with optimal settings if (!this->start_measurement_(this->fine_mode_, this->fine_mtreg_, now)) { - this->status_set_warning(); - this->publish_state(NAN); - this->state_ = IDLE; + this->fail_and_reset_(); break; } @@ -153,9 +149,7 @@ void BH1750Sensor::loop() { case READING_FINE_RESULT: { float lx; if (!this->read_measurement_(lx)) { - this->status_set_warning(); - this->publish_state(NAN); - this->state_ = IDLE; + this->fail_and_reset_(); break; } @@ -264,6 +258,12 @@ void BH1750Sensor::process_coarse_result_(float lx) { ESP_LOGV(TAG, "L result: %.1f -> Calculated mode=%d, mtreg=%d", lx, (int) this->fine_mode_, this->fine_mtreg_); } +void BH1750Sensor::fail_and_reset_() { + this->status_set_warning(); + this->publish_state(NAN); + this->state_ = IDLE; +} + float BH1750Sensor::get_setup_priority() const { return setup_priority::DATA; } } // namespace esphome::bh1750 diff --git a/esphome/components/bh1750/bh1750.h b/esphome/components/bh1750/bh1750.h index b9b3fa3f47..0460427954 100644 --- a/esphome/components/bh1750/bh1750.h +++ b/esphome/components/bh1750/bh1750.h @@ -49,6 +49,7 @@ class BH1750Sensor : public sensor::Sensor, public PollingComponent, public i2c: bool start_measurement_(BH1750Mode mode, uint8_t mtreg, uint32_t now); bool read_measurement_(float &lx_out); void process_coarse_result_(float lx); + void fail_and_reset_(); }; } // namespace esphome::bh1750