From 8c0e5e9d9a180f8331dd04eb310fa9e8563e18b7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2026 22:40:45 -0500 Subject: [PATCH] [api] Address Copilot review on proxy benchmarks PR - Make UARTFlushResult in the serial_proxy stub a scoped enum class with matching scoped enumerator return in flush_port(), so the stub signature lines up with the real esphome::uart::UARTFlushResult. - Replace heap-leaking lazy-init in get_ir_timings_100() with a function-local static const std::vector populated by a regular helper function. Same lazy-init behavior, no leak in valgrind/ASan, no lambda IIFE. - Emit field 6 (modulation = 1) in build_infrared_rf_transmit_wire() so the bytes match the documented field list and the decode benchmark also exercises the field-6 decode_varint path. --- .../components/api/bench_proto_proxy.cpp | 27 +++++++++++-------- .../components/serial_proxy/serial_proxy.h | 4 +-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/benchmarks/components/api/bench_proto_proxy.cpp b/tests/benchmarks/components/api/bench_proto_proxy.cpp index ae7fc0722c..fa3191a969 100644 --- a/tests/benchmarks/components/api/bench_proto_proxy.cpp +++ b/tests/benchmarks/components/api/bench_proto_proxy.cpp @@ -151,18 +151,19 @@ BENCHMARK(Decode_SerialProxyWriteRequest); #if defined(USE_IR_RF) || defined(USE_RADIO_FREQUENCY) -// Heap-allocated on first use to avoid C++17 lambda IIFE patterns that some -// callgrind/valgrind versions handle awkwardly during benchmark init. -static const std::vector &get_ir_timings_100() { - static std::vector *timings = nullptr; - if (timings == nullptr) { - timings = new std::vector(); - timings->reserve(100); - for (int i = 0; i < 100; i++) { - timings->push_back((i % 2 == 0) ? 560 : -560); - } +// Mark/space pairs simulating a typical RC-5 / NEC capture (100 timings). +static std::vector make_ir_timings_100() { + std::vector v; + v.reserve(100); + for (int i = 0; i < 100; i++) { + v.push_back((i % 2 == 0) ? 560 : -560); } - return *timings; + return v; +} + +static const std::vector &get_ir_timings_100() { + static const std::vector timings = make_ir_timings_100(); + return timings; } static void Encode_InfraredRFReceiveEvent(benchmark::State &state) { @@ -247,6 +248,10 @@ static APIBuffer build_infrared_rf_transmit_wire() { put_varint(static_cast(packed_len)); std::memcpy(bytes + len, packed, packed_len); len += packed_len; + // field 6: modulation = 1 (non-zero so it's actually emitted and exercises + // decode_varint for this field, matching the documented layout above). + put_byte(0x30); + put_varint(1); APIBuffer buf; buf.resize(len); diff --git a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h index d967ad58b1..bab27549e7 100644 --- a/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h +++ b/tests/benchmarks/stubs/esphome/components/serial_proxy/serial_proxy.h @@ -14,7 +14,7 @@ class APIConnection; } // namespace api namespace uart { -enum UARTFlushResult : uint8_t { +enum class UARTFlushResult : uint8_t { UART_FLUSH_RESULT_SUCCESS, UART_FLUSH_RESULT_ASSUMED_SUCCESS, UART_FLUSH_RESULT_TIMEOUT, @@ -36,7 +36,7 @@ class SerialProxy { void write_from_client(const uint8_t *data, size_t len) {} void set_modem_pins(uint32_t line_states) {} uint32_t get_modem_pins() const { return 0; } - uart::UARTFlushResult flush_port() { return uart::UART_FLUSH_RESULT_SUCCESS; } + uart::UARTFlushResult flush_port() { return uart::UARTFlushResult::UART_FLUSH_RESULT_SUCCESS; } protected: uint32_t instance_index_{0};