[core] Move comment to PROGMEM on ESP8266 (#12554)

This commit is contained in:
J. Nick Koston
2025-12-22 11:37:51 -10:00
committed by GitHub
parent bdbe72b7f1
commit f238f93312
7 changed files with 105 additions and 30 deletions

View File

@@ -287,7 +287,9 @@ std::string WebServer::get_config_json() {
JsonObject root = builder.root();
root[ESPHOME_F("title")] = App.get_friendly_name().empty() ? App.get_name() : App.get_friendly_name();
root[ESPHOME_F("comment")] = App.get_comment_ref();
char comment_buffer[ESPHOME_COMMENT_SIZE];
App.get_comment_string(comment_buffer);
root[ESPHOME_F("comment")] = comment_buffer;
#if defined(USE_WEBSERVER_OTA_DISABLED) || !defined(USE_WEBSERVER_OTA)
root[ESPHOME_F("ota")] = false; // Note: USE_WEBSERVER_OTA_DISABLED only affects web_server, not captive_portal
#else

View File

@@ -12,6 +12,7 @@
#include "esphome/core/hal.h"
#include "esphome/core/helpers.h"
#include "esphome/core/preferences.h"
#include "esphome/core/progmem.h"
#include "esphome/core/scheduler.h"
#include "esphome/core/string_ref.h"
#include "esphome/core/version.h"
@@ -107,8 +108,7 @@ static const uint32_t TEARDOWN_TIMEOUT_REBOOT_MS = 1000; // 1 second for quick
class Application {
public:
void pre_setup(const std::string &name, const std::string &friendly_name, const char *comment,
bool name_add_mac_suffix) {
void pre_setup(const std::string &name, const std::string &friendly_name, bool name_add_mac_suffix) {
arch_init();
this->name_add_mac_suffix_ = name_add_mac_suffix;
if (name_add_mac_suffix) {
@@ -127,7 +127,6 @@ class Application {
this->name_ = name;
this->friendly_name_ = friendly_name;
}
this->comment_ = comment;
}
#ifdef USE_DEVICES
@@ -264,10 +263,19 @@ class Application {
return "";
}
/// Get the comment of this Application set by pre_setup().
std::string get_comment() const { return this->comment_; }
/// Get the comment as StringRef (avoids allocation)
StringRef get_comment_ref() const { return StringRef(this->comment_); }
/// Copy the comment string into the provided buffer
/// Buffer must be ESPHOME_COMMENT_SIZE bytes (compile-time enforced)
void get_comment_string(std::span<char, ESPHOME_COMMENT_SIZE> buffer) {
ESPHOME_strncpy_P(buffer.data(), ESPHOME_COMMENT_STR, buffer.size());
buffer[buffer.size() - 1] = '\0';
}
/// Get the comment of this Application as a string
std::string get_comment() {
char buffer[ESPHOME_COMMENT_SIZE];
this->get_comment_string(buffer);
return std::string(buffer);
}
bool is_name_add_mac_suffix_enabled() const { return this->name_add_mac_suffix_; }
@@ -513,7 +521,6 @@ class Application {
// Pointer-sized members first
Component *current_component_{nullptr};
const char *comment_{nullptr};
// std::vector (3 pointers each: begin, end, capacity)
// Partitioned vector design for looping components

View File

@@ -7,4 +7,6 @@
#define ESPHOME_CONFIG_HASH 0x12345678U // NOLINT
#define ESPHOME_BUILD_TIME 1700000000 // NOLINT
#define ESPHOME_COMMENT_SIZE 1 // NOLINT
static const char ESPHOME_BUILD_TIME_STR[] = "2024-01-01 00:00:00 +0000";
static const char ESPHOME_COMMENT_STR[] = "";

View File

@@ -209,7 +209,7 @@ CONFIG_SCHEMA = cv.All(
cv.Required(CONF_NAME): cv.valid_name,
cv.Optional(CONF_FRIENDLY_NAME, ""): cv.All(cv.string, cv.Length(max=120)),
cv.Optional(CONF_AREA): validate_area_config,
cv.Optional(CONF_COMMENT): cv.string,
cv.Optional(CONF_COMMENT): cv.All(cv.string, cv.Length(max=255)),
cv.Required(CONF_BUILD_PATH): cv.string,
cv.Optional(CONF_PLATFORMIO_OPTIONS, default={}): cv.Schema(
{
@@ -505,7 +505,6 @@ async def to_code(config: ConfigType) -> None:
cg.App.pre_setup(
config[CONF_NAME],
config[CONF_FRIENDLY_NAME],
config.get(CONF_COMMENT, ""),
config[CONF_NAME_ADD_MAC_SUFFIX],
)
)

View File

@@ -21,6 +21,7 @@ from esphome.const import (
from esphome.core import CORE, EsphomeError
from esphome.helpers import (
copy_file_if_changed,
cpp_string_escape,
get_str_env,
is_ha_addon,
read_file,
@@ -271,7 +272,7 @@ def copy_src_tree():
"esphome", "core", "build_info_data.h"
)
build_info_json_path = CORE.relative_build_path("build_info.json")
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
# Defensively force a rebuild if the build_info files don't exist, or if
# there was a config change which didn't actually cause a source change
@@ -292,7 +293,9 @@ def copy_src_tree():
if sources_changed:
write_file(
build_info_data_h_path,
generate_build_info_data_h(config_hash, build_time, build_time_str),
generate_build_info_data_h(
config_hash, build_time, build_time_str, comment
),
)
write_file(
build_info_json_path,
@@ -332,31 +335,39 @@ def generate_version_h():
)
def get_build_info() -> tuple[int, int, str]:
def get_build_info() -> tuple[int, int, str, str]:
"""Calculate build_info values from current config.
Returns:
Tuple of (config_hash, build_time, build_time_str)
Tuple of (config_hash, build_time, build_time_str, comment)
"""
config_hash = CORE.config_hash
build_time = int(time.time())
build_time_str = time.strftime("%Y-%m-%d %H:%M:%S %z", time.localtime(build_time))
return config_hash, build_time, build_time_str
comment = CORE.comment or ""
return config_hash, build_time, build_time_str, comment
def generate_build_info_data_h(
config_hash: int, build_time: int, build_time_str: str
config_hash: int, build_time: int, build_time_str: str, comment: str
) -> str:
"""Generate build_info_data.h header with config hash and build time."""
"""Generate build_info_data.h header with config hash, build time, and comment."""
# cpp_string_escape returns '"escaped"', slice off the quotes since template has them
escaped_comment = cpp_string_escape(comment)[1:-1]
# +1 for null terminator
comment_size = len(comment) + 1
return f"""#pragma once
// Auto-generated build_info data
#define ESPHOME_CONFIG_HASH 0x{config_hash:08x}U // NOLINT
#define ESPHOME_BUILD_TIME {build_time} // NOLINT
#define ESPHOME_COMMENT_SIZE {comment_size} // NOLINT
#ifdef USE_ESP8266
#include <pgmspace.h>
static const char ESPHOME_BUILD_TIME_STR[] PROGMEM = "{build_time_str}";
static const char ESPHOME_COMMENT_STR[] PROGMEM = "{escaped_comment}";
#else
static const char ESPHOME_BUILD_TIME_STR[] = "{build_time_str}";
static const char ESPHOME_COMMENT_STR[] = "{escaped_comment}";
#endif
"""

View File

@@ -12,7 +12,7 @@
using namespace esphome;
void setup() {
App.pre_setup("livingroom", "LivingRoom", "comment", false);
App.pre_setup("livingroom", "LivingRoom", false);
auto *log = new logger::Logger(115200, 512); // NOLINT
log->pre_setup();
log->set_uart_selection(logger::UART_SELECTION_UART0);

View File

@@ -1186,8 +1186,9 @@ def test_get_build_info_new_build(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0x12345678
mock_core.comment = "Test comment"
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
assert config_hash == 0x12345678
assert isinstance(build_time, int)
@@ -1195,6 +1196,7 @@ def test_get_build_info_new_build(
assert isinstance(build_time_str, str)
# Verify build_time_str format matches expected pattern
assert len(build_time_str) >= 19 # e.g., "2025-12-15 16:27:44 +0000"
assert comment == "Test comment"
@patch("esphome.writer.CORE")
@@ -1206,6 +1208,7 @@ def test_get_build_info_always_returns_current_time(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0x12345678
mock_core.comment = ""
# Create existing build_info.json with matching config_hash and version
existing_build_time = 1700000000
@@ -1222,7 +1225,7 @@ def test_get_build_info_always_returns_current_time(
)
with patch("esphome.writer.__version__", "2025.1.0-dev"):
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
assert config_hash == 0x12345678
# get_build_info now always returns current time
@@ -1240,6 +1243,7 @@ def test_get_build_info_config_changed(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0xABCDEF00 # Different from existing
mock_core.comment = ""
# Create existing build_info.json with different config_hash
existing_build_time = 1700000000
@@ -1255,7 +1259,7 @@ def test_get_build_info_config_changed(
)
with patch("esphome.writer.__version__", "2025.1.0-dev"):
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
assert config_hash == 0xABCDEF00
assert build_time != existing_build_time # New time generated
@@ -1271,6 +1275,7 @@ def test_get_build_info_version_changed(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0x12345678
mock_core.comment = ""
# Create existing build_info.json with different version
existing_build_time = 1700000000
@@ -1286,7 +1291,7 @@ def test_get_build_info_version_changed(
)
with patch("esphome.writer.__version__", "2025.1.0-dev"): # New version
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
assert config_hash == 0x12345678
assert build_time != existing_build_time # New time generated
@@ -1302,11 +1307,12 @@ def test_get_build_info_invalid_json(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0x12345678
mock_core.comment = ""
# Create invalid JSON file
build_info_path.write_text("not valid json {{{")
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
assert config_hash == 0x12345678
assert isinstance(build_time, int)
@@ -1322,12 +1328,13 @@ def test_get_build_info_missing_keys(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0x12345678
mock_core.comment = ""
# Create JSON with missing keys
build_info_path.write_text(json.dumps({"config_hash": 0x12345678}))
with patch("esphome.writer.__version__", "2025.1.0-dev"):
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
assert config_hash == 0x12345678
assert isinstance(build_time, int)
@@ -1343,8 +1350,9 @@ def test_get_build_info_build_time_str_format(
build_info_path = tmp_path / "build_info.json"
mock_core.relative_build_path.return_value = build_info_path
mock_core.config_hash = 0x12345678
mock_core.comment = ""
config_hash, build_time, build_time_str = get_build_info()
config_hash, build_time, build_time_str, comment = get_build_info()
# Verify the format matches "%Y-%m-%d %H:%M:%S %z"
# e.g., "2025-12-15 16:27:44 +0000"
@@ -1357,36 +1365,73 @@ def test_generate_build_info_data_h_format() -> None:
config_hash = 0x12345678
build_time = 1700000000
build_time_str = "2023-11-14 22:13:20 +0000"
comment = "Test comment"
result = generate_build_info_data_h(config_hash, build_time, build_time_str)
result = generate_build_info_data_h(
config_hash, build_time, build_time_str, comment
)
assert "#pragma once" in result
assert "#define ESPHOME_CONFIG_HASH 0x12345678U" in result
assert "#define ESPHOME_BUILD_TIME 1700000000" in result
assert "#define ESPHOME_COMMENT_SIZE 13" in result # len("Test comment") + 1
assert 'ESPHOME_BUILD_TIME_STR[] = "2023-11-14 22:13:20 +0000"' in result
assert 'ESPHOME_COMMENT_STR[] = "Test comment"' in result
def test_generate_build_info_data_h_esp8266_progmem() -> None:
"""Test generate_build_info_data_h includes PROGMEM for ESP8266."""
result = generate_build_info_data_h(0xABCDEF01, 1700000000, "test")
result = generate_build_info_data_h(0xABCDEF01, 1700000000, "test", "comment")
# Should have ESP8266 PROGMEM conditional
assert "#ifdef USE_ESP8266" in result
assert "#include <pgmspace.h>" in result
assert "PROGMEM" in result
# Both build time and comment should have PROGMEM versions
assert 'ESPHOME_BUILD_TIME_STR[] PROGMEM = "test"' in result
assert 'ESPHOME_COMMENT_STR[] PROGMEM = "comment"' in result
def test_generate_build_info_data_h_hash_formatting() -> None:
"""Test generate_build_info_data_h formats hash with leading zeros."""
# Test with small hash value that needs leading zeros
result = generate_build_info_data_h(0x00000001, 0, "test")
result = generate_build_info_data_h(0x00000001, 0, "test", "")
assert "#define ESPHOME_CONFIG_HASH 0x00000001U" in result
# Test with larger hash value
result = generate_build_info_data_h(0xFFFFFFFF, 0, "test")
result = generate_build_info_data_h(0xFFFFFFFF, 0, "test", "")
assert "#define ESPHOME_CONFIG_HASH 0xffffffffU" in result
def test_generate_build_info_data_h_comment_escaping() -> None:
r"""Test generate_build_info_data_h properly escapes special characters in comment.
Uses cpp_string_escape which outputs octal escapes for special characters:
- backslash (ASCII 92) -> \134
- double quote (ASCII 34) -> \042
- newline (ASCII 10) -> \012
"""
# Test backslash escaping (ASCII 92 = octal 134)
result = generate_build_info_data_h(0, 0, "test", "backslash\\here")
assert 'ESPHOME_COMMENT_STR[] = "backslash\\134here"' in result
# Test quote escaping (ASCII 34 = octal 042)
result = generate_build_info_data_h(0, 0, "test", 'has "quotes"')
assert 'ESPHOME_COMMENT_STR[] = "has \\042quotes\\042"' in result
# Test newline escaping (ASCII 10 = octal 012)
result = generate_build_info_data_h(0, 0, "test", "line1\nline2")
assert 'ESPHOME_COMMENT_STR[] = "line1\\012line2"' in result
def test_generate_build_info_data_h_empty_comment() -> None:
"""Test generate_build_info_data_h handles empty comment."""
result = generate_build_info_data_h(0, 0, "test", "")
assert "#define ESPHOME_COMMENT_SIZE 1" in result # Just null terminator
assert 'ESPHOME_COMMENT_STR[] = ""' in result
@patch("esphome.writer.CORE")
@patch("esphome.writer.iter_components")
@patch("esphome.writer.walk_files")
@@ -1445,6 +1490,7 @@ def test_copy_src_tree_writes_build_info_files(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF
mock_core.comment = "Test comment"
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = [("core", mock_component)]
@@ -1466,6 +1512,8 @@ def test_copy_src_tree_writes_build_info_files(
assert "#define ESPHOME_CONFIG_HASH 0xdeadbeefU" in build_info_h_content
assert "#define ESPHOME_BUILD_TIME" in build_info_h_content
assert "ESPHOME_BUILD_TIME_STR" in build_info_h_content
assert "#define ESPHOME_COMMENT_SIZE" in build_info_h_content
assert "ESPHOME_COMMENT_STR" in build_info_h_content
# Verify build_info.json was written
build_info_json_path = build_path / "build_info.json"
@@ -1517,6 +1565,7 @@ def test_copy_src_tree_detects_config_hash_change(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF # Different from existing
mock_core.comment = ""
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = []
@@ -1578,6 +1627,7 @@ def test_copy_src_tree_detects_version_change(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF
mock_core.comment = ""
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = []
@@ -1627,6 +1677,7 @@ def test_copy_src_tree_handles_invalid_build_info_json(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF
mock_core.comment = ""
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = []
@@ -1700,6 +1751,7 @@ def test_copy_src_tree_build_info_timestamp_behavior(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF
mock_core.comment = ""
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = [("test", mock_component)]
@@ -1794,6 +1846,7 @@ def test_copy_src_tree_detects_removed_source_file(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF
mock_core.comment = ""
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = [] # No components = file should be removed
@@ -1855,6 +1908,7 @@ def test_copy_src_tree_ignores_removed_generated_file(
mock_core.relative_build_path.side_effect = lambda *args: build_path.joinpath(*args)
mock_core.defines = []
mock_core.config_hash = 0xDEADBEEF
mock_core.comment = ""
mock_core.target_platform = "test_platform"
mock_core.config = {}
mock_iter_components.return_value = []