[improv_serial][esp32_improv] Use buffer based RPC response builder (#18793)

2026-08-30 17:46:01 +00:00 · 2026-08-27 01:01:35 -05:00
parent 20b38ac8b0
commit fb132b4421
13 changed files with 280 additions and 68 deletions
@@ -1,5 +1,7 @@
 #include "esp32_improv_component.h"

+#include <array>
+
 #include "esphome/components/bytebuffer/bytebuffer.h"
 #include "esphome/components/esp32_ble/ble.h"
 #include "esphome/components/esp32_ble_server/ble_2902.h"
@@ -19,7 +21,13 @@ using namespace bytebuffer;

 static const char *const TAG = "esp32_improv.component";
 static constexpr size_t IMPROV_MAX_LOG_BYTES = 128;
-static const char *const ESPHOME_MY_LINK = "https://my.home-assistant.io/redirect/config_flow_start?domain=esphome";
+static constexpr char ESPHOME_MY_LINK[] = "https://my.home-assistant.io/redirect/config_flow_start?domain=esphome";
+// command + data length + trailing byte
+static constexpr size_t RPC_RESPONSE_OVERHEAD = 3;
+// Reserves the ESPHOME_MY_LINK entry; a maximal next URL displaces only the
+// lower value web server URL
+static constexpr size_t MAX_NEXT_URL_LEN =
+    improv::RPC_RESPONSE_MAX_SIZE - RPC_RESPONSE_OVERHEAD - 1 - sizeof(ESPHOME_MY_LINK);
 static constexpr uint16_t STOP_ADVERTISING_DELAY =
    10000;  // Delay (ms) before stopping service to allow BLE clients to read the final state
 static constexpr uint16_t NAME_ADVERTISING_INTERVAL = 60000;  // Advertise name every 60 seconds
@@ -285,8 +293,9 @@ void ESP32ImprovComponent::set_error_(improv::Error error) {
  }
 }

-void ESP32ImprovComponent::send_response_(std::vector<uint8_t> &&response) {
-  this->rpc_response_->set_value(std::move(response));
+void ESP32ImprovComponent::send_response_(std::span<const uint8_t> response) {
+  // The BLE characteristic owns its value, so one exact-size copy is required here
+  this->rpc_response_->set_value(std::vector<uint8_t>(response.begin(), response.end()));
  if (this->state_ != improv::STATE_STOPPED)
    this->rpc_response_->notify();
 }
@@ -430,40 +439,35 @@ void ESP32ImprovComponent::check_wifi_connection_() {
    this->connecting_sta_ = {};
    this->cancel_timeout("wifi-connect-timeout");

-    // Build URL list with minimal allocations
-    // Maximum 3 URLs: custom next_url + ESPHOME_MY_LINK + webserver URL
-    std::string url_strings[3];
-    size_t url_count = 0;
+    // Build the URL list directly into a stack buffer with no heap allocation
+    std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+    improv::RpcResponseBuilder builder(buf, improv::WIFI_SETTINGS);

 #ifdef USE_ESP32_IMPROV_NEXT_URL
    // Add next_url if configured (should be first per Improv BLE spec)
-    {
-      char url_buffer[384];
-      size_t len = this->get_formatted_next_url_(url_buffer, sizeof(url_buffer));
-      if (len > 0) {
-        url_strings[url_count++] = std::string(url_buffer, len);
-      }
-    }
+    this->add_next_url_(builder, MAX_NEXT_URL_LEN);
 #endif

-    // Add default URLs for backward compatibility
-    url_strings[url_count++] = ESPHOME_MY_LINK;
+    // Add default URLs for backward compatibility; MAX_NEXT_URL_LEN reserves this
+    // entry's space, so it always fits
+    builder.add_string(ESPHOME_MY_LINK, sizeof(ESPHOME_MY_LINK) - 1);
 #ifdef USE_WEBSERVER
    for (auto &ip : wifi::global_wifi_component->wifi_sta_ip_addresses()) {
      if (ip.is_ip4()) {
-        // "http://" (7) + IPv4 max (15) + ":" (1) + port max (5) + null = 29
-        char url_buffer[32];
-        memcpy(url_buffer, "http://", 7);  // NOLINT(bugprone-not-null-terminated-result) - str_to null-terminates
-        ip.str_to(url_buffer + 7);
-        size_t len = strlen(url_buffer);
-        snprintf(url_buffer + len, sizeof(url_buffer) - len, ":%d", USE_WEBSERVER_PORT);
-        url_strings[url_count++] = url_buffer;
+        char ip_buf[network::IP_ADDRESS_BUFFER_SIZE];
+        ip.str_to(ip_buf);
+        // "http://" (7) + IP (40) + ":" (1) + port (5) + null (1) = 54
+        char webserver_url[7 + network::IP_ADDRESS_BUFFER_SIZE + 1 + 5 + 1];
+        size_t len =
+            buf_append_printf(webserver_url, sizeof(webserver_url), 0, "http://%s:%u", ip_buf, USE_WEBSERVER_PORT);
+        if (!builder.add_string(webserver_url, len)) {
+          ESP_LOGW(TAG, "Response full; URL dropped");
+        }
        break;
      }
    }
 #endif
-    this->send_response_(improv::build_rpc_response(improv::WIFI_SETTINGS,
-                                                    std::vector<std::string>(url_strings, url_strings + url_count)));
+    this->send_response_(builder.finish());
  } else if (this->is_active() && this->state_ != improv::STATE_PROVISIONED) {
    ESP_LOGD(TAG, "WiFi provisioned externally");
  }
@@ -22,6 +22,7 @@
 #include "esphome/components/output/binary_output.h"
 #endif

+#include <span>
 #include <vector>

 #ifdef USE_ESP32
@@ -109,7 +110,7 @@ class ESP32ImprovComponent final : public Component, public improv_base::ImprovB
  void set_state_(improv::State state, bool update_advertising = true);
  void set_error_(improv::Error error);
  improv::State get_initial_state_() const;
-  void send_response_(std::vector<uint8_t> &&response);
+  void send_response_(std::span<const uint8_t> response);
  void process_incoming_data_();
  void on_wifi_connect_timeout_();
  void check_wifi_connection_();
@@ -4,10 +4,13 @@
 #include "esphome/components/network/util.h"
 #include "esphome/core/application.h"
 #include "esphome/core/defines.h"
+#include "esphome/core/log.h"

 namespace esphome::improv_base {

 #if defined(USE_ESP32_IMPROV_NEXT_URL) || defined(USE_IMPROV_SERIAL_NEXT_URL)
+static const char *const TAG = "improv_base";
+
 static constexpr const char DEVICE_NAME_PLACEHOLDER[] = "{{device_name}}";
 static constexpr size_t DEVICE_NAME_PLACEHOLDER_LEN = sizeof(DEVICE_NAME_PLACEHOLDER) - 1;
 static constexpr const char IP_ADDRESS_PLACEHOLDER[] = "{{ip_address}}";
@@ -62,6 +65,21 @@ size_t ImprovBase::get_formatted_next_url_(char *buffer, size_t buffer_size) {
  *out = '\0';
  return out - buffer;
 }
+
+void ImprovBase::add_next_url_(improv::RpcResponseBuilder &builder, size_t max_len) {
+  // The builder rejects strings above 254 bytes, so anything longer than this
+  // buffer could never be sent anyway
+  char url_buffer[256];
+  size_t len = this->get_formatted_next_url_(url_buffer, sizeof(url_buffer));
+  if (len == 0) {
+    return;
+  }
+  // max_len is the transport's budget for this entry; skipping an over-long URL
+  // here keeps the rest of the response sendable instead of oversizing the frame
+  if (len > max_len || !builder.add_string(url_buffer, len)) {
+    ESP_LOGW(TAG, "Next URL too long; skipping");
+  }
+}
 #endif

 }  // namespace esphome::improv_base
@@ -3,6 +3,10 @@
 #include <cstddef>
 #include "esphome/core/defines.h"

+#if defined(USE_ESP32_IMPROV_NEXT_URL) || defined(USE_IMPROV_SERIAL_NEXT_URL)
+#include <improv.h>
+#endif
+
 namespace esphome::improv_base {

 class ImprovBase {
@@ -15,6 +19,8 @@ class ImprovBase {
 #if defined(USE_ESP32_IMPROV_NEXT_URL) || defined(USE_IMPROV_SERIAL_NEXT_URL)
  /// Format next_url_ into buffer, replacing placeholders. Returns length written.
  size_t get_formatted_next_url_(char *buffer, size_t buffer_size);
+  /// Append the formatted next_url to the RPC response, warning if it does not fit.
+  void add_next_url_(improv::RpcResponseBuilder &builder, size_t max_len);
  const char *next_url_{nullptr};
 #endif
 };
@@ -9,6 +9,8 @@
 #include "esphome/components/logger/logger.h"
 #include "esphome/components/wifi/scan_list.h"

+#include <array>
+
 namespace esphome::improv_serial {

 static const char *const TAG = "improv_serial";
@@ -61,8 +63,7 @@ void ImprovSerialComponent::loop() {
      this->cancel_timeout("wifi-connect-timeout");
      this->set_state_(improv::STATE_PROVISIONED);

-      std::vector<uint8_t> url = this->build_rpc_settings_response_(improv::WIFI_SETTINGS);
-      this->send_response_(url);
+      this->send_settings_response_(improv::WIFI_SETTINGS);
    }
  }
 }
@@ -142,16 +143,11 @@ void ImprovSerialComponent::write_data_(const uint8_t *data, const size_t size)
 #endif
 }

-std::vector<uint8_t> ImprovSerialComponent::build_rpc_settings_response_(improv::Command command) {
-  std::vector<std::string> urls;
+void ImprovSerialComponent::send_settings_response_(improv::Command command) {
+  std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+  improv::RpcResponseBuilder builder(buf, command);
 #ifdef USE_IMPROV_SERIAL_NEXT_URL
-  {
-    char url_buffer[384];
-    size_t len = this->get_formatted_next_url_(url_buffer, sizeof(url_buffer));
-    if (len > 0) {
-      urls.emplace_back(url_buffer, len);
-    }
-  }
+  this->add_next_url_(builder, MAX_NEXT_URL_LEN);
 #endif
 #ifdef USE_WEBSERVER
  for (auto &ip : wifi::global_wifi_component->wifi_sta_ip_addresses()) {
@@ -160,25 +156,63 @@ std::vector<uint8_t> ImprovSerialComponent::build_rpc_settings_response_(improv:
      ip.str_to(ip_buf);
      // "http://" (7) + IP (40) + ":" (1) + port (5) + null (1) = 54
      char webserver_url[7 + network::IP_ADDRESS_BUFFER_SIZE + 1 + 5 + 1];
-      snprintf(webserver_url, sizeof(webserver_url), "http://%s:%u", ip_buf, USE_WEBSERVER_PORT);
-      urls.emplace_back(webserver_url);
+      // buf_append_printf keeps the format string in flash on ESP8266
+      size_t len =
+          buf_append_printf(webserver_url, sizeof(webserver_url), 0, "http://%s:%u", ip_buf, USE_WEBSERVER_PORT);
+      if (!builder.add_string(webserver_url, len)) {
+        ESP_LOGW(TAG, "Response full; URL dropped");
+      }
      break;
    }
  }
 #endif
-  std::vector<uint8_t> data = improv::build_rpc_response(command, urls, false);
-  return data;
+  this->send_response_(builder.finish(false));
 }

-std::vector<uint8_t> ImprovSerialComponent::build_version_info_() {
+void ImprovSerialComponent::send_version_info_() {
+// Entry cost per field is sizeof(lit): a length byte plus the string
 #ifdef ESPHOME_PROJECT_NAME
-  std::vector<std::string> infos = {ESPHOME_PROJECT_NAME, ESPHOME_PROJECT_VERSION, ESPHOME_VARIANT, App.get_name()};
+  static constexpr size_t INFO_ENTRIES_LEN =
+      sizeof(ESPHOME_PROJECT_NAME) + sizeof(ESPHOME_PROJECT_VERSION) + sizeof(ESPHOME_VARIANT);
 #else
-  std::vector<std::string> infos = {"ESPHome", ESPHOME_VERSION, ESPHOME_VARIANT, App.get_name()};
+  static constexpr size_t INFO_ENTRIES_LEN = sizeof("ESPHome") + sizeof(ESPHOME_VERSION) + sizeof(ESPHOME_VARIANT);
 #endif
-  std::vector<uint8_t> data = improv::build_rpc_response(improv::GET_DEVICE_INFO, infos, false);
-  return data;
-};
+  static_assert(INFO_ENTRIES_LEN < MAX_SERIAL_PAYLOAD,
+                "esphome project name and version too long for the improv_serial device info frame");
+  std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+  improv::RpcResponseBuilder builder(buf, improv::GET_DEVICE_INFO);
+#ifdef USE_ESP8266
+  // Keep each literal in flash and copy it through an exact size stack buffer,
+  // so a long project name or version can never be truncated
+#define IMPROV_ADD_INFO(lit) \
+  do { \
+    static const char progmem_str[] PROGMEM = lit; \
+    char tmp[sizeof(lit)]; \
+    progmem_memcpy(tmp, progmem_str, sizeof(lit)); \
+    builder.add_string(tmp, sizeof(lit) - 1); \
+  } while (0)
+#else
+  // Literals are directly flash mapped on all other platforms
+#define IMPROV_ADD_INFO(lit) builder.add_string(lit, sizeof(lit) - 1)
+#endif
+#ifdef ESPHOME_PROJECT_NAME
+  IMPROV_ADD_INFO(ESPHOME_PROJECT_NAME);
+  IMPROV_ADD_INFO(ESPHOME_PROJECT_VERSION);
+#else
+  IMPROV_ADD_INFO("ESPHome");
+  IMPROV_ADD_INFO(ESPHOME_VERSION);
+#endif
+  IMPROV_ADD_INFO(ESPHOME_VARIANT);
+#undef IMPROV_ADD_INFO
+  // Only the device name length is unknown at compile time
+  const auto &name = App.get_name();
+  if (INFO_ENTRIES_LEN + 1 + name.size() <= MAX_SERIAL_PAYLOAD) {
+    builder.add_string(name.c_str(), name.size());
+  } else {
+    ESP_LOGW(TAG, "Response full; device name dropped");
+  }
+  this->send_response_(builder.finish(false));
+}

 bool ImprovSerialComponent::parse_improv_serial_byte_(uint8_t byte) {
  size_t at = this->rx_buffer_.size();
@@ -229,32 +263,35 @@ bool ImprovSerialComponent::parse_improv_payload_(improv::ImprovCommand &command
      }
      this->set_state_(this->state_);
      if (this->state_ == improv::STATE_PROVISIONED) {
-        std::vector<uint8_t> url = this->build_rpc_settings_response_(improv::GET_CURRENT_STATE);
-        this->send_response_(url);
+        this->send_settings_response_(improv::GET_CURRENT_STATE);
      }
      return true;
    case improv::GET_DEVICE_INFO: {
-      std::vector<uint8_t> info = this->build_version_info_();
-      this->send_response_(info);
+      this->send_version_info_();
      return true;
    }
    case improv::GET_WIFI_NETWORKS: {
      const auto &results = wifi::global_wifi_component->get_scan_result();
+      std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
      for (const auto &scan : results) {
        bool with_auth = false;
        if (!wifi::should_show_scan_entry(results, scan, with_auth))
          continue;
        // Send each ssid separately to avoid overflowing the buffer
        char rssi_buf[5];  // int8_t: -128 to 127, max 4 chars + null
-        *int8_to_str(rssi_buf, scan.get_rssi()) = '\0';
-        std::vector<uint8_t> data = improv::build_rpc_response(
-            improv::GET_WIFI_NETWORKS, {scan.get_ssid().str(), rssi_buf, YESNO(with_auth)}, false);
-        this->send_response_(data);
+        char *rssi_end = int8_to_str(rssi_buf, scan.get_rssi());
+        *rssi_end = '\0';
+        improv::RpcResponseBuilder builder(buf, improv::GET_WIFI_NETWORKS);
+        // SSID(32) + RSSI(4) + YESNO(3) entries always fit the payload
+        const auto &ssid = scan.get_ssid();
+        builder.add_string(ssid.c_str(), ssid.size());
+        builder.add_string(rssi_buf, rssi_end - rssi_buf);
+        builder.add_string(YESNO(with_auth));
+        this->send_response_(builder.finish(false));
      }
      // Send empty response to signify the end of the list.
-      std::vector<uint8_t> data =
-          improv::build_rpc_response(improv::GET_WIFI_NETWORKS, std::vector<std::string>{}, false);
-      this->send_response_(data);
+      improv::RpcResponseBuilder builder(buf, improv::GET_WIFI_NETWORKS);
+      this->send_response_(builder.finish(false));
      return true;
    }
    default: {
@@ -282,7 +319,14 @@ void ImprovSerialComponent::set_error_(improv::Error error) {
  this->write_data_();
 }

-void ImprovSerialComponent::send_response_(std::vector<uint8_t> &response) {
+void ImprovSerialComponent::send_response_(std::span<const uint8_t> response) {
+  // The serial frame length field is a single byte
+  if (response.size() > MAX_SERIAL_RESPONSE) {
+    ESP_LOGE(TAG, "Response too long");
+    // Fail fast instead of leaving the client to wait out its timeout
+    this->set_error_(improv::ERROR_UNKNOWN);
+    return;
+  }
  this->tx_header_[TX_TYPE_IDX] = TYPE_RPC_RESPONSE;
  this->write_data_(response.data(), response.size());
 }
@@ -8,6 +8,7 @@
 #include "esphome/core/helpers.h"
 #ifdef USE_WIFI
 #include <improv.h>
+#include <span>
 #include <vector>

 #ifdef USE_IMPROV_SERIAL_UART
@@ -47,6 +48,22 @@ enum ImprovSerialType : uint8_t {
 static const uint16_t IMPROV_SERIAL_TIMEOUT = 100;
 static const uint8_t IMPROV_SERIAL_VERSION = 1;

+// The serial frame length field is one byte
+static constexpr size_t MAX_SERIAL_RESPONSE = 255;
+// command + data length + trailing byte
+static constexpr size_t RPC_RESPONSE_OVERHEAD = 3;
+static constexpr size_t MAX_SERIAL_PAYLOAD = MAX_SERIAL_RESPONSE - RPC_RESPONSE_OVERHEAD;
+#ifdef USE_WEBSERVER
+// length byte + "http://" + IPv4 + ":" + port
+static constexpr size_t WEBSERVER_URL_RESERVE = 1 + 7 + 15 + 1 + 5;
+#else
+static constexpr size_t WEBSERVER_URL_RESERVE = 0;
+#endif
+// Entry budget minus its own length byte
+static constexpr size_t MAX_NEXT_URL_LEN = MAX_SERIAL_PAYLOAD - WEBSERVER_URL_RESERVE - 1;
+
+static_assert(MAX_SERIAL_RESPONSE <= improv::RPC_RESPONSE_MAX_SIZE, "builder buffer too small for the frame");
+
 class ImprovSerialComponent final : public Component, public improv_base::ImprovBase {
 public:
  void setup() override;
@@ -66,11 +83,11 @@ class ImprovSerialComponent final : public Component, public improv_base::Improv
  void set_state_(improv::State state);
  void send_current_state_(improv::State state);
  void set_error_(improv::Error error);
-  void send_response_(std::vector<uint8_t> &response);
+  void send_response_(std::span<const uint8_t> response);
  void on_wifi_connect_timeout_();

-  std::vector<uint8_t> build_rpc_settings_response_(improv::Command command);
-  std::vector<uint8_t> build_version_info_();
+  void send_settings_response_(improv::Command command);
+  void send_version_info_();

  ESPHOME_ALWAYS_INLINE optional<uint8_t> read_byte_() {
    optional<uint8_t> byte;
@@ -188,6 +188,8 @@ struct IPAddress {
  }
  IPAddress(const std::string &in_address) { inet_aton(in_address.c_str(), &ip_addr_); }
  IPAddress(const ip_addr_t *other_ip) { ip_addr_ = *other_ip; }
+  bool is_ip4() const { return true; }
+  bool is_ip6() const { return false; }
  /// Write IP address to buffer. Buffer must be at least IP_ADDRESS_BUFFER_SIZE bytes.
  char *str_to(char *buf) const {
    inet_ntop(AF_INET, &ip_addr_, buf, IP_ADDRESS_BUFFER_SIZE);
@@ -81,8 +81,6 @@
 #define USE_HTTP_REQUEST_OTA_WATCHDOG_TIMEOUT 8000  // NOLINT
 #define USE_I2S_AUDIO_SPDIF_MODE
 #define USE_IMAGE
-#define USE_IMPROV_SERIAL
-#define USE_IMPROV_SERIAL_NEXT_URL
 #define USE_INFRARED
 #define USE_IR_RF
 #define USE_JSON
@@ -223,6 +221,9 @@
 #define USE_API_USER_DEFINED_ACTION_RESPONSES_JSON
 #define API_MAX_SEND_QUEUE 8
 #define MAX_API_CONNECTIONS 6
+// The Improv library is not in the Zephyr tidy environment
+#define USE_IMPROV_SERIAL
+#define USE_IMPROV_SERIAL_NEXT_URL
 #define USE_MD5
 #define USE_NOISE
 #define USE_SHA256
@@ -0,0 +1,6 @@
+# The builder test compares against the Improv library's build_rpc_response,
+# so the library must be part of the unit test build.
+# Keep the version in sync with the pin in esphome/components/improv_base/__init__.py.
+esphome:
+  libraries:
+    - improv/Improv@1.2.7
@@ -0,0 +1,102 @@
+#include <gtest/gtest.h>
+
+#include <array>
+#include <cstdint>
+#include <cstring>
+#include <string>
+#include <vector>
+
+#include <improv.h>
+
+namespace esphome::improv_base::testing {
+
+namespace {
+
+std::vector<uint8_t> build_with_builder(improv::Command command, const std::vector<std::string> &datum,
+                                        bool add_checksum) {
+  std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+  improv::RpcResponseBuilder builder(buf, command);
+  for (const auto &str : datum) {
+    EXPECT_TRUE(builder.add_string(str.c_str(), str.size()));
+  }
+  auto out = builder.finish(add_checksum);
+  return {out.begin(), out.end()};
+}
+
+}  // namespace
+
+// The serial path sends builder output where build_rpc_response bytes went before,
+// so the two must match exactly, including the trailing 0x00 when checksums are off.
+TEST(RpcResponseBuilder, ByteIdenticalToBuildRpcResponse) {
+  const std::vector<std::string> device_info = {"ESPHome", "2026.9.0", "ESP32", "test-device"};
+  const std::vector<std::string> network = {"MySSID", "-67", "YES"};
+  const std::vector<std::string> empty = {};
+  const std::vector<std::string> max_payload = {std::string(254, 'x')};
+
+  for (bool add_checksum : {false, true}) {
+    for (const auto *datum : {&device_info, &network, &empty, &max_payload}) {
+      EXPECT_EQ(build_with_builder(improv::GET_DEVICE_INFO, *datum, add_checksum),
+                improv::build_rpc_response(improv::GET_DEVICE_INFO, *datum, add_checksum));
+    }
+  }
+}
+
+// Golden bytes independent of the library: command, data length, string entries,
+// then the trailing byte (0x00 without checksum, additive checksum with).
+TEST(RpcResponseBuilder, GoldenBytes) {
+  EXPECT_EQ(build_with_builder(improv::GET_WIFI_NETWORKS, {}, false), (std::vector<uint8_t>{0x04, 0x00, 0x00}));
+  EXPECT_EQ(build_with_builder(improv::GET_WIFI_NETWORKS, {"ab"}, false),
+            (std::vector<uint8_t>{0x04, 0x03, 0x02, 'a', 'b', 0x00}));
+  // Checksum: 0x04 + 0x03 + 0x02 + 'a' + 'b' = 0xCC
+  EXPECT_EQ(build_with_builder(improv::GET_WIFI_NETWORKS, {"ab"}, true),
+            (std::vector<uint8_t>{0x04, 0x03, 0x02, 'a', 'b', 0xCC}));
+}
+
+// esp32_improv calls finish() and build_rpc_response() with no checksum flag,
+// so the two defaults must agree
+TEST(RpcResponseBuilder, DefaultChecksumFlagMatches) {
+  const std::vector<std::string> urls = {"https://example.com"};
+  std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+  improv::RpcResponseBuilder builder(buf, improv::WIFI_SETTINGS);
+  for (const auto &str : urls) {
+    EXPECT_TRUE(builder.add_string(str.c_str(), str.size()));
+  }
+  auto out = builder.finish();
+  EXPECT_EQ(std::vector<uint8_t>(out.begin(), out.end()), improv::build_rpc_response(improv::WIFI_SETTINGS, urls));
+}
+
+TEST(RpcResponseBuilder, PayloadBudget) {
+  std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+
+  // 254 byte string fills the payload exactly; a second entry no longer fits
+  improv::RpcResponseBuilder full(buf, improv::GET_DEVICE_INFO);
+  const std::string big(254, 'x');
+  EXPECT_TRUE(full.add_string(big.c_str(), big.size()));
+  EXPECT_FALSE(full.add_string("y", 1));
+
+  // 255 byte string can never fit (its length byte would exceed the budget)
+  improv::RpcResponseBuilder over(buf, improv::GET_DEVICE_INFO);
+  const std::string too_big(255, 'y');
+  EXPECT_FALSE(over.add_string(too_big.c_str(), too_big.size()));
+  // A wildly out of range length must not wrap the position arithmetic
+  EXPECT_FALSE(over.add_string("z", static_cast<size_t>(-1)));
+  auto out = over.finish(false);
+  EXPECT_EQ(std::vector<uint8_t>(out.begin(), out.end()), (std::vector<uint8_t>{0x03, 0x00, 0x00}));
+}
+
+TEST(RpcResponseBuilder, FinishIsIdempotent) {
+  std::array<uint8_t, improv::RPC_RESPONSE_MAX_SIZE> buf;
+  improv::RpcResponseBuilder builder(buf, improv::GET_DEVICE_INFO);
+  EXPECT_TRUE(builder.add_string("abc", 3));
+  auto first = builder.finish(true);
+  const std::vector<uint8_t> expected(first.begin(), first.end());
+
+  EXPECT_FALSE(builder.add_string("late", 4));
+  auto again = builder.finish(true);
+  EXPECT_EQ(std::vector<uint8_t>(again.begin(), again.end()), expected);
+  // The checksum flag on a later call is ignored
+  auto no_checksum = builder.finish(false);
+  EXPECT_EQ(std::vector<uint8_t>(no_checksum.begin(), no_checksum.end()), expected);
+}
+
+}  // namespace esphome::improv_base::testing
@@ -5,4 +5,6 @@ wifi:
 logger:
  hardware_uart: UART0

+# next_url compiles the USE_IMPROV_SERIAL_NEXT_URL branch and add_next_url_
 improv_serial:
+  next_url: https://example.com/?device_name={{device_name}}&ip_address={{ip_address}}
@@ -38,3 +38,5 @@ uart_mock:

 improv_serial:
  uart_id: mock_uart
+  # Deterministic on host: only the device name placeholder is used
+  next_url: https://example.com/?device={{device_name}}
@@ -133,8 +133,15 @@ async def test_improv_serial_uart(
        )
        await waiter.wait_for("save_wifi_sta ssid=NewNet")
        await waiter.wait_for("uart_mock", f"TX 12 bytes: {state_frame_hex(0x04)}")
-        # Settings RPC response with no URLs: payload [0x01, 0x00, 0x00] and footer
-        await waiter.wait_for("uart_mock", "TX 3 bytes: 01:00:00")
-        await waiter.wait_for(
-            "uart_mock", f"TX 2 bytes: {rpc_footer_hex(bytes([0x01, 0x00, 0x00]))}"
+        # Settings RPC response carries the formatted next_url and its footer
+        next_url = b"https://example.com/?device=improv-uart"
+        payload = (
+            bytes([CMD_WIFI_SETTINGS, len(next_url) + 1, len(next_url)])
+            + next_url
+            + b"\x00"
        )
+        await waiter.wait_for(
+            "uart_mock",
+            f"TX {len(payload)} bytes: " + ":".join(f"{b:02X}" for b in payload),
+        )
+        await waiter.wait_for("uart_mock", f"TX 2 bytes: {rpc_footer_hex(payload)}")