From 92b85f98e861638b70d6021b838f52f1998342cc Mon Sep 17 00:00:00 2001
From: Nate Lust <natelust@gmail.com>
Date: Sun, 10 Oct 2021 04:33:04 -0400
Subject: [PATCH] Sgp40 fix (#2462)

* Sample from SGP40 sensor at the appropriate interval

The spg40 sensor must be sampled at 1Hz for the VOC index algorithm
to work correctly. This commit introduces a on device timer to
sample correctly seperately from updating the public state of the
component.

* Add missing configuration values for SGP40

The SGP40 component was not printing all of it's configuration in
dump_config, add in the missing store_baseline value.

* Address review comments

* Format according to clang-tidy

* Attempt 2 at clang tidy
---
 esphome/components/sgp40/sgp40.cpp | 35 ++++++++++++++++++++++++------
 esphome/components/sgp40/sgp40.h   |  2 ++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/esphome/components/sgp40/sgp40.cpp b/esphome/components/sgp40/sgp40.cpp
index fddd0255b8..a3d2c74eb7 100644
--- a/esphome/components/sgp40/sgp40.cpp
+++ b/esphome/components/sgp40/sgp40.cpp
@@ -77,6 +77,20 @@ void SGP40Component::setup() {
   }
 
   this->self_test_();
+
+  /* The official spec for this sensor at https://docs.rs-online.com/1956/A700000007055193.pdf
+  indicates this sensor should be driven at 1Hz. Comments from the developers at:
+  https://github.com/Sensirion/embedded-sgp/issues/136 indicate the algorithm should be a bit
+  resilient to slight timing variations so the software timer should be accurate enough for
+  this.
+
+  This block starts sampling from the sensor at 1Hz, and is done seperately from the call
+  to the update method. This seperation is to support getting accurate measurements but
+  limit the amount of communication done over wifi for power consumption or to keep the
+  number of records reported from being overwhelming.
+  */
+  ESP_LOGD(TAG, "Component requires sampling of 1Hz, setting up background sampler");
+  this->set_interval(1000, [this]() { this->update_voc_index(); });
 }
 
 void SGP40Component::self_test_() {
@@ -224,21 +238,26 @@ uint8_t SGP40Component::generate_crc_(const uint8_t *data, uint8_t datalen) {
   return crc;
 }
 
-void SGP40Component::update() {
-  this->seconds_since_last_store_ += this->update_interval_ / 1000;
-
-  uint32_t voc_index = this->measure_voc_index_();
+void SGP40Component::update_voc_index() {
+  this->seconds_since_last_store_ += 1;
 
+  this->voc_index_ = this->measure_voc_index_();
   if (this->samples_read_ < this->samples_to_stabalize_) {
     this->samples_read_++;
     ESP_LOGD(TAG, "Sensor has not collected enough samples yet. (%d/%d) VOC index is: %u", this->samples_read_,
-             this->samples_to_stabalize_, voc_index);
+             this->samples_to_stabalize_, this->voc_index_);
+    return;
+  }
+}
+
+void SGP40Component::update() {
+  if (this->samples_read_ < this->samples_to_stabalize_) {
     return;
   }
 
-  if (voc_index != UINT16_MAX) {
+  if (this->voc_index_ != UINT16_MAX) {
     this->status_clear_warning();
-    this->publish_state(voc_index);
+    this->publish_state(this->voc_index_);
   } else {
     this->status_set_warning();
   }
@@ -247,6 +266,8 @@ void SGP40Component::update() {
 void SGP40Component::dump_config() {
   ESP_LOGCONFIG(TAG, "SGP40:");
   LOG_I2C_DEVICE(this);
+  ESP_LOGCONFIG(TAG, "  store_baseline: %d", this->store_baseline_);
+
   if (this->is_failed()) {
     switch (this->error_code_) {
       case COMMUNICATION_FAILED:
diff --git a/esphome/components/sgp40/sgp40.h b/esphome/components/sgp40/sgp40.h
index 62936102e7..bb68a1ffcf 100644
--- a/esphome/components/sgp40/sgp40.h
+++ b/esphome/components/sgp40/sgp40.h
@@ -46,6 +46,7 @@ class SGP40Component : public PollingComponent, public sensor::Sensor, public i2
 
   void setup() override;
   void update() override;
+  void update_voc_index();
   void dump_config() override;
   float get_setup_priority() const override { return setup_priority::DATA; }
   void set_store_baseline(bool store_baseline) { store_baseline_ = store_baseline; }
@@ -72,6 +73,7 @@ class SGP40Component : public PollingComponent, public sensor::Sensor, public i2
   bool store_baseline_;
   int32_t state0_;
   int32_t state1_;
+  int32_t voc_index_ = 0;
   uint8_t samples_read_ = 0;
   uint8_t samples_to_stabalize_ = static_cast<int8_t>(VOC_ALGORITHM_INITIAL_BLACKOUT) * 2;