From 363086b4f17c0a56bad9b53f62ef30202495eeb7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 5 Mar 2026 15:35:31 -1000 Subject: [PATCH] Extract make_app_name_cpp as testable free function with unit tests Move the pure logic (string building, escaping, byte length calculation) out of the nested closure into a module-level function that can be tested without codegen mocks. Tests cover both copilot review concerns: empty friendly_name with MAC suffix, and UTF-8 byte length for non-ASCII characters. --- esphome/core/config.py | 62 ++++++++++++---------- tests/unit_tests/core/test_config.py | 77 ++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 27 deletions(-) diff --git a/esphome/core/config.py b/esphome/core/config.py index f21f8ae3742..a1b2c0d281e 100644 --- a/esphome/core/config.py +++ b/esphome/core/config.py @@ -64,6 +64,32 @@ _APP_NAME_BUF_VAR = "esphome_app_name_buf" _APP_NAME_MAC_SEP = "-" _APP_FRIENDLY_NAME_BUF_VAR = "esphome_app_friendly_name_buf" _APP_FRIENDLY_NAME_MAC_SEP = " " +# Placeholder suffix for MAC address (last 6 hex chars) +_MAC_SUFFIX_PLACEHOLDER = "XXXXXX" + + +def make_app_name_cpp( + value: str, var_name: str, sep: str, *, add_mac_suffix: bool +) -> tuple[str, str | None, int]: + """Compute C++ expression and optional global declaration for an app name. + + Returns (cpp_expr, global_decl_or_none, byte_length). + - cpp_expr: The C++ expression to pass to pre_setup (var name or string literal). + - global_decl: A static char[] declaration string, or None if not needed. + - byte_length: The UTF-8 byte length of the string value. + """ + if add_mac_suffix: + buf_value = "" if not value else f"{value}{sep}{_MAC_SUFFIX_PLACEHOLDER}" + escaped = cpp_string_escape(buf_value) + return ( + var_name, + f"static char {var_name}[] = {escaped};", + len(buf_value.encode("utf-8")), + ) + if not value: + return '""', None, 0 + return cpp_string_escape(value), None, len(value.encode("utf-8")) + StartupTrigger = cg.esphome_ns.class_( "StartupTrigger", cg.Component, automation.Trigger.template() @@ -562,37 +588,19 @@ async def to_code(config: ConfigType) -> None: friendly_name = config[CONF_FRIENDLY_NAME] name_add_mac_suffix = config[CONF_NAME_ADD_MAC_SUFFIX] - def _make_app_name_expr( + def _emit_app_name( value: str, var_name: str, sep: str ) -> tuple[cg.Expression, int]: - """Create a name expression for pre_setup. - - With MAC suffix: emits a static mutable buffer with placeholder suffix. - Without: passes the string literal directly as const char*. - Returns (expression, byte_length). - """ - if name_add_mac_suffix: - value_with_placeholder = "" if not value else f"{value}{sep}XXXXXX" - cg.add_global( - cg.RawStatement( - f"static char {var_name}[] = {cpp_string_escape(value_with_placeholder)};" - ) - ) - return ( - cg.RawExpression(var_name), - len(value_with_placeholder.encode("utf-8")), - ) - if not value: - return cg.RawExpression('""'), 0 - return ( - cg.RawExpression(cpp_string_escape(value)), - len(value.encode("utf-8")), + """Emit codegen for an app name and return (expression, byte_length).""" + cpp_expr, global_decl, byte_len = make_app_name_cpp( + value, var_name, sep, add_mac_suffix=name_add_mac_suffix ) + if global_decl is not None: + cg.add_global(cg.RawStatement(global_decl)) + return cg.RawExpression(cpp_expr), byte_len - name_expr, name_len = _make_app_name_expr( - name, _APP_NAME_BUF_VAR, _APP_NAME_MAC_SEP - ) - friendly_expr, friendly_len = _make_app_name_expr( + name_expr, name_len = _emit_app_name(name, _APP_NAME_BUF_VAR, _APP_NAME_MAC_SEP) + friendly_expr, friendly_len = _emit_app_name( friendly_name, _APP_FRIENDLY_NAME_BUF_VAR, _APP_FRIENDLY_NAME_MAC_SEP ) if name_add_mac_suffix: diff --git a/tests/unit_tests/core/test_config.py b/tests/unit_tests/core/test_config.py index 88801a9ca03..474d31a90af 100644 --- a/tests/unit_tests/core/test_config.py +++ b/tests/unit_tests/core/test_config.py @@ -23,6 +23,7 @@ from esphome.const import ( from esphome.core import CORE, config from esphome.core.config import ( Area, + make_app_name_cpp, preload_core_config, valid_include, valid_project_name, @@ -969,3 +970,79 @@ def test_config_hash_different_for_different_configs() -> None: hash2 = CORE.config_hash assert hash1 != hash2 + + +def test_make_app_name_cpp_no_mac_simple() -> None: + """Test simple name without MAC suffix returns string literal.""" + cpp_expr, global_decl, byte_len = make_app_name_cpp( + "my-device", "buf", "-", add_mac_suffix=False + ) + assert cpp_expr == '"my-device"' + assert global_decl is None + assert byte_len == 9 + + +def test_make_app_name_cpp_no_mac_empty() -> None: + """Test empty name without MAC suffix.""" + cpp_expr, global_decl, byte_len = make_app_name_cpp( + "", "buf", "-", add_mac_suffix=False + ) + assert cpp_expr == '""' + assert global_decl is None + assert byte_len == 0 + + +def test_make_app_name_cpp_mac_suffix() -> None: + """Test name with MAC suffix emits static buffer.""" + cpp_expr, global_decl, byte_len = make_app_name_cpp( + "my-device", "esphome_app_name_buf", "-", add_mac_suffix=True + ) + assert cpp_expr == "esphome_app_name_buf" + assert global_decl is not None + assert "static char esphome_app_name_buf[]" in global_decl + assert "my-device-XXXXXX" in global_decl + assert byte_len == len("my-device-XXXXXX") + + +def test_make_app_name_cpp_mac_suffix_empty() -> None: + """Test empty name with MAC suffix emits empty static buffer.""" + cpp_expr, global_decl, byte_len = make_app_name_cpp( + "", "esphome_app_name_buf", "-", add_mac_suffix=True + ) + assert cpp_expr == "esphome_app_name_buf" + assert global_decl is not None + assert "static char esphome_app_name_buf[]" in global_decl + assert byte_len == 0 + + +def test_make_app_name_cpp_mac_suffix_space_sep() -> None: + """Test friendly name uses space separator for MAC suffix.""" + cpp_expr, global_decl, byte_len = make_app_name_cpp( + "My Device", "esphome_app_friendly_name_buf", " ", add_mac_suffix=True + ) + assert cpp_expr == "esphome_app_friendly_name_buf" + assert global_decl is not None + assert "My Device XXXXXX" in global_decl + assert byte_len == len("My Device XXXXXX") + + +def test_make_app_name_cpp_non_ascii_utf8_length() -> None: + """Test non-ASCII characters use UTF-8 byte length.""" + _, global_decl, byte_len = make_app_name_cpp( + "café", "buf", "-", add_mac_suffix=False + ) + assert byte_len == len("café".encode()) # 5 bytes, not 4 chars + assert global_decl is None + + +def test_make_app_name_cpp_non_ascii_mac_suffix_utf8_length() -> None: + """Test non-ASCII with MAC suffix uses UTF-8 byte length.""" + _, _, byte_len = make_app_name_cpp("café", "buf", "-", add_mac_suffix=True) + assert byte_len == len("café-XXXXXX".encode()) + + +def test_make_app_name_cpp_special_chars_escaped() -> None: + """Test special characters are properly escaped in C++ string.""" + cpp_expr, _, _ = make_app_name_cpp('my "device"', "buf", "-", add_mac_suffix=False) + # cpp_string_escape uses octal escapes for quotes + assert '"' not in cpp_expr[1:-1] # no unescaped quotes inside the outer quotes