From d2c388f8934f4508756b08684648bb2f374a716d Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Thu, 4 Jun 2026 18:01:00 -0400 Subject: [PATCH] [ota][logger][esp32][internal_temperature] Fix clang-tidy findings surfaced by RISC-V analysis (#16811) --- esphome/components/esp32/crash_handler.cpp | 5 +++++ .../internal_temperature/internal_temperature.h | 11 +++++++++++ .../internal_temperature_esp32.cpp | 12 +++--------- esphome/components/logger/logger_esp32.cpp | 4 ++-- esphome/components/ota/ota_backend_esp_idf.h | 3 ++- 5 files changed, 23 insertions(+), 12 deletions(-) diff --git a/esphome/components/esp32/crash_handler.cpp b/esphome/components/esp32/crash_handler.cpp index ed61b61936..a7de48a6ee 100644 --- a/esphome/components/esp32/crash_handler.cpp +++ b/esphome/components/esp32/crash_handler.cpp @@ -41,6 +41,7 @@ static inline bool is_return_addr(uint32_t addr) { // Use memcpy for alignment safety — RISC-V C extension means code addresses // are only 2-byte aligned, so addr-4 may not be 4-byte aligned. uint32_t inst; + // NOLINTNEXTLINE(performance-no-int-to-ptr) - reading code memory at a raw address is the point memcpy(&inst, (const void *) (addr - 4), sizeof(inst)); // RISC-V instruction encoding: bits [6:0] = opcode, bits [11:7] = rd uint32_t opcode = inst & 0x7f; // Extract 7-bit opcode @@ -51,6 +52,7 @@ static inline bool is_return_addr(uint32_t addr) { // Check for 2-byte compressed c.jalr before this address (C extension). // c.jalr saves to ra implicitly: funct4=1001, rs1!=0, rs2=0, op=10 if (addr >= 2) { + // NOLINTNEXTLINE(performance-no-int-to-ptr) - reading code memory at a raw address is the point uint16_t c_inst = *(uint16_t *) (addr - 2); if ((c_inst & 0xf07f) == 0x9002 && (c_inst & 0x0f80) != 0) return true; @@ -101,6 +103,7 @@ static uint8_t IRAM_ATTR capture_riscv_backtrace(RvExcFrame *frame, uint32_t *ou out[count++] = frame->ra; } *reg_count = count; + // NOLINTNEXTLINE(performance-no-int-to-ptr) - walking the raw stack by address is the point auto *scan_start = (uint32_t *) frame->sp; for (uint32_t i = 0; i < 64 && count < max; i++) { uint32_t val = scan_start[i]; @@ -354,6 +357,8 @@ void crash_handler_log() { #if SOC_CPU_CORES_NUM > 1 append_addrs_to_hint(hint, sizeof(hint), pos, s_raw_crash_data.other_backtrace, s_raw_crash_data.other_backtrace_count, s_raw_crash_data.other_reg_frame_count); +#else + (void) pos; // There is no second-core append on single-core targets, so pos would otherwise be unread. #endif ESP_LOGE(TAG, "%s", hint); } diff --git a/esphome/components/internal_temperature/internal_temperature.h b/esphome/components/internal_temperature/internal_temperature.h index 4810e8478d..41fea5a255 100644 --- a/esphome/components/internal_temperature/internal_temperature.h +++ b/esphome/components/internal_temperature/internal_temperature.h @@ -3,6 +3,12 @@ #include "esphome/components/sensor/sensor.h" #include "esphome/core/component.h" +// Every ESP32 variant except the original one exposes the on-chip sensor through +// the IDF temperature_sensor driver (the original uses the legacy temprature_sens_read). +#if defined(USE_ESP32) && !defined(USE_ESP32_VARIANT_ESP32) +#include "driver/temperature_sensor.h" +#endif + namespace esphome::internal_temperature { class InternalTemperatureSensor : public sensor::Sensor, public PollingComponent { @@ -13,6 +19,11 @@ class InternalTemperatureSensor : public sensor::Sensor, public PollingComponent void dump_config() override; void update() override; + +#if defined(USE_ESP32) && !defined(USE_ESP32_VARIANT_ESP32) + protected: + temperature_sensor_handle_t tsens_{nullptr}; +#endif }; } // namespace esphome::internal_temperature diff --git a/esphome/components/internal_temperature/internal_temperature_esp32.cpp b/esphome/components/internal_temperature/internal_temperature_esp32.cpp index 09121fa9c9..1c44a9a238 100644 --- a/esphome/components/internal_temperature/internal_temperature_esp32.cpp +++ b/esphome/components/internal_temperature/internal_temperature_esp32.cpp @@ -19,12 +19,6 @@ namespace esphome::internal_temperature { static const char *const TAG = "internal_temperature.esp32"; -#if defined(USE_ESP32_VARIANT_ESP32C2) || defined(USE_ESP32_VARIANT_ESP32C3) || defined(USE_ESP32_VARIANT_ESP32C5) || \ - defined(USE_ESP32_VARIANT_ESP32C6) || defined(USE_ESP32_VARIANT_ESP32C61) || defined(USE_ESP32_VARIANT_ESP32H2) || \ - defined(USE_ESP32_VARIANT_ESP32P4) || defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3) -static temperature_sensor_handle_t tsensNew = NULL; -#endif // USE_ESP32_VARIANT - void InternalTemperatureSensor::update() { float temperature = NAN; bool success = false; @@ -37,7 +31,7 @@ void InternalTemperatureSensor::update() { defined(USE_ESP32_VARIANT_ESP32C5) || defined(USE_ESP32_VARIANT_ESP32C6) || defined(USE_ESP32_VARIANT_ESP32C61) || \ defined(USE_ESP32_VARIANT_ESP32H2) || defined(USE_ESP32_VARIANT_ESP32P4) || defined(USE_ESP32_VARIANT_ESP32S2) || \ defined(USE_ESP32_VARIANT_ESP32S3) - esp_err_t result = temperature_sensor_get_celsius(tsensNew, &temperature); + esp_err_t result = temperature_sensor_get_celsius(this->tsens_, &temperature); success = (result == ESP_OK); if (!success) { ESP_LOGE(TAG, "Reading failed (%d)", result); @@ -60,14 +54,14 @@ void InternalTemperatureSensor::setup() { defined(USE_ESP32_VARIANT_ESP32P4) || defined(USE_ESP32_VARIANT_ESP32S2) || defined(USE_ESP32_VARIANT_ESP32S3) temperature_sensor_config_t tsens_config = TEMPERATURE_SENSOR_CONFIG_DEFAULT(-10, 80); - esp_err_t result = temperature_sensor_install(&tsens_config, &tsensNew); + esp_err_t result = temperature_sensor_install(&tsens_config, &this->tsens_); if (result != ESP_OK) { ESP_LOGE(TAG, "Install failed (%d)", result); this->mark_failed(); return; } - result = temperature_sensor_enable(tsensNew); + result = temperature_sensor_enable(this->tsens_); if (result != ESP_OK) { ESP_LOGE(TAG, "Enabling failed (%d)", result); this->mark_failed(); diff --git a/esphome/components/logger/logger_esp32.cpp b/esphome/components/logger/logger_esp32.cpp index b216a5427d..05fc959ceb 100644 --- a/esphome/components/logger/logger_esp32.cpp +++ b/esphome/components/logger/logger_esp32.cpp @@ -30,7 +30,7 @@ namespace esphome::logger { static const char *const TAG = "logger"; #ifdef USE_LOGGER_UART_SELECTION_USB_SERIAL_JTAG -static void init_usb_serial_jtag_() { +static void init_usb_serial_jtag() { setvbuf(stdin, NULL, _IONBF, 0); // Disable buffering on stdin #if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 3, 0) @@ -109,7 +109,7 @@ void Logger::pre_setup() { #ifdef USE_LOGGER_USB_SERIAL_JTAG case UART_SELECTION_USB_SERIAL_JTAG: #ifdef USE_LOGGER_UART_SELECTION_USB_SERIAL_JTAG - init_usb_serial_jtag_(); + init_usb_serial_jtag(); #endif break; #endif diff --git a/esphome/components/ota/ota_backend_esp_idf.h b/esphome/components/ota/ota_backend_esp_idf.h index 73dd685df6..a49a5e34b3 100644 --- a/esphome/components/ota/ota_backend_esp_idf.h +++ b/esphome/components/ota/ota_backend_esp_idf.h @@ -54,9 +54,10 @@ class IDFOTABackend final { #endif private: + // Keep md5_ first since its digest_ is alignas(32) on DMA-SHA variants; md5_set_ stays last so buf_ packs tightly. + md5::MD5Digest md5_{}; esp_ota_handle_t update_handle_{0}; const esp_partition_t *partition_; - md5::MD5Digest md5_{}; char expected_bin_md5_[32]; bool md5_set_{false}; #ifdef USE_OTA_PARTITIONS