Restructure to allow asynchronous measurement, more detailed logging

esphome/components/mcp3428/mcp3428.cpp

@@ -44,6 +44,7 @@ void MCP3428Component::setup() {
     return;
   }
   this->prev_config_ = config;
+  single_measurement_active_ = false;
 }
 
 void MCP3428Component::dump_config() {
@@ -54,8 +55,14 @@ void MCP3428Component::dump_config() {
   }
 }
 
-float MCP3428Component::request_measurement(MCP3428Multiplexer multiplexer, MCP3428Gain gain,
+bool MCP3428Component::request_measurement(MCP3428Multiplexer multiplexer, MCP3428Gain gain,
-                                            MCP3428Resolution resolution) {
+                                           MCP3428Resolution resolution, uint32_t &timeout_wait) {
+  if (single_measurement_active_) {
+    timeout_wait = MEASUREMENT_TIME_16BIT_MS;  // maximum time
+    return false;
+  }
+
+  // calculate config byte
   uint8_t config = 0;
   // set ready bit to 1, will starts measurement in single shot mode and mark measurement as not yet ready in continuous
   // mode
@@ -71,39 +78,68 @@ float MCP3428Component::request_measurement(MCP3428Multiplexer multiplexer, MCP3
   // set gain
   config |= gain;
 
+  // find measurement wait time
+  switch (resolution) {
+    case MCP3428Resolution::MCP3428_12_BITS:
+      timeout_wait = MEASUREMENT_TIME_12BIT_MS;
+      break;
+    case MCP3428Resolution::MCP3428_14_BITS:
+      timeout_wait = MEASUREMENT_TIME_14BIT_MS;
+      break;
+    default:
+      timeout_wait = MEASUREMENT_TIME_16BIT_MS;
+      break;
+  }
+
   // If continuous mode and config (besides ready bit) are the same there is no need to upload new config, reading the
-  // result is enough
+  // result should be enough
-  if (!((this->prev_config_ & 0b00010000) > 0 and (this->prev_config_ & 0b01111111) == (config & 0b01111111))) {
+  if ((this->prev_config_ & 0b00010000) != 0 and (this->prev_config_ & 0b01111111) == (config & 0b01111111)) {
+    if (millis() - this->last_config_write_ms_ > timeout_wait) {
+      timeout_wait = 0;  // measurement probably immediately available
+    }
+  } else {
     if (this->write(&config, 1) != i2c::ErrorCode::NO_ERROR) {
-      this->status_set_warning();
+      this->status_set_warning("Error writing configuration to chip.");
-      return NAN;
+      timeout_wait = 1000;
+      single_measurement_active_ = false;
+      return false;
     }
     this->prev_config_ = config;
+    this->last_config_write_ms_ = millis();
   }
 
-  // MCP is now configured, read output until ready flag is 0 for a valid measurement
+  if (this->continuous_mode_) {
-  uint32_t start = millis();
+    this->single_measurement_active_ = false;
+  } else {
+    this->single_measurement_active_ = true;
+  }
+  return true;
+}
+
+bool MCP3428Component::poll_result(float &voltage) {
   uint8_t anwser[3];
-  while (true) {
+  voltage = NAN;
-    if (this->read(anwser, 3) != i2c::ErrorCode::NO_ERROR) {
+  if (this->read(anwser, 3) != i2c::ErrorCode::NO_ERROR) {
-      this->status_set_warning();
+    this->status_set_warning("Communication error polling component");
-      return NAN;
+    return false;
-    }
-    if ((anwser[2] & 0b10000000) == 0) {
-      // ready flag is 0, valid measurement received
-      break;
-    }
-    if (millis() - start > 100) {
-      ESP_LOGW(TAG, "Reading MCP3428 measurement timed out");
-      this->status_set_warning();
-      return NAN;
-    }
-    yield();
   }
+  if ((anwser[2] & 0b10000000) == 0) {
+    // ready flag is 0, valid measurement received
+    voltage = this->convert_anwser_to_voltage(anwser);
+    single_measurement_active_ = false;
+    this->status_clear_warning();
+    return true;
+  } else {
+    return false;
+  }
+}
 
-  // got valid measurement prepare tick size
+float MCP3428Component::convert_anwser_to_voltage(uint8_t *anwser) {
-  float tick_voltage = 2.048f / 32768;  // ref voltage 2.048V/non-sign bits, default 15 bits
+  uint8_t config_resolution = (this->prev_config_ >> 2) & 0b00000011;
-  switch (resolution) {
+  uint8_t config_gain = this->prev_config_ & 0b00000011;
+
+  float tick_voltage = 2.048f / 32768;  // ref voltage 2.048V/non-sign bits, default 16 bits
+  switch (config_resolution) {
     case MCP3428Resolution::MCP3428_12_BITS:
       tick_voltage *= 16;
       break;
@@ -113,7 +149,7 @@ float MCP3428Component::request_measurement(MCP3428Multiplexer multiplexer, MCP3
     default:  // nothing to do for 16 bit
       break;
   }
-  switch (gain) {
+  switch (config_gain) {
     case MCP3428Gain::MCP3428_GAIN_2:
       tick_voltage /= 2;
       break;
@@ -126,10 +162,9 @@ float MCP3428Component::request_measurement(MCP3428Multiplexer multiplexer, MCP3
     default:
       break;
   }
+
   // convert code (first 2 bytes of cleaned up anwser) into voltage ticks
   int16_t ticks = anwser[0] << 8 | anwser[1];
-
-  this->status_clear_warning();
   return tick_voltage * ticks;
 }
 

esphome/components/mcp3428/mcp3428.h

@@ -27,6 +27,10 @@ enum MCP3428Resolution {
   MCP3428_16_BITS = 0b10,
 };
 
+static const uint32_t MEASUREMENT_TIME_12BIT_MS = 5;
+static const uint32_t MEASUREMENT_TIME_14BIT_MS = 17;
+static const uint32_t MEASUREMENT_TIME_16BIT_MS = 67;
+
 class MCP3428Component : public Component, public i2c::I2CDevice {
  public:
   void setup() override;
@@ -35,12 +39,24 @@ class MCP3428Component : public Component, public i2c::I2CDevice {
   float get_setup_priority() const override { return setup_priority::DATA; }
   void set_continuous_mode(bool continuous_mode) { continuous_mode_ = continuous_mode; }
 
-  /// Helper method to request a measurement from a sensor.
+  // Helper method to request a measurement from a sensor. Returns true if measurement is started and false if sensor is
-  float request_measurement(MCP3428Multiplexer multiplexer, MCP3428Gain gain, MCP3428Resolution resolution);
+  // busy. Due to asyncronous measurement will return a best guess as to the necessary wait time for either request
+  // retry or polling.
+  bool request_measurement(MCP3428Multiplexer multiplexer, MCP3428Gain gain, MCP3428Resolution resolution,
+                           uint32_t &timeout_wait);
+  // poll component for a measurement. Returns true if value is available and sets voltage to the result.
+  bool poll_result(float &voltage);
+
+  void abandon_current_measurement() { single_measurement_active_ = false; }
 
  protected:
+  float convert_anwser_to_voltage(uint8_t *anwser);
+
   uint8_t prev_config_{0};
+  uint32_t last_config_write_ms_{0};
   bool continuous_mode_;
+
+  bool single_measurement_active_;
 };
 
 }  // namespace mcp3428

esphome/components/mcp3428/sensor/mcp3428_sensor.cpp

@@ -6,17 +6,51 @@ namespace esphome {
 namespace mcp3428 {
 
 static const char *const TAG = "mcp3426/7/8.sensor";
+static const uint8_t MEASUREMENT_INITIATE_MAX_TRIES = 10;
 
 float MCP3428Sensor::sample() {
-  return this->parent_->request_measurement(this->multiplexer_, this->gain_, this->resolution_);
+  uint32_t wait = 0;
+  float res = NAN;
+  // initiate Measurement
+  if (!this->parent_->request_measurement(this->multiplexer_, this->gain_, this->resolution_, wait)) {
+    return res;  // if sensor is busy there is no easy way the situation can be resolved in a synchronous manner
+  }
+  delay(wait);  // certainly not ideal but necessary when the result needs to be returned now
+
+  bool success = this->parent_->poll_result(res);
+  if ((!success) || std::isnan(res)) {
+    this->parent_->abandon_current_measurement();
+  } else {
+    this->status_clear_warning();
+  }
+  return res;
 }
 
 void MCP3428Sensor::update() {
-  float v = this->sample();
+  this->set_retry(MEASUREMENT_TIME_16BIT_MS, MEASUREMENT_INITIATE_MAX_TRIES,
-  if (!std::isnan(v)) {
+                  [this](const uint8_t remaining_initiate_attempts) {
-    ESP_LOGD(TAG, "'%s': Got Voltage=%fV", this->get_name().c_str(), v);
+                    uint32_t wait;
-    this->publish_state(v);
+                    if (this->parent_->request_measurement(this->multiplexer_, this->gain_, this->resolution_, wait)) {
-  }
+                      // measurement started, set timeout for retrieving value
+                      this->set_timeout(wait, [this]() {
+                        float res = NAN;
+                        bool success = this->parent_->poll_result(res);
+                        if (success && !std::isnan(res)) {
+                          ESP_LOGD(TAG, "'%s': Got Voltage=%fV", this->get_name().c_str(), res);
+                          this->publish_state(res);
+                          this->status_clear_warning();
+                        } else {
+                          this->status_set_warning("No valid measurement returned");
+                          this->parent_->abandon_current_measurement();
+                        }
+                      });
+                      return RetryResult::DONE;
+                    }
+                    if (remaining_initiate_attempts == 0) {
+                      this->status_set_warning("Could not initiate Measurement");
+                    }
+                    return RetryResult::RETRY;
+                  });
 }
 
 void MCP3428Sensor::dump_config() {

esphome/components/mcp3428/sensor/mcp3428_sensor.h

@@ -21,11 +21,14 @@ class MCP3428Sensor : public sensor::Sensor,
   void set_multiplexer(MCP3428Multiplexer multiplexer) { this->multiplexer_ = multiplexer; }
   void set_gain(MCP3428Gain gain) { this->gain_ = gain; }
   void set_resolution(MCP3428Resolution resolution) { this->resolution_ = resolution; }
+  // the sample function should ONLY be used when one channel is read in continuous mode or 12 bit conversion is used as
+  // it blocks!
   float sample() override;
 
   void dump_config() override;
 
  protected:
+  int initial_measurement_request_ms_;
   MCP3428Multiplexer multiplexer_;
   MCP3428Gain gain_;
   MCP3428Resolution resolution_;