From ad3e2f83b8586e9ee4ca989d94d378af91ab1cd7 Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Wed, 29 Jul 2026 11:57:54 -0500 Subject: [PATCH] [sgp4x] Fix datasheet conformance issues (#17828) Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- esphome/components/sgp4x/sgp4x.cpp | 56 +++++++++++++++++------------- esphome/components/sgp4x/sgp4x.h | 7 ++-- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/esphome/components/sgp4x/sgp4x.cpp b/esphome/components/sgp4x/sgp4x.cpp index 4e14833c16..bc6fe794a0 100644 --- a/esphome/components/sgp4x/sgp4x.cpp +++ b/esphome/components/sgp4x/sgp4x.cpp @@ -18,7 +18,7 @@ void SGP4xComponent::setup() { this->mark_failed(); return; } - this->serial_number_ = (uint64_t(raw_serial_number[0]) << 24) | (uint64_t(raw_serial_number[1]) << 16) | + this->serial_number_ = (uint64_t(raw_serial_number[0]) << 32) | (uint64_t(raw_serial_number[1]) << 16) | (uint64_t(raw_serial_number[2])); ESP_LOGD(TAG, "Serial number: %" PRIu64, this->serial_number_); @@ -32,7 +32,6 @@ void SGP4xComponent::setup() { featureset &= 0x1FF; if (featureset == SGP40_FEATURESET) { this->sgp_type_ = SGP40; - this->self_test_time_ = SPG40_SELFTEST_TIME; this->measure_time_ = SGP40_MEASURE_TIME; if (this->nox_sensor_) { ESP_LOGE(TAG, "SGP41 required for NOx, disabling NOx sensor"); @@ -42,7 +41,6 @@ void SGP4xComponent::setup() { } } else if (featureset == SGP41_FEATURESET) { this->sgp_type_ = SGP41; - this->self_test_time_ = SPG41_SELFTEST_TIME; this->measure_time_ = SGP41_MEASURE_TIME; } else { ESP_LOGD(TAG, "Unknown feature set 0x%0X", featureset); @@ -86,6 +84,8 @@ void SGP4xComponent::setup() { if (std::isnormal(this->voc_baselines_storage_.state0) && std::isnormal(this->voc_baselines_storage_.state1)) { ESP_LOGV(TAG, "Setting VOC baseline from save state0: %f, state1: %f", this->voc_baselines_storage_.state0, this->voc_baselines_storage_.state1); + // Sensirion advises restoring states only after interruptions shorter than 10 minutes; with no way to know + // how long the device was off, restoring a stale state still beats a fresh 12-hour learning phase voc_algorithm_.set_states(this->voc_baselines_storage_.state0, this->voc_baselines_storage_.state1); } } @@ -114,11 +114,15 @@ void SGP4xComponent::self_test_() { this->error_code_ = COMMUNICATION_FAILED; ESP_LOGD(TAG, ESP_LOG_MSG_COMM_FAIL); this->mark_failed(); + return; } - this->set_timeout(this->self_test_time_, [this]() { + this->set_timeout(SGP4X_SELF_TEST_TIME, [this]() { uint16_t reply = 0; - if (!this->read_data(reply) || (reply != 0xD400)) { + // SGP40: MSB is 0xD4 on success, LSB is undefined; SGP41: MSB is undefined, LSB bits 0/1 flag VOC/NOx pixel + // failures + bool passed = this->read_data(reply) && (this->sgp_type_ == SGP41 ? (reply & 0x0003) == 0 : (reply >> 8) == 0xD4); + if (!passed) { this->error_code_ = SELF_TEST_FAILED; ESP_LOGW(TAG, "Self-test failed (0x%X)", reply); this->mark_failed(); @@ -136,8 +140,8 @@ void SGP4xComponent::update_gas_indices_() { if (this->nox_sensor_ != nullptr) this->nox_index_ = this->nox_algorithm_.process(this->nox_sraw_); ESP_LOGV(TAG, "VOC: %" PRId32 ", NOx: %" PRId32, this->voc_index_, this->nox_index_); - // Store baselines after defined interval or if the difference between current and stored baseline becomes too - // much + // Store baselines once the minimum interval has passed and the state has drifted from the stored copy; + // both conditions limit flash wear if (this->store_baseline_ && this->seconds_since_last_store_ > SHORTEST_BASELINE_STORE_INTERVAL) { this->voc_algorithm_.get_states(this->voc_state0_, this->voc_state1_); if (std::abs(this->voc_baselines_storage_.state0 - this->voc_state0_) > MAXIMUM_STORAGE_DIFF_STATE0 || @@ -187,27 +191,28 @@ void SGP4xComponent::measure_raw_() { uint16_t command; uint16_t data[2]; size_t response_words; - // Use SGP40 measure command if we don't care about NOx - if (nox_sensor_ == nullptr) { + if (this->sgp_type_ == SGP40) { command = SGP40_CMD_MEASURE_RAW; response_words = 1; + } else if (this->nox_conditioning_start_.has_value() && millis() - *this->nox_conditioning_start_ < 10000) { + // SGP41 must run the NOx conditioning command for the first 10 seconds + command = SGP41_CMD_NOX_CONDITIONING; + response_words = 1; } else { - // SGP41 sensor must use NOx conditioning command for the first 10 seconds - if (this->nox_conditioning_start_.has_value() && millis() - *this->nox_conditioning_start_ < 10000) { - command = SGP41_CMD_NOX_CONDITIONING; - response_words = 1; - } else { - this->nox_conditioning_start_.reset(); - command = SGP41_CMD_MEASURE_RAW; - response_words = 2; - } + this->nox_conditioning_start_.reset(); + command = SGP41_CMD_MEASURE_RAW; + response_words = 2; + } + if (command == SGP41_CMD_NOX_CONDITIONING) { + // Conditioning requires the default parameters (compensation disabled) + data[0] = 0x8000; + data[1] = 0x6666; + } else { + // first parameter are the relative humidity ticks + data[0] = (uint16_t) std::llround((humidity * 65535) / 100); + // second parameter are the temperature ticks + data[1] = (uint16_t) (((temperature + 45) * 65535) / 175); } - uint16_t rhticks = (uint16_t) std::llround((humidity * 65535) / 100); - uint16_t tempticks = (uint16_t) (((temperature + 45) * 65535) / 175); - // first parameter are the relative humidity ticks - data[0] = rhticks; - // secomd parameter are the temperature ticks - data[1] = tempticks; if (!this->write_command(command, data, 2)) { ESP_LOGD(TAG, "write error (%d)", this->last_error_); @@ -279,7 +284,8 @@ void SGP4xComponent::dump_config() { " Type: %s\n" " Serial number: %" PRIu64 "\n" " Minimum Samples: %f", - sgp_type_ == SGP41 ? "SGP41" : "SPG40", this->serial_number_, GasIndexAlgorithm_INITIAL_BLACKOUT); + this->sgp_type_ == SGP41 ? "SGP41" : "SGP40", this->serial_number_, + GasIndexAlgorithm_INITIAL_BLACKOUT); } LOG_UPDATE_INTERVAL(this); diff --git a/esphome/components/sgp4x/sgp4x.h b/esphome/components/sgp4x/sgp4x.h index 4504c25448..2aaf06601b 100644 --- a/esphome/components/sgp4x/sgp4x.h +++ b/esphome/components/sgp4x/sgp4x.h @@ -39,16 +39,14 @@ static const uint16_t SGP4X_CMD_SELF_TEST = 0x280e; static const uint16_t SGP40_CMD_MEASURE_RAW = 0x260F; static const uint16_t SGP41_CMD_MEASURE_RAW = 0x2619; static const uint16_t SGP41_CMD_NOX_CONDITIONING = 0x2612; -static const uint8_t SGP41_SUBCMD_NOX_CONDITIONING = 0x12; // Shortest time interval of 3H for storing baseline values. // Prevents wear of the flash because of too many write operations const uint32_t SHORTEST_BASELINE_STORE_INTERVAL = 10800; -static const uint16_t SPG40_SELFTEST_TIME = 250; // 250 ms for self test -static const uint16_t SPG41_SELFTEST_TIME = 320; // 320 ms for self test +static const uint16_t SGP4X_SELF_TEST_TIME = 320; // maximum self-test duration for both SGP40 and SGP41 static const uint16_t SGP40_MEASURE_TIME = 30; static const uint16_t SGP41_MEASURE_TIME = 55; -// Store anyway if the baseline difference exceeds the max storage diff value +// Once the store interval has passed, store only if the baseline drifted from the stored copy by more than these // state0 is mean of variance estimator, hence can have larger absolute values and a larger diff threshold const float MAXIMUM_STORAGE_DIFF_STATE0 = 50.0f; // state1 is std of variance estimator, so it typically has smaller absolute values than state0, hence we use a smaller @@ -115,7 +113,6 @@ class SGP4xComponent final : public PollingComponent, uint64_t serial_number_; bool self_test_complete_; - uint16_t self_test_time_; sensor::Sensor *voc_sensor_{nullptr}; VOCGasIndexAlgorithm voc_algorithm_;