Apply review cleanups and deprecate get_name() off ESP8266

Inline get_name_to() off ESP8266, add write_name_to() and name_equals() so
web_server avoids copies, always emit ESPHOME_PSTR, and follow the get_icon()
pattern: get_name() is a static_assert stub on ESP8266 and ESPDEPRECATED
elsewhere until 2027.5.0.
This commit is contained in:
J. Nick Koston
2026-10-08 17:18:00 -10:00
parent 4ef86fe534
commit 6f5108eb70
19 changed files with 86 additions and 94 deletions
@@ -59,9 +59,9 @@ void MQTTAlarmControlPanelComponent::setup() {
if (!root.isNull()) {
const char *state = root[ESPHOME_F("state")];
if (state == nullptr) {
ESP_LOGW(TAG, "'%s': JSON payload missing 'state' key", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGW(TAG, "'%s': JSON payload missing 'state' key", LOG_STR_ARG(this->log_name_()));
} else if (!apply_command(call, state)) {
ESP_LOGW(TAG, "'%s': Received unknown state in JSON payload: %s", LOG_STR_ARG(this->friendly_name_()), state);
ESP_LOGW(TAG, "'%s': Received unknown state in JSON payload: %s", LOG_STR_ARG(this->log_name_()), state);
} else {
const char *code = root[ESPHOME_F("code")];
if (code != nullptr) {
@@ -70,7 +70,7 @@ void MQTTAlarmControlPanelComponent::setup() {
}
}
} else if (!apply_command(call, payload.c_str())) {
ESP_LOGW(TAG, "'%s': Received unknown command payload %s", LOG_STR_ARG(this->friendly_name_()), payload.c_str());
ESP_LOGW(TAG, "'%s': Received unknown command payload %s", LOG_STR_ARG(this->log_name_()), payload.c_str());
}
call.perform();
});
+1 -1
View File
@@ -19,7 +19,7 @@ void MQTTButtonComponent::setup() {
if (payload == "PRESS") {
this->button_->press();
} else {
ESP_LOGW(TAG, "'%s': Received unknown status payload: %s", LOG_STR_ARG(this->friendly_name_()), payload.c_str());
ESP_LOGW(TAG, "'%s': Received unknown status payload: %s", LOG_STR_ARG(this->log_name_()), payload.c_str());
this->status_momentary_warning("state", 5000);
}
});
+6 -6
View File
@@ -186,11 +186,11 @@ bool MQTTComponent::send_discovery_() {
StringRef discovery_topic = this->get_discovery_topic_to_(discovery_topic_buf, discovery_info);
if (discovery_info.clean) {
ESP_LOGV(TAG, "'%s': Cleaning discovery", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGV(TAG, "'%s': Cleaning discovery", LOG_STR_ARG(this->log_name_()));
return global_mqtt_client->publish(discovery_topic.c_str(), "", 0, this->qos_, true);
}
ESP_LOGV(TAG, "'%s': Sending discovery", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGV(TAG, "'%s': Sending discovery", LOG_STR_ARG(this->log_name_()));
// NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks) false positive with ArduinoJson
return global_mqtt_client->publish_json(
@@ -208,7 +208,8 @@ bool MQTTComponent::send_discovery_() {
// Fields from EntityBase
char name_buf[ENTITY_NAME_BUF_SIZE];
root[MQTT_NAME] = this->get_entity()->has_own_name() ? this->get_entity()->get_name_to(name_buf) : StringRef();
StringRef name = this->get_entity()->get_name_to(name_buf);
root[MQTT_NAME] = this->get_entity()->has_own_name() ? name : StringRef();
if (this->is_disabled_by_default_())
root[MQTT_ENABLED_BY_DEFAULT] = false;
@@ -255,8 +256,7 @@ bool MQTTComponent::send_discovery_() {
StringRef object_id = this->get_default_object_id_to_(object_id_buf);
if (discovery_info.unique_id_generator == MQTT_MAC_ADDRESS_UNIQUE_ID_GENERATOR) {
char friendly_name_hash[9];
buf_append_printf(friendly_name_hash, sizeof(friendly_name_hash), 0, "%08" PRIx32,
fnv1_hash(this->get_entity()->get_name_to(name_buf).c_str()));
buf_append_printf(friendly_name_hash, sizeof(friendly_name_hash), 0, "%08" PRIx32, fnv1_hash(name.c_str()));
// Format: mac-component_type-hash (e.g. "aabbccddeeff-sensor-12345678")
// MAC (12) + "-" (1) + domain (max 20) + "-" (1) + hash (8) + null (1) = 43
char unique_id[MAC_ADDRESS_BUFFER_SIZE + ESPHOME_DOMAIN_MAX_LEN + 11];
@@ -408,7 +408,7 @@ void MQTTComponent::schedule_resend_state() { this->resend_state_ = true; }
bool MQTTComponent::is_connected_() const { return global_mqtt_client->is_connected(); }
// Pull these properties from EntityBase if not overridden
const LogString *MQTTComponent::friendly_name_() const { return this->get_entity()->get_log_name(); }
const LogString *MQTTComponent::log_name_() const { return this->get_entity()->get_log_name(); }
StringRef MQTTComponent::get_default_object_id_to_(std::span<char, OBJECT_ID_MAX_LEN> buf) const {
return this->get_entity()->get_object_id_to(buf);
}
+1 -1
View File
@@ -299,7 +299,7 @@ class MQTTComponent : public Component {
virtual const EntityBase *get_entity() const = 0;
/// Get the friendly name of this MQTT component.
const LogString *friendly_name_() const;
const LogString *log_name_() const;
/// Get the icon field of this component into a stack buffer
const char *get_icon_to_(std::span<char, MAX_ICON_LENGTH> buf) const { return this->get_entity()->get_icon_to(buf); }
+7 -7
View File
@@ -32,15 +32,15 @@ void MQTTFanComponent::setup() {
auto val = parse_on_off(payload.c_str());
switch (val) {
case PARSE_ON:
ESP_LOGD(TAG, "'%s' Turning Fan ON.", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s' Turning Fan ON.", LOG_STR_ARG(this->log_name_()));
this->state_->turn_on().perform();
break;
case PARSE_OFF:
ESP_LOGD(TAG, "'%s' Turning Fan OFF.", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s' Turning Fan OFF.", LOG_STR_ARG(this->log_name_()));
this->state_->turn_off().perform();
break;
case PARSE_TOGGLE:
ESP_LOGD(TAG, "'%s' Toggling Fan.", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s' Toggling Fan.", LOG_STR_ARG(this->log_name_()));
this->state_->toggle().perform();
break;
case PARSE_NONE:
@@ -56,11 +56,11 @@ void MQTTFanComponent::setup() {
auto val = parse_on_off(payload.c_str(), "forward", "reverse");
switch (val) {
case PARSE_ON:
ESP_LOGD(TAG, "'%s': Setting direction FORWARD", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s': Setting direction FORWARD", LOG_STR_ARG(this->log_name_()));
this->state_->make_call().set_direction(fan::FanDirection::FORWARD).perform();
break;
case PARSE_OFF:
ESP_LOGD(TAG, "'%s': Setting direction REVERSE", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s': Setting direction REVERSE", LOG_STR_ARG(this->log_name_()));
this->state_->make_call().set_direction(fan::FanDirection::REVERSE).perform();
break;
case PARSE_TOGGLE:
@@ -83,11 +83,11 @@ void MQTTFanComponent::setup() {
auto val = parse_on_off(payload.c_str(), "oscillate_on", "oscillate_off");
switch (val) {
case PARSE_ON:
ESP_LOGD(TAG, "'%s': Setting oscillating ON", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s': Setting oscillating ON", LOG_STR_ARG(this->log_name_()));
this->state_->make_call().set_oscillating(true).perform();
break;
case PARSE_OFF:
ESP_LOGD(TAG, "'%s': Setting oscillating OFF", LOG_STR_ARG(this->friendly_name_()));
ESP_LOGD(TAG, "'%s': Setting oscillating OFF", LOG_STR_ARG(this->log_name_()));
this->state_->make_call().set_oscillating(false).perform();
break;
case PARSE_TOGGLE:
+1 -1
View File
@@ -24,7 +24,7 @@ void MQTTLockComponent::setup() {
} else if (ESPHOME_strcasecmp_P(payload.c_str(), ESPHOME_PSTR("OPEN")) == 0) {
this->lock_->open();
} else {
ESP_LOGW(TAG, "'%s': Received unknown status payload: %s", LOG_STR_ARG(this->friendly_name_()), payload.c_str());
ESP_LOGW(TAG, "'%s': Received unknown status payload: %s", LOG_STR_ARG(this->log_name_()), payload.c_str());
this->status_momentary_warning("state", 5000);
}
});
+1 -2
View File
@@ -28,8 +28,7 @@ void MQTTSwitchComponent::setup() {
break;
case PARSE_NONE:
default:
ESP_LOGW(TAG, "'%s': Received unknown status payload: %s", LOG_STR_ARG(this->friendly_name_()),
payload.c_str());
ESP_LOGW(TAG, "'%s': Received unknown status payload: %s", LOG_STR_ARG(this->log_name_()), payload.c_str());
this->status_momentary_warning("state", 5000);
break;
}
+1 -1
View File
@@ -19,7 +19,7 @@ void MQTTUpdateComponent::setup() {
if (payload == "INSTALL") {
this->update_->perform();
} else {
ESP_LOGW(TAG, "'%s': Received unknown update payload: %s", LOG_STR_ARG(this->friendly_name_()), payload.c_str());
ESP_LOGW(TAG, "'%s': Received unknown update payload: %s", LOG_STR_ARG(this->log_name_()), payload.c_str());
this->status_momentary_warning("state", 5000);
}
});
+4 -9
View File
@@ -176,8 +176,7 @@ EntityMatchResult UrlMatch::match_entity(EntityBase *entity) const {
#endif
// Match by entity name
char name_buf[ENTITY_NAME_BUF_SIZE];
if (this->id == entity->get_name_to(name_buf)) {
if (entity->name_equals(this->id)) {
result.matched = true;
}
@@ -559,10 +558,7 @@ void WebServer::handle_js_request(AsyncWebServerRequest *request) {
// Build unique id as: {domain}/{device_name}/{entity_name} or {domain}/{entity_name}
// Uses names (not object_id) to avoid UTF-8 collision issues
static void set_json_id(JsonObject root, EntityBase *obj, const char *prefix, JsonDetail start_config) {
char name_buf[ENTITY_NAME_BUF_SIZE];
StringRef name = obj->get_name_to(name_buf);
size_t prefix_len = strlen(prefix);
size_t name_len = name.size();
#ifdef USE_DEVICES
Device *device = obj->get_device();
@@ -592,14 +588,13 @@ static void set_json_id(JsonObject root, EntityBase *obj, const char *prefix, Js
*p++ = '/';
}
#endif
memcpy(p, name.c_str(), name_len);
p[name_len] = '\0';
const char *name = p;
obj->write_name_to(p, id_buf + ID_BUF_SIZE - p);
root[ESPHOME_F("id")] = static_cast<const char *>(id_buf);
if (start_config == DETAIL_ALL) {
root[ESPHOME_F("domain")] = prefix;
// Use .c_str() to avoid instantiating set<StringRef> template (saves ~24B)
root[ESPHOME_F("name")] = name.c_str();
root[ESPHOME_F("name")] = name;
#ifdef USE_DEVICES
if (device_name) {
root[ESPHOME_F("device")] = device_name;
+18 -18
View File
@@ -11,11 +11,7 @@ namespace esphome {
ESPHOME_LOG_TAG(TAG, "entity_base");
void EntityBase::configure_entity_(const char *name, uint32_t object_id_hash, uint32_t entity_fields) {
#ifdef USE_ESP8266
this->name_ = StringRef(name, strlen_P(name)); // codegen names are in flash
#else
this->name_ = StringRef(name);
#endif
this->name_ = StringRef(name, ESPHOME_strlen_P(name));
if (this->name_.empty()) {
#ifdef USE_DEVICES
if (this->device_ != nullptr) {
@@ -129,24 +125,28 @@ void EntityBase::calc_object_id_() {
this->object_id_hash_ = fnv1_hash_object_id(name.c_str(), name.size());
}
StringRef EntityBase::get_name_to([[maybe_unused]] std::span<char, ENTITY_NAME_BUF_SIZE> buffer) const {
#ifdef USE_ESP8266
size_t len = std::min(this->name_.size(), buffer.size() - 1);
progmem_memcpy(buffer.data(), this->name_.c_str(), len);
buffer[len] = '\0';
return StringRef(buffer.data(), len);
#else
return this->name_;
#endif
size_t EntityBase::write_name_to(char *buf, size_t buf_size) const {
size_t len = std::min(this->name_.size(), buf_size - 1);
progmem_memcpy(buf, this->name_.c_str(), len);
buf[len] = '\0';
return len;
}
#ifdef USE_ESP8266
StringRef EntityBase::get_name_to(std::span<char, ENTITY_NAME_BUF_SIZE> buffer) const {
return StringRef(buffer.data(), this->write_name_to(buffer.data(), buffer.size()));
}
bool EntityBase::name_equals(const StringRef &other) const {
return other.size() == this->name_.size() && memcmp_P(other.c_str(), this->name_.c_str(), other.size()) == 0;
}
#endif
size_t EntityBase::write_object_id_to(char *buf, size_t buf_size) const {
size_t len = std::min(this->name_.size(), buf_size - 1);
const auto *name = reinterpret_cast<const uint8_t *>(this->name_.c_str());
size_t len = this->write_name_to(buf, buf_size);
for (size_t i = 0; i < len; i++) {
buf[i] = to_sanitized_char(to_snake_case_char(static_cast<char>(progmem_read_byte(name + i))));
buf[i] = to_sanitized_char(to_snake_case_char(buf[i]));
}
buf[len] = '\0';
return len;
}
+18 -8
View File
@@ -70,17 +70,27 @@ static constexpr uint8_t ENTITY_FIELD_ENTITY_CATEGORY_SHIFT = 26;
// The generic Entity base class that provides an interface common to all Entities.
class EntityBase {
public:
#ifndef USE_ESP8266
// Get the name of this Entity. Not available on ESP8266, where the name lives in flash;
// use get_name_to() or get_log_name() instead.
#ifdef USE_ESP8266
// On ESP8266 the name is in flash and cannot be read as a plain string.
template<typename T = int> const StringRef &get_name() const {
static_assert(sizeof(T) == 0, "get_name() unavailable on ESP8266 (name is in flash). "
"Use get_name_to() with a stack buffer, or get_log_name() for logging.");
return this->name_;
}
/// Get the name of this Entity, copied out of flash into buffer.
StringRef get_name_to(std::span<char, ENTITY_NAME_BUF_SIZE> buffer) const;
bool name_equals(const StringRef &other) const;
#else
ESPDEPRECATED("Use get_name_to() or get_log_name() instead. Will be removed in ESPHome 2027.5.0", "2026.11.0")
const StringRef &get_name() const { return this->name_; }
StringRef get_name_to(std::span<char, ENTITY_NAME_BUF_SIZE> /*buffer*/) const { return this->name_; }
bool name_equals(const StringRef &other) const { return this->name_ == other; }
#endif
/// Get the name of this Entity. On ESP8266 it is copied out of flash into buffer;
/// elsewhere the stored name is returned and buffer is unused.
StringRef get_name_to(std::span<char, ENTITY_NAME_BUF_SIZE> buffer) const;
/// Copy the name into buf (null terminated), returns its length.
size_t write_name_to(char *buf, size_t buf_size) const;
/// Get the name for a "%s" log argument (LOG_STR_ARG); may point to flash on ESP8266.
/// Get the name for a "%s" log argument (LOG_STR_ARG).
const LogString *get_log_name() const { return reinterpret_cast<const LogString *>(this->name_.c_str()); }
// Get whether this Entity has its own name or it should use the device friendly_name.
@@ -225,7 +235,7 @@ class EntityBase {
void calc_object_id_();
StringRef name_; // May point to flash on ESP8266; only pass it to "%s" log arguments there
StringRef name_; // Points to flash on ESP8266; read it with progmem helpers or as a "%s" log argument
uint32_t object_id_hash_{};
#ifdef USE_DEVICES
Device *device_{};
+2 -3
View File
@@ -330,9 +330,8 @@ def finalize_entity_strings(var: MockObj, config: ConfigType) -> None:
)
# Build inline comment describing the packed flags for readability
comment = _describe_packed_flags(config, entity_category)
if entity_name and CORE.is_esp8266:
# The C++ side reads entity names with flash-safe accessors on ESP8266
entity_name = RawExpression(f"PSTR({cpp_string_escape(entity_name)})")
if entity_name:
entity_name = RawExpression(f"ESPHOME_PSTR({cpp_string_escape(entity_name)})")
register_method = config.get(_KEY_REGISTER_METHOD)
if register_method is not None:
expr = getattr(App, f"register_{register_method}")(
+2 -1
View File
@@ -719,7 +719,8 @@ def blob_in(main_cpp: str) -> bytes:
def entity_hash(main_cpp: str, variable: str) -> int:
"""The key the generated code passes to configure_entity_, which ListEntities then sends."""
match = re.search(
rf'App\.register_switch\({variable}, "[^"]*", (\d+)UL, \d+\)', main_cpp
rf'App\.register_switch\({variable}, ESPHOME_PSTR\("[^"]*"\), (\d+)UL, \d+\)',
main_cpp,
)
assert match is not None, variable
return int(match.group(1))
@@ -31,7 +31,7 @@ def test_binary_sensor_sets_mandatory_fields(generate_main):
)
# Then
assert 'App.register_binary_sensor(bs_1, "test bs1",' in main_cpp
assert 'App.register_binary_sensor(bs_1, ESPHOME_PSTR("test bs1"),' in main_cpp
assert "bs_1->set_pin(" in main_cpp
+1 -1
View File
@@ -29,7 +29,7 @@ def test_button_sets_mandatory_fields(generate_main):
main_cpp = generate_main("tests/component_tests/button/test_button.yaml")
# Then
assert 'App.register_button(wol_1, "wol_test_1",' in main_cpp
assert 'App.register_button(wol_1, ESPHOME_PSTR("wol_test_1"),' in main_cpp
assert "wol_2->set_macaddr(18, 52, 86, 120, 144, 171);" in main_cpp
+2 -2
View File
@@ -16,12 +16,12 @@ def extract_packed_value(main_cpp: str, var_name: str) -> int:
escaped_var = re.escape(var_name)
legacy_pattern = (
rf"{escaped_var}->configure_entity_\("
r'"(?:\\.|[^"\\])*"'
r'(?:ESPHOME_PSTR\()?"(?:\\.|[^"\\])*"\)?'
r",\s*\w+,\s*(\d+)\)"
)
combined_pattern = (
rf"App\.register_\w+\(\s*{escaped_var}\s*,\s*"
r'"(?:\\.|[^"\\])*"'
r'(?:ESPHOME_PSTR\()?"(?:\\.|[^"\\])*"\)?'
r",\s*\w+,\s*(\d+)\)"
)
match = re.search(combined_pattern, main_cpp) or re.search(legacy_pattern, main_cpp)
+1 -1
View File
@@ -28,7 +28,7 @@ def test_text_sets_mandatory_fields(generate_main):
main_cpp = generate_main("tests/component_tests/text/test_text.yaml")
# Then
assert 'App.register_text(it_1, "test 1 text",' in main_cpp
assert 'App.register_text(it_1, ESPHOME_PSTR("test 1 text"),' in main_cpp
def test_text_config_value_internal_set(generate_main):
@@ -28,9 +28,18 @@ def test_text_sensor_sets_mandatory_fields(generate_main):
main_cpp = generate_main("tests/component_tests/text_sensor/test_text_sensor.yaml")
# Then
assert 'App.register_text_sensor(ts_1, "Template Text Sensor 1",' in main_cpp
assert 'App.register_text_sensor(ts_2, "Template Text Sensor 2",' in main_cpp
assert 'App.register_text_sensor(ts_3, "Template Text Sensor 3",' in main_cpp
assert (
'App.register_text_sensor(ts_1, ESPHOME_PSTR("Template Text Sensor 1"),'
in main_cpp
)
assert (
'App.register_text_sensor(ts_2, ESPHOME_PSTR("Template Text Sensor 2"),'
in main_cpp
)
assert (
'App.register_text_sensor(ts_3, ESPHOME_PSTR("Template Text Sensor 3"),'
in main_cpp
)
def test_text_sensor_config_value_internal_set(generate_main):
+4 -25
View File
@@ -18,10 +18,6 @@ from esphome.const import (
CONF_INTERNAL,
CONF_NAME,
CONF_UNIT_OF_MEASUREMENT,
KEY_CORE,
KEY_TARGET_PLATFORM,
PLATFORM_ESP32,
PLATFORM_ESP8266,
)
from esphome.core import CORE, ID, entity_helpers
from esphome.core.entity_helpers import (
@@ -43,9 +39,9 @@ from esphome.helpers import fnv1_hash, sanitize, snake_case
from .common import load_config_from_fixture
# Pre-compiled regex pattern for extracting names from configure_entity_/set_name calls
# Matches: .configure_entity_("name", ...) or .set_name("name", ...)
# Matches: .configure_entity_(ESPHOME_PSTR("name"), ...) or .set_name("name", ...)
ENTITY_NAME_PATTERN = re.compile(
r'\.(?:configure_entity_|set_name)\(["\']([^"\']*)["\']'
r'\.(?:configure_entity_|set_name)\((?:ESPHOME_PSTR\()?["\']([^"\']*)["\']'
)
FIXTURES_DIR = Path(__file__).parent.parent / "fixtures" / "core" / "entity_helpers"
@@ -272,7 +268,6 @@ def setup_test_environment() -> Generator[list[str], None, None]:
# Set CORE state for tests
CORE.name = "test-device"
CORE.friendly_name = "Test Device"
CORE.data[KEY_CORE] = {KEY_TARGET_PLATFORM: PLATFORM_ESP32}
# Store original add function
original_add = entity_helpers.add
@@ -288,7 +283,6 @@ def setup_test_environment() -> Generator[list[str], None, None]:
yield added_expressions
# Clean up
entity_helpers.add = original_add
CORE.data.pop(KEY_CORE, None)
def extract_object_id_from_config(config: dict[str, Any]) -> str | None:
@@ -1082,23 +1076,8 @@ async def test_finalize_no_flags(setup_test_environment: list[str]) -> None:
packed = _extract_packed_value(added_expressions)
assert packed == 0
assert "//" not in added_expressions[0]
@pytest.mark.asyncio
async def test_finalize_name_in_flash_on_esp8266(
setup_test_environment: list[str],
) -> None:
"""Test entity names are emitted with PSTR on ESP8266."""
added_expressions = setup_test_environment
CORE.data[KEY_CORE][KEY_TARGET_PLATFORM] = PLATFORM_ESP8266
var = MockObj("sensor1")
config = {
CONF_NAME: "Test",
CONF_DISABLED_BY_DEFAULT: False,
}
await _setup_entity_impl(var, config, "sensor")
finalize_entity_strings(var, config)
assert 'PSTR("Test")' in added_expressions[0]
# Names go through ESPHOME_PSTR so they stay in flash on ESP8266
assert 'ESPHOME_PSTR("Test")' in added_expressions[0]
@pytest.mark.asyncio