[Modbus Controller] Added preference to change command retries (#7312)

Co-authored-by: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com>
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>

esphome/components/modbus_controller/__init__.py

@@ -21,6 +21,7 @@ from .const import (
     CONF_CUSTOM_COMMAND,
     CONF_FORCE_NEW_RANGE,
     CONF_MODBUS_CONTROLLER_ID,
+    CONF_MAX_CMD_RETRIES,
     CONF_ON_COMMAND_SENT,
     CONF_REGISTER_COUNT,
     CONF_REGISTER_TYPE,
@@ -131,6 +132,7 @@ CONFIG_SCHEMA = cv.All(
             cv.Optional(
                 CONF_COMMAND_THROTTLE, default="0ms"
             ): cv.positive_time_period_milliseconds,
+            cv.Optional(CONF_MAX_CMD_RETRIES, default=4): cv.positive_int,
             cv.Optional(CONF_OFFLINE_SKIP_UPDATES, default=0): cv.positive_int,
             cv.Optional(
                 CONF_SERVER_REGISTERS,
@@ -257,6 +259,7 @@ async def to_code(config):
     var = cg.new_Pvariable(config[CONF_ID])
     cg.add(var.set_allow_duplicate_commands(config[CONF_ALLOW_DUPLICATE_COMMANDS]))
     cg.add(var.set_command_throttle(config[CONF_COMMAND_THROTTLE]))
+    cg.add(var.set_max_cmd_retries(config[CONF_MAX_CMD_RETRIES]))
     cg.add(var.set_offline_skip_updates(config[CONF_OFFLINE_SKIP_UPDATES]))
     if CONF_SERVER_REGISTERS in config:
         for server_register in config[CONF_SERVER_REGISTERS]:

esphome/components/modbus_controller/const.py

@@ -5,6 +5,7 @@ CONF_COMMAND_THROTTLE = "command_throttle"
 CONF_OFFLINE_SKIP_UPDATES = "offline_skip_updates"
 CONF_CUSTOM_COMMAND = "custom_command"
 CONF_FORCE_NEW_RANGE = "force_new_range"
+CONF_MAX_CMD_RETRIES = "max_cmd_retries"
 CONF_MODBUS_CONTROLLER_ID = "modbus_controller_id"
 CONF_MODBUS_FUNCTIONCODE = "modbus_functioncode"
 CONF_ON_COMMAND_SENT = "on_command_sent"

esphome/components/modbus_controller/modbus_controller.cpp

@@ -18,11 +18,11 @@ void ModbusController::setup() { this->create_register_ranges_(); }
 bool ModbusController::send_next_command_() {
   uint32_t last_send = millis() - this->last_command_timestamp_;
 
-  if ((last_send > this->command_throttle_) && !waiting_for_response() && !command_queue_.empty()) {
+  if ((last_send > this->command_throttle_) && !waiting_for_response() && !this->command_queue_.empty()) {
-    auto &command = command_queue_.front();
+    auto &command = this->command_queue_.front();
 
     // remove from queue if command was sent too often
-    if (command->send_countdown < 1) {
+    if (!command->should_retry(this->max_cmd_retries_)) {
       if (!this->module_offline_) {
         ESP_LOGW(TAG, "Modbus device=%d set offline", this->address_);
 
@@ -34,11 +34,9 @@ bool ModbusController::send_next_command_() {
         }
       }
       this->module_offline_ = true;
-      ESP_LOGD(
+      ESP_LOGD(TAG, "Modbus command to device=%d register=0x%02X no response received - removed from send queue",
-          TAG,
+               this->address_, command->register_address);
-          "Modbus command to device=%d register=0x%02X countdown=%d no response received - removed from send queue",
+      this->command_queue_.pop_front();
-          this->address_, command->register_address, command->send_countdown);
-      command_queue_.pop_front();
     } else {
       ESP_LOGV(TAG, "Sending next modbus command to device %d register 0x%02X count %d", this->address_,
                command->register_address, command->register_count);
@@ -50,11 +48,11 @@ bool ModbusController::send_next_command_() {
 
       // remove from queue if no handler is defined
       if (!command->on_data_func) {
-        command_queue_.pop_front();
+        this->command_queue_.pop_front();
       }
     }
   }
-  return (!command_queue_.empty());
+  return (!this->command_queue_.empty());
 }
 
 // Queue incoming response
@@ -77,7 +75,7 @@ void ModbusController::on_modbus_data(const std::vector<uint8_t> &data) {
     current_command->payload = data;
     this->incoming_queue_.push(std::move(current_command));
     ESP_LOGV(TAG, "Modbus response queued");
-    command_queue_.pop_front();
+    this->command_queue_.pop_front();
   }
 }
 
@@ -99,7 +97,7 @@ void ModbusController::on_modbus_error(uint8_t function_code, uint8_t exception_
              "payload size=%zu",
              function_code, current_command->register_address, current_command->register_count,
              current_command->payload.size());
-    command_queue_.pop_front();
+    this->command_queue_.pop_front();
   }
 }
 
@@ -178,7 +176,7 @@ void ModbusController::queue_command(const ModbusCommandItem &command) {
   if (!this->allow_duplicate_commands_) {
     // check if this command is already qeued.
     // not very effective but the queue is never really large
-    for (auto &item : command_queue_) {
+    for (auto &item : this->command_queue_) {
       if (item->is_equal(command)) {
         ESP_LOGW(TAG, "Duplicate modbus command found: type=0x%x address=%u count=%u",
                  static_cast<uint8_t>(command.register_type), command.register_address, command.register_count);
@@ -189,7 +187,7 @@ void ModbusController::queue_command(const ModbusCommandItem &command) {
       }
     }
   }
-  command_queue_.push_back(make_unique<ModbusCommandItem>(command));
+  this->command_queue_.push_back(make_unique<ModbusCommandItem>(command));
 }
 
 void ModbusController::update_range_(RegisterRange &r) {
@@ -224,8 +222,8 @@ void ModbusController::update_range_(RegisterRange &r) {
 // Once we get a response to the command it is removed from the queue and the next command is send
 //
 void ModbusController::update() {
-  if (!command_queue_.empty()) {
+  if (!this->command_queue_.empty()) {
-    ESP_LOGV(TAG, "%zu modbus commands already in queue", command_queue_.size());
+    ESP_LOGV(TAG, "%zu modbus commands already in queue", this->command_queue_.size());
   } else {
     ESP_LOGV(TAG, "Updating modbus component");
   }
@@ -346,6 +344,8 @@ size_t ModbusController::create_register_ranges_() {
 void ModbusController::dump_config() {
   ESP_LOGCONFIG(TAG, "ModbusController:");
   ESP_LOGCONFIG(TAG, "  Address: 0x%02X", this->address_);
+  ESP_LOGCONFIG(TAG, "  Max Command Retries: %d", this->max_cmd_retries_);
+  ESP_LOGCONFIG(TAG, "  Offline Skip Updates: %d", this->offline_skip_updates_);
 #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE
   ESP_LOGCONFIG(TAG, "sensormap");
   for (auto &it : sensorset_) {
@@ -560,8 +560,9 @@ bool ModbusCommandItem::send() {
   } else {
     modbusdevice->send_raw(this->payload);
   }
-  ESP_LOGV(TAG, "Command sent %d 0x%X %d", uint8_t(this->function_code), this->register_address, this->register_count);
+  this->send_count_++;
-  send_countdown--;
+  ESP_LOGV(TAG, "Command sent %d 0x%X %d send_count: %d", uint8_t(this->function_code), this->register_address,
+           this->register_count, this->send_count_);
   return true;
 }
 

esphome/components/modbus_controller/modbus_controller.h

@@ -312,7 +312,6 @@ struct RegisterRange {
 class ModbusCommandItem {
  public:
   static const size_t MAX_PAYLOAD_BYTES = 240;
-  static const uint8_t MAX_SEND_REPEATS = 5;
   ModbusController *modbusdevice;
   uint16_t register_address;
   uint16_t register_count;
@@ -322,9 +321,9 @@ class ModbusCommandItem {
       on_data_func;
   std::vector<uint8_t> payload = {};
   bool send();
-  // wrong commands (esp. custom commands) can block the send queue
+  /// Check if the command should be retried based on the max_retries parameter
-  // limit the number of repeats
+  bool should_retry(uint8_t max_retries) { return this->send_count_ <= max_retries; };
-  uint8_t send_countdown{MAX_SEND_REPEATS};
+
   /// factory methods
   /** Create modbus read command
    *  Function code 02-04
@@ -413,6 +412,11 @@ class ModbusCommandItem {
           &&handler = nullptr);
 
   bool is_equal(const ModbusCommandItem &other);
+
+ protected:
+  // wrong commands (esp. custom commands) can block the send queue, limit the number of repeats.
+  /// How many times this command has been sent
+  uint8_t send_count_{0};
 };
 
 /** Modbus controller class.
@@ -464,6 +468,10 @@ class ModbusController : public PollingComponent, public modbus::ModbusDevice {
   bool get_module_offline() { return module_offline_; }
   /// Set callback for commands
   void add_on_command_sent_callback(std::function<void(int, int)> &&callback);
+  /// called by esphome generated code to set the max_cmd_retries.
+  void set_max_cmd_retries(uint8_t max_cmd_retries) { this->max_cmd_retries_ = max_cmd_retries; }
+  /// get how many times a command will be (re)sent if no response is received
+  uint8_t get_max_cmd_retries() { return this->max_cmd_retries_; }
 
  protected:
   /// parse sensormap_ and create range of sequential addresses
@@ -498,6 +506,8 @@ class ModbusController : public PollingComponent, public modbus::ModbusDevice {
   bool module_offline_;
   /// how many updates to skip if module is offline
   uint16_t offline_skip_updates_;
+  /// How many times we will retry a command if we get no response
+  uint8_t max_cmd_retries_{4};
   CallbackManager<void(int, int)> command_sent_callback_{};
 };
 

tests/components/modbus_controller/test.esp32-ard.yaml

@@ -29,3 +29,4 @@ modbus_controller:
         value_type: S_DWORD_R
         read_lambda: |-
           return 42.3;
+    max_cmd_retries: 0

tests/components/modbus_controller/test.esp32-idf.yaml

@@ -13,3 +13,4 @@ modbus_controller:
     address: 0x2
     modbus_id: mod_bus1
     allow_duplicate_commands: true
+    max_cmd_retries: 10