From edd847ea403c4a0fba05d652b8e30e09fd3e6a85 Mon Sep 17 00:00:00 2001 From: Keith Burzinski Date: Sun, 1 Dec 2024 18:27:32 -0600 Subject: [PATCH] [modbus_controller] Clang fixes (#7899) --- .../modbus_controller/modbus_controller.cpp | 40 ++++++++--------- .../modbus_controller/modbus_controller.h | 44 +++++++++---------- .../number/modbus_number.cpp | 16 +++---- .../modbus_controller/number/modbus_number.h | 8 ++-- .../output/modbus_output.cpp | 13 +++--- .../modbus_controller/output/modbus_output.h | 10 ++--- .../select/modbus_select.cpp | 7 +-- .../modbus_controller/select/modbus_select.h | 8 ++-- .../switch/modbus_switch.cpp | 8 ++-- .../modbus_controller/switch/modbus_switch.h | 4 +- 10 files changed, 80 insertions(+), 78 deletions(-) diff --git a/esphome/components/modbus_controller/modbus_controller.cpp b/esphome/components/modbus_controller/modbus_controller.cpp index f8b72af817..641ba68223 100644 --- a/esphome/components/modbus_controller/modbus_controller.cpp +++ b/esphome/components/modbus_controller/modbus_controller.cpp @@ -152,11 +152,11 @@ void ModbusController::on_modbus_read_registers(uint8_t function_code, uint16_t } SensorSet ModbusController::find_sensors_(ModbusRegisterType register_type, uint16_t start_address) const { - auto reg_it = find_if(begin(register_ranges_), end(register_ranges_), [=](RegisterRange const &r) { + auto reg_it = find_if(begin(this->register_ranges_), end(this->register_ranges_), [=](RegisterRange const &r) { return (r.start_address == start_address && r.register_type == register_type); }); - if (reg_it == register_ranges_.end()) { + if (reg_it == this->register_ranges_.end()) { ESP_LOGE(TAG, "No matching range for sensor found - start_address : 0x%X", start_address); } else { return reg_it->sensors; @@ -240,18 +240,18 @@ void ModbusController::update() { // walk through the sensors and determine the register ranges to read size_t ModbusController::create_register_ranges_() { - register_ranges_.clear(); - if (this->parent_->role == modbus::ModbusRole::CLIENT && sensorset_.empty()) { + this->register_ranges_.clear(); + if (this->parent_->role == modbus::ModbusRole::CLIENT && this->sensorset_.empty()) { ESP_LOGW(TAG, "No sensors registered"); return 0; } // iterator is sorted see SensorItemsComparator for details - auto ix = sensorset_.begin(); + auto ix = this->sensorset_.begin(); RegisterRange r = {}; uint8_t buffer_offset = 0; SensorItem *prev = nullptr; - while (ix != sensorset_.end()) { + while (ix != this->sensorset_.end()) { SensorItem *curr = *ix; ESP_LOGV(TAG, "Register: 0x%X %d %d %d offset=%u skip=%u addr=%p", curr->start_address, curr->register_count, @@ -278,12 +278,12 @@ size_t ModbusController::create_register_ranges_() { // this register can re-use the data from the previous register // remove this sensore because start_address is changed (sort-order) - ix = sensorset_.erase(ix); + ix = this->sensorset_.erase(ix); curr->start_address = r.start_address; curr->offset += prev->offset; - sensorset_.insert(curr); + this->sensorset_.insert(curr); // move iterator backwards because it will be incremented later ix--; @@ -293,14 +293,14 @@ size_t ModbusController::create_register_ranges_() { // this register can extend the current range // remove this sensore because start_address is changed (sort-order) - ix = sensorset_.erase(ix); + ix = this->sensorset_.erase(ix); curr->start_address = r.start_address; curr->offset += buffer_offset; buffer_offset += curr->get_register_size(); r.register_count += curr->register_count; - sensorset_.insert(curr); + this->sensorset_.insert(curr); // move iterator backwards because it will be incremented later ix--; @@ -327,7 +327,7 @@ size_t ModbusController::create_register_ranges_() { ix++; } else { ESP_LOGV(TAG, "Add range 0x%X %d skip:%d", r.start_address, r.register_count, r.skip_updates); - register_ranges_.push_back(r); + this->register_ranges_.push_back(r); r = {}; buffer_offset = 0; // do not increment the iterator here because the current sensor has to be re-evaluated @@ -339,10 +339,10 @@ size_t ModbusController::create_register_ranges_() { if (r.register_count > 0) { // Add the last range ESP_LOGV(TAG, "Add last range 0x%X %d skip:%d", r.start_address, r.register_count, r.skip_updates); - register_ranges_.push_back(r); + this->register_ranges_.push_back(r); } - return register_ranges_.size(); + return this->register_ranges_.size(); } void ModbusController::dump_config() { @@ -352,18 +352,18 @@ void ModbusController::dump_config() { 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_) { + for (auto &it : this->sensorset_) { ESP_LOGCONFIG(TAG, " Sensor type=%zu start=0x%X offset=0x%X count=%d size=%d", static_cast(it->register_type), it->start_address, it->offset, it->register_count, it->get_register_size()); } ESP_LOGCONFIG(TAG, "ranges"); - for (auto &it : register_ranges_) { + for (auto &it : this->register_ranges_) { ESP_LOGCONFIG(TAG, " Range type=%zu start=0x%X count=%d skip_updates=%d", static_cast(it.register_type), it.start_address, it.register_count, it.skip_updates); } ESP_LOGCONFIG(TAG, "server registers"); - for (auto &r : server_registers_) { + for (auto &r : this->server_registers_) { ESP_LOGCONFIG(TAG, " Address=0x%02X value_type=%zu register_count=%u", r->address, static_cast(r->value_type), r->register_count); } @@ -372,11 +372,11 @@ void ModbusController::dump_config() { void ModbusController::loop() { // Incoming data to process? - if (!incoming_queue_.empty()) { - auto &message = incoming_queue_.front(); + if (!this->incoming_queue_.empty()) { + auto &message = this->incoming_queue_.front(); if (message != nullptr) process_modbus_data_(message.get()); - incoming_queue_.pop(); + this->incoming_queue_.pop(); } else { // all messages processed send pending commands @@ -391,7 +391,7 @@ void ModbusController::on_write_register_response(ModbusRegisterType register_ty void ModbusController::dump_sensors_() { ESP_LOGV(TAG, "sensors"); - for (auto &it : sensorset_) { + for (auto &it : this->sensorset_) { ESP_LOGV(TAG, " Sensor start=0x%X count=%d size=%d offset=%d", it->start_address, it->register_count, it->get_register_size(), it->offset); } diff --git a/esphome/components/modbus_controller/modbus_controller.h b/esphome/components/modbus_controller/modbus_controller.h index 2a0b936bf5..dfd52e44bc 100644 --- a/esphome/components/modbus_controller/modbus_controller.h +++ b/esphome/components/modbus_controller/modbus_controller.h @@ -240,14 +240,14 @@ class SensorItem { } // Override register size for modbus devices not using 1 register for one dword void set_register_size(uint8_t register_size) { response_bytes = register_size; } - ModbusRegisterType register_type; - SensorValueType sensor_value_type; - uint16_t start_address; - uint32_t bitmask; - uint8_t offset; - uint8_t register_count; + ModbusRegisterType register_type{ModbusRegisterType::CUSTOM}; + SensorValueType sensor_value_type{SensorValueType::RAW}; + uint16_t start_address{0}; + uint32_t bitmask{0}; + uint8_t offset{0}; + uint8_t register_count{0}; uint8_t response_bytes{0}; - uint16_t skip_updates; + uint16_t skip_updates{0}; std::vector custom_data{}; bool force_new_range{false}; }; @@ -261,9 +261,9 @@ class ServerRegister { this->register_count = register_count; this->read_lambda = std::move(read_lambda); } - uint16_t address; - SensorValueType value_type; - uint8_t register_count; + uint16_t address{0}; + SensorValueType value_type{SensorValueType::RAW}; + uint8_t register_count{0}; std::function read_lambda; }; @@ -312,11 +312,11 @@ struct RegisterRange { class ModbusCommandItem { public: static const size_t MAX_PAYLOAD_BYTES = 240; - ModbusController *modbusdevice; - uint16_t register_address; - uint16_t register_count; - ModbusFunctionCode function_code; - ModbusRegisterType register_type; + ModbusController *modbusdevice{nullptr}; + uint16_t register_address{0}; + uint16_t register_count{0}; + ModbusFunctionCode function_code{ModbusFunctionCode::CUSTOM}; + ModbusRegisterType register_type{ModbusRegisterType::CUSTOM}; std::function &data)> on_data_func; std::vector payload = {}; @@ -493,23 +493,23 @@ class ModbusController : public PollingComponent, public modbus::ModbusDevice { /// Collection of all sensors for this component SensorSet sensorset_; /// Collection of all server registers for this component - std::vector server_registers_; + std::vector server_registers_{}; /// Continuous range of modbus registers - std::vector register_ranges_; + std::vector register_ranges_{}; /// Hold the pending requests to be sent std::list> command_queue_; /// modbus response data waiting to get processed std::queue> incoming_queue_; /// if duplicate commands can be sent - bool allow_duplicate_commands_; + bool allow_duplicate_commands_{false}; /// when was the last send operation - uint32_t last_command_timestamp_; + uint32_t last_command_timestamp_{0}; /// min time in ms between sending modbus commands - uint16_t command_throttle_; + uint16_t command_throttle_{0}; /// if module didn't respond the last command - bool module_offline_; + bool module_offline_{false}; /// how many updates to skip if module is offline - uint16_t offline_skip_updates_; + uint16_t offline_skip_updates_{0}; /// How many times we will retry a command if we get no response uint8_t max_cmd_retries_{4}; /// Command sent callback diff --git a/esphome/components/modbus_controller/number/modbus_number.cpp b/esphome/components/modbus_controller/number/modbus_number.cpp index 001cfb5787..ea8467d5a3 100644 --- a/esphome/components/modbus_controller/number/modbus_number.cpp +++ b/esphome/components/modbus_controller/number/modbus_number.cpp @@ -8,7 +8,7 @@ namespace modbus_controller { static const char *const TAG = "modbus.number"; void ModbusNumber::parse_and_publish(const std::vector &data) { - float result = payload_to_float(data, *this) / multiply_by_; + float result = payload_to_float(data, *this) / this->multiply_by_; // Is there a lambda registered // call it with the pre converted value and the raw data array @@ -43,7 +43,7 @@ void ModbusNumber::control(float value) { return; } } else { - write_value = multiply_by_ * write_value; + write_value = this->multiply_by_ * write_value; } if (!data.empty()) { @@ -63,21 +63,21 @@ void ModbusNumber::control(float value) { // Create and send the write command if (this->register_count == 1 && !this->use_write_multiple_) { // since offset is in bytes and a register is 16 bits we get the start by adding offset/2 - write_cmd = - ModbusCommandItem::create_write_single_command(parent_, this->start_address + this->offset / 2, data[0]); + write_cmd = ModbusCommandItem::create_write_single_command(this->parent_, this->start_address + this->offset / 2, + data[0]); } else { - write_cmd = ModbusCommandItem::create_write_multiple_command(parent_, this->start_address + this->offset / 2, - this->register_count, data); + write_cmd = ModbusCommandItem::create_write_multiple_command( + this->parent_, this->start_address + this->offset / 2, this->register_count, data); } // publish new value write_cmd.on_data_func = [this, write_cmd, value](ModbusRegisterType register_type, uint16_t start_address, const std::vector &data) { // gets called when the write command is ack'd from the device - parent_->on_write_register_response(write_cmd.register_type, start_address, data); + this->parent_->on_write_register_response(write_cmd.register_type, start_address, data); this->publish_state(value); }; } - parent_->queue_command(write_cmd); + this->parent_->queue_command(write_cmd); this->publish_state(value); } void ModbusNumber::dump_config() { LOG_NUMBER(TAG, "Modbus Number", this); } diff --git a/esphome/components/modbus_controller/number/modbus_number.h b/esphome/components/modbus_controller/number/modbus_number.h index 544d161cbc..8f77b2e014 100644 --- a/esphome/components/modbus_controller/number/modbus_number.h +++ b/esphome/components/modbus_controller/number/modbus_number.h @@ -29,7 +29,7 @@ class ModbusNumber : public number::Number, public Component, public SensorItem void parse_and_publish(const std::vector &data) override; float get_setup_priority() const override { return setup_priority::HARDWARE; } void set_parent(ModbusController *parent) { this->parent_ = parent; } - void set_write_multiply(float factor) { multiply_by_ = factor; } + void set_write_multiply(float factor) { this->multiply_by_ = factor; } using transform_func_t = std::function(ModbusNumber *, float, const std::vector &)>; using write_transform_func_t = std::function(ModbusNumber *, float, std::vector &)>; @@ -39,9 +39,9 @@ class ModbusNumber : public number::Number, public Component, public SensorItem protected: void control(float value) override; - optional transform_func_; - optional write_transform_func_; - ModbusController *parent_; + optional transform_func_{nullopt}; + optional write_transform_func_{nullopt}; + ModbusController *parent_{nullptr}; float multiply_by_{1.0}; bool use_write_multiple_{false}; }; diff --git a/esphome/components/modbus_controller/output/modbus_output.cpp b/esphome/components/modbus_controller/output/modbus_output.cpp index 79cd2d49c2..f0f6e64f10 100644 --- a/esphome/components/modbus_controller/output/modbus_output.cpp +++ b/esphome/components/modbus_controller/output/modbus_output.cpp @@ -27,7 +27,7 @@ void ModbusFloatOutput::write_state(float value) { return; } } else { - value = multiply_by_ * value; + value = this->multiply_by_ * value; } // lambda didn't set payload if (data.empty()) { @@ -40,12 +40,13 @@ void ModbusFloatOutput::write_state(float value) { // Create and send the write command ModbusCommandItem write_cmd; if (this->register_count == 1 && !this->use_write_multiple_) { - write_cmd = ModbusCommandItem::create_write_single_command(parent_, this->start_address + this->offset, data[0]); + write_cmd = + ModbusCommandItem::create_write_single_command(this->parent_, this->start_address + this->offset, data[0]); } else { - write_cmd = ModbusCommandItem::create_write_multiple_command(parent_, this->start_address + this->offset, + write_cmd = ModbusCommandItem::create_write_multiple_command(this->parent_, this->start_address + this->offset, this->register_count, data); } - parent_->queue_command(write_cmd); + this->parent_->queue_command(write_cmd); } void ModbusFloatOutput::dump_config() { @@ -90,9 +91,9 @@ void ModbusBinaryOutput::write_state(bool state) { // offset for coil and discrete inputs is the coil/register number not bytes if (this->use_write_multiple_) { std::vector states{state}; - cmd = ModbusCommandItem::create_write_multiple_coils(parent_, this->start_address + this->offset, states); + cmd = ModbusCommandItem::create_write_multiple_coils(this->parent_, this->start_address + this->offset, states); } else { - cmd = ModbusCommandItem::create_write_single_coil(parent_, this->start_address + this->offset, state); + cmd = ModbusCommandItem::create_write_single_coil(this->parent_, this->start_address + this->offset, state); } } this->parent_->queue_command(cmd); diff --git a/esphome/components/modbus_controller/output/modbus_output.h b/esphome/components/modbus_controller/output/modbus_output.h index f424671cd1..bceb97affb 100644 --- a/esphome/components/modbus_controller/output/modbus_output.h +++ b/esphome/components/modbus_controller/output/modbus_output.h @@ -25,7 +25,7 @@ class ModbusFloatOutput : public output::FloatOutput, public Component, public S void dump_config() override; void set_parent(ModbusController *parent) { this->parent_ = parent; } - void set_write_multiply(float factor) { multiply_by_ = factor; } + void set_write_multiply(float factor) { this->multiply_by_ = factor; } // Do nothing void parse_and_publish(const std::vector &data) override{}; @@ -37,9 +37,9 @@ class ModbusFloatOutput : public output::FloatOutput, public Component, public S void write_state(float value) override; optional write_transform_func_{nullopt}; - ModbusController *parent_; + ModbusController *parent_{nullptr}; float multiply_by_{1.0}; - bool use_write_multiple_; + bool use_write_multiple_{false}; }; class ModbusBinaryOutput : public output::BinaryOutput, public Component, public SensorItem { @@ -68,8 +68,8 @@ class ModbusBinaryOutput : public output::BinaryOutput, public Component, public void write_state(bool state) override; optional write_transform_func_{nullopt}; - ModbusController *parent_; - bool use_write_multiple_; + ModbusController *parent_{nullptr}; + bool use_write_multiple_{false}; }; } // namespace modbus_controller diff --git a/esphome/components/modbus_controller/select/modbus_select.cpp b/esphome/components/modbus_controller/select/modbus_select.cpp index 33cef39a18..56b8c783ed 100644 --- a/esphome/components/modbus_controller/select/modbus_select.cpp +++ b/esphome/components/modbus_controller/select/modbus_select.cpp @@ -74,12 +74,13 @@ void ModbusSelect::control(const std::string &value) { const uint16_t write_address = this->start_address + this->offset / 2; ModbusCommandItem write_cmd; if ((this->register_count == 1) && (!this->use_write_multiple_)) { - write_cmd = ModbusCommandItem::create_write_single_command(parent_, write_address, data[0]); + write_cmd = ModbusCommandItem::create_write_single_command(this->parent_, write_address, data[0]); } else { - write_cmd = ModbusCommandItem::create_write_multiple_command(parent_, write_address, this->register_count, data); + write_cmd = + ModbusCommandItem::create_write_multiple_command(this->parent_, write_address, this->register_count, data); } - parent_->queue_command(write_cmd); + this->parent_->queue_command(write_cmd); if (this->optimistic_) this->publish_state(value); diff --git a/esphome/components/modbus_controller/select/modbus_select.h b/esphome/components/modbus_controller/select/modbus_select.h index 1c046b11d0..55fb2107dd 100644 --- a/esphome/components/modbus_controller/select/modbus_select.h +++ b/esphome/components/modbus_controller/select/modbus_select.h @@ -42,12 +42,12 @@ class ModbusSelect : public Component, public select::Select, public SensorItem void control(const std::string &value) override; protected: - std::vector mapping_; - ModbusController *parent_; + std::vector mapping_{}; + ModbusController *parent_{nullptr}; bool use_write_multiple_{false}; bool optimistic_{false}; - optional transform_func_; - optional write_transform_func_; + optional transform_func_{nullopt}; + optional write_transform_func_{nullopt}; }; } // namespace modbus_controller diff --git a/esphome/components/modbus_controller/switch/modbus_switch.cpp b/esphome/components/modbus_controller/switch/modbus_switch.cpp index 3a679fbeb8..ec29eca7f8 100644 --- a/esphome/components/modbus_controller/switch/modbus_switch.cpp +++ b/esphome/components/modbus_controller/switch/modbus_switch.cpp @@ -80,18 +80,18 @@ void ModbusSwitch::write_state(bool state) { // offset for coil and discrete inputs is the coil/register number not bytes if (this->use_write_multiple_) { std::vector states{state}; - cmd = ModbusCommandItem::create_write_multiple_coils(parent_, this->start_address + this->offset, states); + cmd = ModbusCommandItem::create_write_multiple_coils(this->parent_, this->start_address + this->offset, states); } else { - cmd = ModbusCommandItem::create_write_single_coil(parent_, this->start_address + this->offset, state); + cmd = ModbusCommandItem::create_write_single_coil(this->parent_, this->start_address + this->offset, state); } } else { // since offset is in bytes and a register is 16 bits we get the start by adding offset/2 if (this->use_write_multiple_) { std::vector bool_states(1, state ? (0xFFFF & this->bitmask) : 0); - cmd = ModbusCommandItem::create_write_multiple_command(parent_, this->start_address + this->offset / 2, 1, + cmd = ModbusCommandItem::create_write_multiple_command(this->parent_, this->start_address + this->offset / 2, 1, bool_states); } else { - cmd = ModbusCommandItem::create_write_single_command(parent_, this->start_address + this->offset / 2, + cmd = ModbusCommandItem::create_write_single_command(this->parent_, this->start_address + this->offset / 2, state ? 0xFFFF & this->bitmask : 0u); } } diff --git a/esphome/components/modbus_controller/switch/modbus_switch.h b/esphome/components/modbus_controller/switch/modbus_switch.h index bfe46f3ac8..fe4b7c1ad5 100644 --- a/esphome/components/modbus_controller/switch/modbus_switch.h +++ b/esphome/components/modbus_controller/switch/modbus_switch.h @@ -40,8 +40,8 @@ class ModbusSwitch : public Component, public switch_::Switch, public SensorItem void set_use_write_mutiple(bool use_write_multiple) { this->use_write_multiple_ = use_write_multiple; } protected: - ModbusController *parent_; - bool use_write_multiple_; + ModbusController *parent_{nullptr}; + bool use_write_multiple_{false}; optional publish_transform_func_{nullopt}; optional write_transform_func_{nullopt}; };