From 7fe92085b4ab3ce3c4b67e5488ee85bddb7ffded Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 15 Sep 2025 21:28:42 -0500 Subject: [PATCH 1/6] preen --- esphome/components/json/json_util.cpp | 13 +++---------- esphome/components/json/json_util.h | 10 ++++++---- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/esphome/components/json/json_util.cpp b/esphome/components/json/json_util.cpp index 40a3496981..e03f95fe7c 100644 --- a/esphome/components/json/json_util.cpp +++ b/esphome/components/json/json_util.cpp @@ -31,6 +31,7 @@ struct SpiRamAllocator : ArduinoJson::Allocator { protected: RAMAllocator allocator_{RAMAllocator(RAMAllocator::NONE)}; }; + #endif std::string build_json(const json_build_t &f) { @@ -73,22 +74,14 @@ bool parse_json(const std::string &data, const json_parse_t &f) { JsonBuilder::JsonBuilder() : doc_( #ifdef USE_PSRAM - [this]() { - auto *alloc = new SpiRamAllocator(); // NOLINT(cppcoreguidelines-owning-memory) - allocator_ = alloc; - return alloc; - }() + (allocator_ = std::make_unique(), allocator_.get()) #else nullptr #endif ) { } -JsonBuilder::~JsonBuilder() { -#ifdef USE_PSRAM - delete static_cast(allocator_); // NOLINT(cppcoreguidelines-owning-memory) -#endif -} +JsonBuilder::~JsonBuilder() = default; std::string JsonBuilder::serialize() { if (doc_.overflowed()) { diff --git a/esphome/components/json/json_util.h b/esphome/components/json/json_util.h index 8eac87b10a..633e8182fb 100644 --- a/esphome/components/json/json_util.h +++ b/esphome/components/json/json_util.h @@ -25,6 +25,9 @@ std::string build_json(const json_build_t &f); /// Parse a JSON string and run the provided json parse function if it's valid. bool parse_json(const std::string &data, const json_parse_t &f); +// Forward declaration to avoid exposing implementation details +struct SpiRamAllocator; + /// Builder class for creating JSON documents without lambdas class JsonBuilder { public: @@ -42,13 +45,12 @@ class JsonBuilder { std::string serialize(); private: +#ifdef USE_PSRAM + std::unique_ptr allocator_; // One heap allocation, but keeps code clean +#endif JsonDocument doc_; JsonObject root_; bool root_created_{false}; - // Allocator must be last member to ensure it's destroyed after doc_ -#ifdef USE_PSRAM - void *allocator_{nullptr}; // Will store SpiRamAllocator*, managed in cpp file -#endif }; } // namespace json From b0b207eddbfd3a5be36e7ede8a41d7bb4947701c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 15 Sep 2025 21:34:19 -0500 Subject: [PATCH 2/6] cleanup --- esphome/components/json/json_util.cpp | 20 ++++++++++++++------ esphome/components/json/json_util.h | 7 +++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/esphome/components/json/json_util.cpp b/esphome/components/json/json_util.cpp index e03f95fe7c..a9cf383a18 100644 --- a/esphome/components/json/json_util.cpp +++ b/esphome/components/json/json_util.cpp @@ -71,17 +71,25 @@ bool parse_json(const std::string &data, const json_parse_t &f) { } // JsonBuilder implementation -JsonBuilder::JsonBuilder() - : doc_( +JsonBuilder::JsonBuilder() { #ifdef USE_PSRAM - (allocator_ = std::make_unique(), allocator_.get()) + // Verify our storage is large enough (and log the actual size for reference) + static_assert(sizeof(SpiRamAllocator) <= sizeof(allocator_storage_), "allocator_storage_ too small"); + // Note: sizeof(SpiRamAllocator) is typically around 24-32 bytes on ESP32 + // Use placement new to construct SpiRamAllocator in the pre-allocated storage + auto *allocator = new (allocator_storage_) SpiRamAllocator(); + doc_ = JsonDocument(allocator); #else - nullptr + doc_ = JsonDocument(); #endif - ) { } -JsonBuilder::~JsonBuilder() = default; +JsonBuilder::~JsonBuilder() { +#ifdef USE_PSRAM + // Explicitly call destructor for placement-new allocated object + reinterpret_cast(allocator_storage_)->~SpiRamAllocator(); +#endif +} std::string JsonBuilder::serialize() { if (doc_.overflowed()) { diff --git a/esphome/components/json/json_util.h b/esphome/components/json/json_util.h index 633e8182fb..de76ca53da 100644 --- a/esphome/components/json/json_util.h +++ b/esphome/components/json/json_util.h @@ -25,9 +25,6 @@ std::string build_json(const json_build_t &f); /// Parse a JSON string and run the provided json parse function if it's valid. bool parse_json(const std::string &data, const json_parse_t &f); -// Forward declaration to avoid exposing implementation details -struct SpiRamAllocator; - /// Builder class for creating JSON documents without lambdas class JsonBuilder { public: @@ -46,7 +43,9 @@ class JsonBuilder { private: #ifdef USE_PSRAM - std::unique_ptr allocator_; // One heap allocation, but keeps code clean + // Storage for SpiRamAllocator - typically around 24-32 bytes on ESP32 + // Static assert in .cpp file ensures this is large enough + std::aligned_storage<32, alignof(void *)>::type allocator_storage_; #endif JsonDocument doc_; JsonObject root_; From 7549d031fdd3ccb52f1cef8c5457fbba8ccafcdf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 15 Sep 2025 21:40:30 -0500 Subject: [PATCH 3/6] cleanup --- esphome/components/json/json_util.cpp | 46 +++------------------------ esphome/components/json/json_util.h | 23 +++++++++++--- 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/esphome/components/json/json_util.cpp b/esphome/components/json/json_util.cpp index a9cf383a18..166fbcd167 100644 --- a/esphome/components/json/json_util.cpp +++ b/esphome/components/json/json_util.cpp @@ -8,32 +8,6 @@ namespace json { static const char *const TAG = "json"; -#ifdef USE_PSRAM -// Build an allocator for the JSON Library using the RAMAllocator class -// This is only compiled when PSRAM is enabled -struct SpiRamAllocator : ArduinoJson::Allocator { - void *allocate(size_t size) override { return this->allocator_.allocate(size); } - - void deallocate(void *pointer) override { - // ArduinoJson's Allocator interface doesn't provide the size parameter in deallocate. - // RAMAllocator::deallocate() requires the size, which we don't have access to here. - // RAMAllocator::deallocate implementation just calls free() regardless of whether - // the memory was allocated with heap_caps_malloc or malloc. - // This is safe because ESP-IDF's heap implementation internally tracks the memory region - // and routes free() to the appropriate heap. - free(pointer); // NOLINT(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc) - } - - void *reallocate(void *ptr, size_t new_size) override { - return this->allocator_.reallocate(static_cast(ptr), new_size); - } - - protected: - RAMAllocator allocator_{RAMAllocator(RAMAllocator::NONE)}; -}; - -#endif - std::string build_json(const json_build_t &f) { // NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks) false positive with ArduinoJson JsonBuilder builder; @@ -71,24 +45,14 @@ bool parse_json(const std::string &data, const json_parse_t &f) { } // JsonBuilder implementation -JsonBuilder::JsonBuilder() { +JsonBuilder::JsonBuilder() + : doc_( #ifdef USE_PSRAM - // Verify our storage is large enough (and log the actual size for reference) - static_assert(sizeof(SpiRamAllocator) <= sizeof(allocator_storage_), "allocator_storage_ too small"); - // Note: sizeof(SpiRamAllocator) is typically around 24-32 bytes on ESP32 - // Use placement new to construct SpiRamAllocator in the pre-allocated storage - auto *allocator = new (allocator_storage_) SpiRamAllocator(); - doc_ = JsonDocument(allocator); + &allocator_ #else - doc_ = JsonDocument(); -#endif -} - -JsonBuilder::~JsonBuilder() { -#ifdef USE_PSRAM - // Explicitly call destructor for placement-new allocated object - reinterpret_cast(allocator_storage_)->~SpiRamAllocator(); + nullptr #endif + ) { } std::string JsonBuilder::serialize() { diff --git a/esphome/components/json/json_util.h b/esphome/components/json/json_util.h index de76ca53da..9699955133 100644 --- a/esphome/components/json/json_util.h +++ b/esphome/components/json/json_util.h @@ -13,6 +13,24 @@ namespace esphome { namespace json { +#ifdef USE_PSRAM +// Allocator for JSON that uses PSRAM on supported devices +struct SpiRamAllocator : ArduinoJson::Allocator { + void *allocate(size_t size) override { return allocator_.allocate(size); } + + void deallocate(void *ptr) override { + free(ptr); // NOLINT(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc) + } + + void *reallocate(void *ptr, size_t new_size) override { + return allocator_.reallocate(static_cast(ptr), new_size); + } + + protected: + RAMAllocator allocator_{RAMAllocator::NONE}; +}; +#endif + /// Callback function typedef for parsing JsonObjects. using json_parse_t = std::function; @@ -29,7 +47,6 @@ bool parse_json(const std::string &data, const json_parse_t &f); class JsonBuilder { public: JsonBuilder(); - ~JsonBuilder(); JsonObject root() { if (!root_created_) { @@ -43,9 +60,7 @@ class JsonBuilder { private: #ifdef USE_PSRAM - // Storage for SpiRamAllocator - typically around 24-32 bytes on ESP32 - // Static assert in .cpp file ensures this is large enough - std::aligned_storage<32, alignof(void *)>::type allocator_storage_; + SpiRamAllocator allocator_; // Just a regular member on the stack! #endif JsonDocument doc_; JsonObject root_; From 7aae946678161c63a0648935f7cbf55df4022120 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 15 Sep 2025 21:44:50 -0500 Subject: [PATCH 4/6] cleanup --- esphome/components/json/json_util.cpp | 11 ----------- esphome/components/json/json_util.h | 8 ++++---- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/esphome/components/json/json_util.cpp b/esphome/components/json/json_util.cpp index 166fbcd167..51c0fcf9cb 100644 --- a/esphome/components/json/json_util.cpp +++ b/esphome/components/json/json_util.cpp @@ -44,17 +44,6 @@ bool parse_json(const std::string &data, const json_parse_t &f) { // NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks) } -// JsonBuilder implementation -JsonBuilder::JsonBuilder() - : doc_( -#ifdef USE_PSRAM - &allocator_ -#else - nullptr -#endif - ) { -} - std::string JsonBuilder::serialize() { if (doc_.overflowed()) { ESP_LOGE(TAG, "JSON document overflow"); diff --git a/esphome/components/json/json_util.h b/esphome/components/json/json_util.h index 9699955133..d85e7eefe0 100644 --- a/esphome/components/json/json_util.h +++ b/esphome/components/json/json_util.h @@ -46,8 +46,6 @@ bool parse_json(const std::string &data, const json_parse_t &f); /// Builder class for creating JSON documents without lambdas class JsonBuilder { public: - JsonBuilder(); - JsonObject root() { if (!root_created_) { root_ = doc_.to(); @@ -60,9 +58,11 @@ class JsonBuilder { private: #ifdef USE_PSRAM - SpiRamAllocator allocator_; // Just a regular member on the stack! + SpiRamAllocator allocator_; // Just a regular member on the stack! + JsonDocument doc_{&allocator_}; // Initialize with allocator +#else + JsonDocument doc_; // Default initialization #endif - JsonDocument doc_; JsonObject root_; bool root_created_{false}; }; From bd11ffd395773537fa169e8a2a611d17cfc819c0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 15 Sep 2025 21:47:15 -0500 Subject: [PATCH 5/6] preen --- esphome/components/json/json_util.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/esphome/components/json/json_util.h b/esphome/components/json/json_util.h index d85e7eefe0..fb991a7168 100644 --- a/esphome/components/json/json_util.h +++ b/esphome/components/json/json_util.h @@ -14,11 +14,18 @@ namespace esphome { namespace json { #ifdef USE_PSRAM -// Allocator for JSON that uses PSRAM on supported devices +// Build an allocator for the JSON Library using the RAMAllocator class +// This is only compiled when PSRAM is enabled struct SpiRamAllocator : ArduinoJson::Allocator { void *allocate(size_t size) override { return allocator_.allocate(size); } void deallocate(void *ptr) override { + // ArduinoJson's Allocator interface doesn't provide the size parameter in deallocate. + // RAMAllocator::deallocate() requires the size, which we don't have access to here. + // RAMAllocator::deallocate implementation just calls free() regardless of whether + // the memory was allocated with heap_caps_malloc or malloc. + // This is safe because ESP-IDF's heap implementation internally tracks the memory region + // and routes free() to the appropriate heap. free(ptr); // NOLINT(cppcoreguidelines-owning-memory,cppcoreguidelines-no-malloc) } From 192e935ef23ff50597208122213e06ee56ede39a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 15 Sep 2025 21:47:18 -0500 Subject: [PATCH 6/6] preen --- esphome/components/json/json_util.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/esphome/components/json/json_util.h b/esphome/components/json/json_util.h index fb991a7168..69b809ec49 100644 --- a/esphome/components/json/json_util.h +++ b/esphome/components/json/json_util.h @@ -65,10 +65,10 @@ class JsonBuilder { private: #ifdef USE_PSRAM - SpiRamAllocator allocator_; // Just a regular member on the stack! - JsonDocument doc_{&allocator_}; // Initialize with allocator + SpiRamAllocator allocator_; + JsonDocument doc_{&allocator_}; #else - JsonDocument doc_; // Default initialization + JsonDocument doc_; #endif JsonObject root_; bool root_created_{false};