From d86d1f9f52022b8ed5f99e7ba7faca822f23584c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 7 Jan 2026 08:29:28 -1000 Subject: [PATCH] [modbus_controller] Replace format_hex_pretty with stack-based format_hex_pretty_to (#12781) --- .../number/modbus_number.cpp | 10 ++++- .../output/modbus_output.cpp | 9 +++- .../switch/modbus_switch.cpp | 10 ++++- esphome/core/helpers.cpp | 44 ++++++++++++++----- esphome/core/helpers.h | 25 +++++++++++ 5 files changed, 85 insertions(+), 13 deletions(-) diff --git a/esphome/components/modbus_controller/number/modbus_number.cpp b/esphome/components/modbus_controller/number/modbus_number.cpp index ea8467d5a3..4a3ec1fc41 100644 --- a/esphome/components/modbus_controller/number/modbus_number.cpp +++ b/esphome/components/modbus_controller/number/modbus_number.cpp @@ -1,5 +1,6 @@ #include #include "modbus_number.h" +#include "esphome/core/helpers.h" #include "esphome/core/log.h" namespace esphome { @@ -7,6 +8,9 @@ namespace modbus_controller { static const char *const TAG = "modbus.number"; +// Maximum uint16_t registers to log in verbose hex output +static constexpr size_t MODBUS_NUMBER_MAX_LOG_REGISTERS = 32; + void ModbusNumber::parse_and_publish(const std::vector &data) { float result = payload_to_float(data, *this) / this->multiply_by_; @@ -47,7 +51,11 @@ void ModbusNumber::control(float value) { } if (!data.empty()) { - ESP_LOGV(TAG, "Modbus Number write raw: %s", format_hex_pretty(data).c_str()); +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE + char hex_buf[format_hex_pretty_uint16_size(MODBUS_NUMBER_MAX_LOG_REGISTERS)]; +#endif + ESP_LOGV(TAG, "Modbus Number write raw: %s", + format_hex_pretty_to(hex_buf, sizeof(hex_buf), data.data(), data.size())); write_cmd = ModbusCommandItem::create_custom_command( this->parent_, data, [this, write_cmd](ModbusRegisterType register_type, uint16_t start_address, const std::vector &data) { diff --git a/esphome/components/modbus_controller/output/modbus_output.cpp b/esphome/components/modbus_controller/output/modbus_output.cpp index 45e786a704..f02d9397ca 100644 --- a/esphome/components/modbus_controller/output/modbus_output.cpp +++ b/esphome/components/modbus_controller/output/modbus_output.cpp @@ -7,6 +7,9 @@ namespace modbus_controller { static const char *const TAG = "modbus_controller.output"; +// Maximum bytes to log in verbose hex output +static constexpr size_t MODBUS_OUTPUT_MAX_LOG_BYTES = 64; + /** Write a value to the device * */ @@ -80,7 +83,11 @@ void ModbusBinaryOutput::write_state(bool state) { } } if (!data.empty()) { - ESP_LOGV(TAG, "Modbus binary output write raw: %s", format_hex_pretty(data).c_str()); +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE + char hex_buf[format_hex_pretty_size(MODBUS_OUTPUT_MAX_LOG_BYTES)]; +#endif + ESP_LOGV(TAG, "Modbus binary output write raw: %s", + format_hex_pretty_to(hex_buf, sizeof(hex_buf), data.data(), data.size())); cmd = ModbusCommandItem::create_custom_command( this->parent_, data, [this, cmd](ModbusRegisterType register_type, uint16_t start_address, const std::vector &data) { diff --git a/esphome/components/modbus_controller/switch/modbus_switch.cpp b/esphome/components/modbus_controller/switch/modbus_switch.cpp index 21c4c1718d..68aa37c9ed 100644 --- a/esphome/components/modbus_controller/switch/modbus_switch.cpp +++ b/esphome/components/modbus_controller/switch/modbus_switch.cpp @@ -1,11 +1,15 @@ #include "modbus_switch.h" +#include "esphome/core/helpers.h" #include "esphome/core/log.h" namespace esphome { namespace modbus_controller { static const char *const TAG = "modbus_controller.switch"; +// Maximum bytes to log in verbose hex output +static constexpr size_t MODBUS_SWITCH_MAX_LOG_BYTES = 64; + void ModbusSwitch::setup() { optional initial_state = Switch::get_initial_state_with_restore_mode(); if (initial_state.has_value()) { @@ -71,7 +75,11 @@ void ModbusSwitch::write_state(bool state) { } } if (!data.empty()) { - ESP_LOGV(TAG, "Modbus Switch write raw: %s", format_hex_pretty(data).c_str()); +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE + char hex_buf[format_hex_pretty_size(MODBUS_SWITCH_MAX_LOG_BYTES)]; +#endif + ESP_LOGV(TAG, "Modbus Switch write raw: %s", + format_hex_pretty_to(hex_buf, sizeof(hex_buf), data.data(), data.size())); cmd = ModbusCommandItem::create_custom_command( this->parent_, data, [this, cmd](ModbusRegisterType register_type, uint16_t start_address, const std::vector &data) { diff --git a/esphome/core/helpers.cpp b/esphome/core/helpers.cpp index 1c68f1a021..8671dc7f82 100644 --- a/esphome/core/helpers.cpp +++ b/esphome/core/helpers.cpp @@ -332,6 +332,37 @@ char *format_hex_pretty_to(char *buffer, size_t buffer_size, const uint8_t *data return format_hex_internal(buffer, buffer_size, data, length, separator, 'A'); } +char *format_hex_pretty_to(char *buffer, size_t buffer_size, const uint16_t *data, size_t length, char separator) { + if (length == 0 || buffer_size == 0) { + if (buffer_size > 0) + buffer[0] = '\0'; + return buffer; + } + // With separator: each uint16_t needs 5 chars (4 hex + 1 sep), except last has no separator + // Without separator: each uint16_t needs 4 chars, plus null terminator + uint8_t stride = separator ? 5 : 4; + size_t max_values = separator ? (buffer_size / stride) : ((buffer_size - 1) / stride); + if (max_values == 0) { + buffer[0] = '\0'; + return buffer; + } + if (length > max_values) { + length = max_values; + } + for (size_t i = 0; i < length; i++) { + size_t pos = i * stride; + buffer[pos] = format_hex_pretty_char((data[i] & 0xF000) >> 12); + buffer[pos + 1] = format_hex_pretty_char((data[i] & 0x0F00) >> 8); + buffer[pos + 2] = format_hex_pretty_char((data[i] & 0x00F0) >> 4); + buffer[pos + 3] = format_hex_pretty_char(data[i] & 0x000F); + if (separator && i < length - 1) { + buffer[pos + 4] = separator; + } + } + buffer[length * stride - (separator ? 1 : 0)] = '\0'; + return buffer; +} + // Shared implementation for uint8_t and string hex formatting static std::string format_hex_pretty_uint8(const uint8_t *data, size_t length, char separator, bool show_length) { if (data == nullptr || length == 0) @@ -356,16 +387,9 @@ std::string format_hex_pretty(const uint16_t *data, size_t length, char separato if (data == nullptr || length == 0) return ""; std::string ret; - uint8_t multiple = separator ? 5 : 4; // 5 if separator is not \0, 4 otherwise - ret.resize(multiple * length - (separator ? 1 : 0)); - for (size_t i = 0; i < length; i++) { - ret[multiple * i] = format_hex_pretty_char((data[i] & 0xF000) >> 12); - ret[multiple * i + 1] = format_hex_pretty_char((data[i] & 0x0F00) >> 8); - ret[multiple * i + 2] = format_hex_pretty_char((data[i] & 0x00F0) >> 4); - ret[multiple * i + 3] = format_hex_pretty_char(data[i] & 0x000F); - if (separator && i != length - 1) - ret[multiple * i + 4] = separator; - } + size_t hex_len = separator ? (length * 5 - 1) : (length * 4); + ret.resize(hex_len); + format_hex_pretty_to(&ret[0], hex_len + 1, data, length, separator); if (show_length && length > 4) return ret + " (" + std::to_string(length) + ")"; return ret; diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 9916078efb..acba420d3e 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -782,6 +782,31 @@ inline char *format_hex_pretty_to(char (&buffer)[N], const uint8_t *data, size_t return format_hex_pretty_to(buffer, N, data, length, separator); } +/// Calculate buffer size needed for format_hex_pretty_to with uint16_t data: "XXXX:XXXX:...:XXXX\0" +constexpr size_t format_hex_pretty_uint16_size(size_t count) { return count * 5; } + +/** + * Format uint16_t array as uppercase hex with separator to pre-allocated buffer. + * Each uint16_t is formatted as 4 hex chars in big-endian order. + * + * @param buffer Output buffer to write to. + * @param buffer_size Size of the output buffer. + * @param data Pointer to uint16_t array. + * @param length Number of uint16_t values. + * @param separator Character to use between values, or '\0' for no separator. + * @return Pointer to buffer. + * + * Buffer size needed: length * 5 with separator (for "XXXX:XXXX\0"), length * 4 + 1 without. + */ +char *format_hex_pretty_to(char *buffer, size_t buffer_size, const uint16_t *data, size_t length, char separator = ':'); + +/// Format uint16_t array as uppercase hex with separator to buffer. Automatically deduces buffer size. +template +inline char *format_hex_pretty_to(char (&buffer)[N], const uint16_t *data, size_t length, char separator = ':') { + static_assert(N >= 5, "Buffer must hold at least one hex uint16_t"); + return format_hex_pretty_to(buffer, N, data, length, separator); +} + /// MAC address size in bytes static constexpr size_t MAC_ADDRESS_SIZE = 6; /// Buffer size for MAC address with separators: "XX:XX:XX:XX:XX:XX\0"