[modbus_controller] Replace format_hex_pretty with stack-based format_hex_pretty_to - #12781
Merged
Conversation
Contributor
|
To use the changes in this PR: # Clone the repository:
git clone https://github.com/esphome/esphome
cd esphome
# Checkout the PR branch:
git fetch origin pull/12781/head:modbus_controller
git checkout modbus_controller
# Install the development version:
script/setup
# Activate the development version:
source venv/bin/activateNow you can run (Added by the PR bot) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #12781 +/- ##
=======================================
Coverage 73.48% 73.48%
=======================================
Files 53 53
Lines 11313 11313
Branches 1535 1535
=======================================
Hits 8313 8313
Misses 2602 2602
Partials 398 398 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Memory Impact AnalysisComponents:
📊 Component Memory Breakdown
🔍 Symbol-Level Changes (click to expand)Changed Symbols
New Symbols (top 15)
Removed Symbols (top 15)
This analysis runs automatically when components change. Memory usage is measured from a representative test configuration. |
Contributor
|
👋 Hi there! This PR modifies 5 file(s) with codeowners. @martgras, @esphome/core - As codeowner(s) of the affected files, your review would be appreciated! 🙏 Note: Automatic review request may have failed, but you're still welcome to review. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes verbose logging in the modbus_controller component by replacing heap-allocating format_hex_pretty(...).c_str() calls with stack-based format_hex_pretty_to() calls. The changes introduce a new format_hex_pretty_to() overload for uint16_t* arrays and refactor the existing format_hex_pretty() for uint16_t to use the new function.
Key changes:
- Adds new
format_hex_pretty_to()overload for uint16_t arrays with corresponding helper functionformat_hex_pretty_uint16_size() - Refactors existing
format_hex_pretty()for uint16_t to delegate to the new buffer-based function - Updates three modbus component files to use stack-allocated buffers with compile guards for verbose logging
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| esphome/core/helpers.h | Declares new format_hex_pretty_to() overload for uint16_t arrays and size calculation helper |
| esphome/core/helpers.cpp | Implements uint16_t version of format_hex_pretty_to() and refactors format_hex_pretty() to use it |
| esphome/components/modbus_controller/switch/modbus_switch.cpp | Replaces heap allocation with stack buffer for verbose logging of uint8_t data |
| esphome/components/modbus_controller/output/modbus_output.cpp | Replaces heap allocation with stack buffer for verbose logging of uint8_t data |
| esphome/components/modbus_controller/number/modbus_number.cpp | Replaces heap allocation with stack buffer for verbose logging of uint16_t data |
swoboda1337
approved these changes
Jan 7, 2026
Member
Author
|
thanks |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Replace heap-allocating
format_hex_pretty(...).c_str()with stack-basedformat_hex_pretty_to()in the modbus_controller component's verbose logging.This eliminates
std::stringheap allocations in the LOGV paths for:modbus_number.cpp- Number write raw logging (usesuint16_tdata)modbus_output.cpp- Binary output write raw logging (usesuint8_tdata)modbus_switch.cpp- Switch write raw logging (usesuint8_tdata)Also adds a new
format_hex_pretty_to()overload foruint16_t*arrays to support the modbus_number case, which formats each 16-bit value as 4 hex chars in big-endian order (matching the existingformat_hex_prettybehavior). The existingformat_hex_prettyfor uint16_t now wraps this new function.The buffers are wrapped in compile guards to avoid stack allocation when verbose logging is disabled.
Buffer sizes:
modbus_number.cpp: 160 bytes (format_hex_pretty_uint16_size(32)) for up to 32 registersmodbus_output.cpp/modbus_switch.cpp: 192 bytes (format_hex_pretty_size(64)) for up to 64 bytesTypes of changes
Related issue or feature (if applicable):
Pull request in esphome-docs with documentation (if applicable):
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).If user exposed functionality or configuration variables are added/changed: