[kuntze] Use the typed modbus read callback and queue all reads at once (#18852)

Co-authored-by: J. Nick Koston <nick@koston.org>

esphome/components/kuntze/kuntze.cpp

@@ -1,87 +1,74 @@
 #include "kuntze.h"
-#include "esphome/core/helpers.h"
 #include "esphome/core/log.h"
-#include "esphome/core/application.h"
 
 namespace esphome::kuntze {
 
 static const char *const TAG = "kuntze";
 
-static const uint16_t REGISTER[] = {4136, 4160, 4680, 6000, 4688, 4728, 5832};
+static constexpr uint16_t REGISTER_PH = 4136;
+static constexpr uint16_t REGISTER_TEMPERATURE = 4160;
+static constexpr uint16_t REGISTER_DIS1 = 4680;
+static constexpr uint16_t REGISTER_DIS2 = 6000;
+static constexpr uint16_t REGISTER_REDOX = 4688;
+static constexpr uint16_t REGISTER_EC = 4728;
+static constexpr uint16_t REGISTER_OCI = 5832;
+static constexpr uint16_t REGISTER[] = {REGISTER_PH,    REGISTER_TEMPERATURE, REGISTER_DIS1, REGISTER_DIS2,
+                                        REGISTER_REDOX, REGISTER_EC,          REGISTER_OCI};
 
-// Maximum bytes to log for Modbus responses (2 registers = 4, plus count = 5)
-static constexpr size_t KUNTZE_MAX_LOG_BYTES = 8;
+void Kuntze::on_read_holding_registers(uint16_t start_address, std::span<const uint16_t> registers,
+                                       modbus::ResponseStatus status) {
+  if (!modbus::succeeded(status) || registers.size() < 2)
+    return;
 
-void Kuntze::on_response(std::span<const uint8_t> request_pdu, std::span<const uint8_t> response_pdu) {
-  auto data = modbus::helpers::server_pdu_payload(response_pdu);
-  auto get_16bit = [&](int i) -> uint16_t { return (uint16_t(data[i * 2]) << 8) | uint16_t(data[i * 2 + 1]); };
+  // Each value is a register pair: the reading, then the number of decimal places in its low byte.
+  float value = registers[0];
+  for (uint16_t i = 0; i < (registers[1] & 0xFF); i++)
+    value /= 10.0f;
 
-  this->waiting_ = false;
-#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE
-  char hex_buf[format_hex_pretty_size(KUNTZE_MAX_LOG_BYTES)];
-#endif
-  ESP_LOGV(TAG, "Data: %s", format_hex_pretty_to(hex_buf, data.data(), data.size()));
-
-  float value = (float) get_16bit(0);
-  for (int i = 0; i < data[3]; i++)
-    value /= 10.0;
-  switch (this->state_) {
-    case 1:
+  switch (start_address) {
+    case REGISTER_PH:
       ESP_LOGD(TAG, "pH=%.1f", value);
       if (this->ph_sensor_ != nullptr)
         this->ph_sensor_->publish_state(value);
       break;
-    case 2:
+    case REGISTER_TEMPERATURE:
       ESP_LOGD(TAG, "temperature=%.1f", value);
       if (this->temperature_sensor_ != nullptr)
         this->temperature_sensor_->publish_state(value);
       break;
-    case 3:
+    case REGISTER_DIS1:
       ESP_LOGD(TAG, "DIS1=%.1f", value);
       if (this->dis1_sensor_ != nullptr)
         this->dis1_sensor_->publish_state(value);
       break;
-    case 4:
+    case REGISTER_DIS2:
       ESP_LOGD(TAG, "DIS2=%.1f", value);
       if (this->dis2_sensor_ != nullptr)
         this->dis2_sensor_->publish_state(value);
       break;
-    case 5:
+    case REGISTER_REDOX:
       ESP_LOGD(TAG, "REDOX=%.1f", value);
       if (this->redox_sensor_ != nullptr)
         this->redox_sensor_->publish_state(value);
       break;
-    case 6:
+    case REGISTER_EC:
       ESP_LOGD(TAG, "EC=%.1f", value);
       if (this->ec_sensor_ != nullptr)
         this->ec_sensor_->publish_state(value);
       break;
-    case 7:
+    case REGISTER_OCI:
       ESP_LOGD(TAG, "OCI=%.1f", value);
       if (this->oci_sensor_ != nullptr)
         this->oci_sensor_->publish_state(value);
       break;
   }
-  if (++this->state_ > 7)
-    this->state_ = 0;
 }
 
-void Kuntze::loop() {
-  uint32_t now = App.get_loop_component_start_time();
-  // timeout after 15 seconds
-  if (this->waiting_ && (now - this->last_send_ > 15000)) {
-    ESP_LOGW(TAG, "timed out waiting for response");
-    this->waiting_ = false;
-  }
-  if (this->waiting_ || (this->state_ == 0))
-    return;
-  this->last_send_ = now;
-  this->read_holding_registers(REGISTER[this->state_ - 1], 2);
-  this->waiting_ = true;
+void Kuntze::update() {
+  for (uint16_t reg : REGISTER)
+    this->read_holding_registers(reg, 2);
 }
 
-void Kuntze::update() { this->state_ = 1; }
-
 void Kuntze::dump_config() {
   ESP_LOGCONFIG(TAG,
                 "Kuntze:\n"

esphome/components/kuntze/kuntze.h

@@ -18,18 +18,14 @@ class Kuntze final : public PollingComponent, public modbus::ModbusClientDevice
   void set_ec_sensor(sensor::Sensor *ec_sensor) { ec_sensor_ = ec_sensor; }
   void set_oci_sensor(sensor::Sensor *oci_sensor) { oci_sensor_ = oci_sensor; }
 
-  void loop() override;
   void update() override;
 
-  void on_response(std::span<const uint8_t> request_pdu, std::span<const uint8_t> response_pdu) override;
+  void on_read_holding_registers(uint16_t start_address, std::span<const uint16_t> registers,
+                                 modbus::ResponseStatus status) override;
 
   void dump_config() override;
 
  protected:
-  int state_{0};
-  bool waiting_{false};
-  uint32_t last_send_{0};
-
   sensor::Sensor *ph_sensor_{nullptr};
   sensor::Sensor *temperature_sensor_{nullptr};
   sensor::Sensor *dis1_sensor_{nullptr};