PR comment fixes

esphome/components/mhz19/mhz19.cpp

@@ -18,50 +18,39 @@ uint8_t mhz19_checksum(const uint8_t *command) {
   return 0xFF - sum + 0x01;
 }
 
-static char hex_buf[MHZ19_PDU_LENGTH * 3 + 1];
-const char *dump_data_buf(const uint8_t *data) {
-  memset(hex_buf, '\0', sizeof(hex_buf));
-  for (int i = 0; i < MHZ19_PDU_LENGTH; i++) {
-    sprintf(hex_buf, "%s%0x%s", hex_buf, data[i], i == MHZ19_PDU_LENGTH - 1 ? "" : " ");
-  }
-  return hex_buf;
-}
-
-static bool setup_done;
-
 void MHZ19Component::setup() {
-  uint8_t response[MHZ19_PDU_LENGTH];
-  while (!this->mhz19_write_command_(MHZ19_COMMAND_GET_PPM, response)) {
-    delay(500);
-  }
-
-  /* MH-Z19B(s == 0) and MH-Z19(s != 0) */
-  uint8_t s = response[5];
-  if (response[5] == 0 && this->model_b_ == false) {
-    ESP_LOGD(TAG, "MH-Z19B detected");
-    this->model_b_ = true;
-  }
-
-  if (this->model_b_) {
     /*
-     * MH-Z19B allows to enable/disable 'automatic baseline calibration' (datasheet MH-Z19B v1.2),
+    By default sensor enables abc, only detect sensor version if we explicitly disabled abc
-     * disable it to prevent sensor baseline drift in not well ventilated areas
+    */
-     */
     if (this->abc_enabled_ == false) {
-      ESP_LOGI(TAG, "Disabling ABC on boot");
+      uint8_t response[MHZ19_PDU_LENGTH];
-      /* per spec response isn't expected but sensor replies anyway.
+      uint32_t start = millis();
-       * Read reply out and discard it so it won't get in the way of following commands */
+      while (!this->mhz19_write_command_(MHZ19_COMMAND_GET_PPM, response)) {
-      this->mhz19_write_command_(MHZ19_COMMAND_ABC_DISABLE, response);
+        if (millis() - start > 500) {
-    } else {
+          /* MH-Z19B(s == 0) and MH-Z19(s != 0) */
-      ESP_LOGI(TAG, "Enabling ABC on boot");
+          uint8_t s = response[5];
-      this->mhz19_write_command_(MHZ19_COMMAND_ABC_ENABLE, response);
+          if (response[5] == 0 && this->model_b_ == false) {
+            ESP_LOGD(TAG, "MH-Z19B detected");
+            this->model_b_ = true;
+          }
+        }
+        yield();
+      }
+      /*
+      Issue MHZ19_COMMAND_ABC_DISABLE only if we successfully detected model b
+      */
+      if (this->model_b_) {
+        /* per spec response isn't expected but sensor replies anyway.
+      * Read reply out and discard it so it won't get in the way of following commands */
+        this->mhz19_write_command_(MHZ19_COMMAND_ABC_DISABLE, response);
+      } else {
+        this->mhz19_write_command_(MHZ19_COMMAND_ABC_ENABLE, response);
+      }
     }
-  }
-  setup_done = true;
 }
 
 void MHZ19Component::update() {
-  if (!setup_done) {
+  if (!this->setup_done) {
     return;
   }
 
@@ -73,7 +62,7 @@ void MHZ19Component::update() {
   }
 
   if (response[0] != 0xFF || response[1] != 0x86) {
-    ESP_LOGW(TAG, "Invalid response from MHZ19! [%s]", dump_data_buf(response));
+    ESP_LOGW(TAG, "Invalid response from MHZ19!");
     this->status_set_warning();
     return;
   }
@@ -81,7 +70,7 @@ void MHZ19Component::update() {
   /* Sensor reports U(15000) during boot, ingnore reported CO2 until it boots */
   uint16_t u = (response[6] << 8) + response[7];
   if (u == 15000) {
-    ESP_LOGD(TAG, "Sensor is booting");
+    ESP_LOGD(TAG, "Sensor is booting, measurements will be available in a while");
     return;
   }
 
@@ -100,9 +89,8 @@ void MHZ19Component::update() {
 bool MHZ19Component::mhz19_write_command_(const uint8_t *command, uint8_t *response) {
   bool ret;
   int rx_error = 0;
-
+  uint32_t start = millis();
   do {
-    ESP_LOGD(TAG, "cmd [%s]", dump_data_buf(command));
     this->write_array(command, MHZ19_PDU_LENGTH);
     this->flush();
 
@@ -111,18 +99,10 @@ bool MHZ19Component::mhz19_write_command_(const uint8_t *command, uint8_t *respo
 
     memset(response, 0, MHZ19_PDU_LENGTH);
     ret = this->read_array(response, MHZ19_PDU_LENGTH);
-    ESP_LOGD(TAG, "resp [%s]", dump_data_buf(response));
 
     uint8_t checksum = mhz19_checksum(response);
     if (checksum != response[8]) {
-      ESP_LOGW(TAG, "MHZ19 Checksum doesn't match: 0x%02X!=0x%02X [%s]",
+      ESP_LOGW(TAG, "MHZ19 Checksum doesn't match: 0x%02X!=0x%02X", response[8], checksum);
-               response[8], checksum, dump_data_buf(response));
-
-      /*
-       * UART0 in NodeMCU v2, sometimes on boot has junk in RX buffer,
-       * check for it and drain all of it before sending commands to sensor
-       */
-      this->drain();
       ret = false;
       if (++rx_error > 2) {
           this->status_set_warning();
@@ -133,6 +113,7 @@ bool MHZ19Component::mhz19_write_command_(const uint8_t *command, uint8_t *respo
     } else {
       rx_error = 0;
     }
+    yield();
   } while (rx_error);
 
   return ret;

esphome/components/mhz19/mhz19.h

@@ -26,6 +26,7 @@ class MHZ19Component : public PollingComponent, public uart::UARTDevice {
   sensor::Sensor *co2_sensor_{nullptr};
   bool model_b_;
   bool abc_enabled_;
+  bool setup_done;
 };
 
 }  // namespace mhz19

esphome/components/mhz19/sensor.py

@@ -2,11 +2,11 @@ import esphome.codegen as cg
 import esphome.config_validation as cv
 from esphome.components import sensor, uart
 from esphome.const import CONF_CO2, CONF_ID, CONF_TEMPERATURE, ICON_PERIODIC_TABLE_CO2, \
-    UNIT_PARTS_PER_MILLION, UNIT_CELSIUS, ICON_THERMOMETER, UNIT_EMPTY, ICON_EMPTY
+    UNIT_PARTS_PER_MILLION, UNIT_CELSIUS, ICON_THERMOMETER
 
 DEPENDENCIES = ['uart']
 
-CONF_ABC = "automatic_baseline_calibration"
+CONF_AUTOMATIC_BASELINE_CALIBRATION = "automatic_baseline_calibration"
 mhz19_ns = cg.esphome_ns.namespace('mhz19')
 MHZ19Component = mhz19_ns.class_('MHZ19Component', cg.PollingComponent, uart.UARTDevice)
 
@@ -14,7 +14,7 @@ CONFIG_SCHEMA = cv.Schema({
     cv.GenerateID(): cv.declare_id(MHZ19Component),
     cv.Required(CONF_CO2): sensor.sensor_schema(UNIT_PARTS_PER_MILLION, ICON_PERIODIC_TABLE_CO2, 0),
     cv.Optional(CONF_TEMPERATURE): sensor.sensor_schema(UNIT_CELSIUS, ICON_THERMOMETER, 0),
-    cv.Optional(CONF_ABC, default=False): cv.boolean,
+    cv.Optional(CONF_AUTOMATIC_BASELINE_CALIBRATION, default=False): cv.boolean,
 }).extend(cv.polling_component_schema('60s')).extend(uart.UART_DEVICE_SCHEMA)
 
 
@@ -31,4 +31,4 @@ def to_code(config):
         sens = yield sensor.new_sensor(config[CONF_TEMPERATURE])
         cg.add(var.set_temperature_sensor(sens))
 
-    cg.add(var.set_abc(config[CONF_ABC]))
+    cg.add(var.set_abc(config[CONF_AUTOMATIC_BASELINE_CALIBRATION]))

esphome/components/uart/uart.cpp

@@ -98,14 +98,6 @@ void UARTComponent::flush() {
   ESP_LOGVV(TAG, "    Flushing...");
   this->hw_serial_->flush();
 }
-
-void UARTComponent::drain() {
-  for (int i = 0; this->available(); i++) {
-    uint8_t junk;
-    this->read_byte(&junk);
-    ESP_LOGVV(TAG, "Draining RX[%d]: %0x", i, junk);
-  }
-}
 #endif  // ESP32
 
 #ifdef ARDUINO_ARCH_ESP8266
@@ -238,24 +230,15 @@ void UARTComponent::flush() {
   ESP_LOGVV(TAG, "    Flushing...");
   if (this->hw_serial_ != nullptr) {
     this->hw_serial_->flush();
+    // ESP8266 HWSerial does not always flush
+    while (this->available() > 0) {
+      this->read();
+    }
   } else {
     this->sw_serial_->flush();
   }
 }
 
-void UARTComponent::drain() {
-  if (this->hw_serial_ != nullptr) {
-    for (int i = 0; this->available(); i++) {
-      uint8_t junk;
-      this->read_byte(&junk);
-      ESP_LOGVV(TAG, "Draining RX[%d]: %0x", i, junk);
-    }
-  } else {
-    this->sw_serial_->drain();
-  }
-}
-
-
 void ESP8266SoftwareSerial::setup(int8_t tx_pin, int8_t rx_pin, uint32_t baud_rate) {
   this->bit_time_ = F_CPU / baud_rate;
   if (tx_pin != -1) {
@@ -343,13 +326,6 @@ uint8_t ESP8266SoftwareSerial::peek_byte() {
 }
 void ESP8266SoftwareSerial::flush() { this->rx_in_pos_ = this->rx_out_pos_ = 0; }
 
-void ESP8266SoftwareSerial::drain() {
-  for (int i = 0; this->available(); i++) {
-    uint8_t junk = this->read_byte();
-    ESP_LOGVV(TAG, "Draining RX[%d]: %0x", i, junk);
-  }
-}
-
 int ESP8266SoftwareSerial::available() {
   int avail = int(this->rx_in_pos_) - int(this->rx_out_pos_);
   if (avail < 0)

esphome/components/uart/uart.h

@@ -16,7 +16,6 @@ class ESP8266SoftwareSerial {
   uint8_t peek_byte();
 
   void flush();
-  void drain();
 
   void write_byte(uint8_t data);
 
@@ -64,8 +63,6 @@ class UARTComponent : public Component, public Stream {
 
   void flush() override;
 
-  void drain();
-
   float get_setup_priority() const override { return setup_priority::BUS; }
 
   size_t write(uint8_t data) override;