From 138c7b5cd8cc2a5a20ef98a7540320dd329f4c7f Mon Sep 17 00:00:00 2001 From: Nicu Hodos Date: Thu, 17 Oct 2024 17:15:43 +0200 Subject: [PATCH 1/3] read heap stats once and put them in both sensors and json attributes --- src/esp.h | 19 +++++++++++++++---- src/ha.h | 5 ++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/esp.h b/src/esp.h index 0fe1095..7180f6b 100644 --- a/src/esp.h +++ b/src/esp.h @@ -12,9 +12,16 @@ namespace HaESP { } activeSensors; Task tHeap(5 * TASK_MINUTE, TASK_FOREVER, []() { - Sensor::mapSensors["heap_fragmentation"]->updateState(to_string(ESP.getHeapFragmentation()).c_str()); - Sensor::mapSensors["heap_free"]->updateState(to_string(ESP.getFreeHeap()).c_str()); - Sensor::mapSensors["heap_max_free_block"]->updateState(to_string(ESP.getMaxFreeBlockSize()).c_str()); + uint32_t hfree; + uint32_t hmax; + uint8_t hfrag; + ESP.getHeapStats(&hfree, &hmax, &hfrag); + + char value[256]; + sprintf(value, "{\"fragmentation\":%d,\"heap\":{\"Heap free\":\"%d B\",\"Heap max free block\":\"%d B\"}}", hfrag, hfree, hmax); + Sensor::mapSensors["heap_fragmentation"]->updateState(value); + Sensor::mapSensors["heap_free"]->updateState(to_string(hfree).c_str()); + Sensor::mapSensors["heap_max_free_block"]->updateState(to_string(hmax).c_str()); }, &ts); Task tRestartInfo(TASK_IMMEDIATE, TASK_ONCE, []() { @@ -23,9 +30,13 @@ namespace HaESP { template Builder& heapStats(Builder& builder) { - builder.addDiagnostic(Builder::instance(new Sensor{ "Heap fragmentation", "heap_fragmentation" }) + auto heap = new Sensor{ "Heap fragmentation", "heap_fragmentation" }; + builder.addDiagnostic(Builder::instance(heap) .withUnitMeasure("%") .withPrecision(0) + .withValueTemplate("{{ value_json.fragmentation }}") + .overrideConfig("json_attributes_topic", heap->stateTopic) + .overrideConfig("json_attributes_template", "{{ value_json.heap | tojson }}") .build()); builder.addDiagnostic(Builder::instance(new Sensor{ "Heap free", "heap_free" }) .withDeviceClass("data_size") diff --git a/src/ha.h b/src/ha.h index 7103b45..364fef8 100644 --- a/src/ha.h +++ b/src/ha.h @@ -205,8 +205,7 @@ namespace Ha { return *this; } - template - Builder& overrideConfig(const char* key, V value) { + Builder& overrideConfig(const char* key, float value) { cmp->jsonNumbers.add({key, value}); return *this; } @@ -293,6 +292,7 @@ namespace Ha { struct StateConfig { static unordered_map mapStateTopics; + char stateTopic[TOPIC_SIZE]; StateConfig(Component* cmp) : cmp(cmp) {} @@ -305,7 +305,6 @@ namespace Ha { } protected: - char stateTopic[TOPIC_SIZE]; Component* cmp; void buildConfig(JsonDocument& jsonDoc) { if (stateTopic[0]) jsonDoc["state_topic"] = stateTopic; From baa3d2d0b868692f85cd8b0a8c4d0953e3989a70 Mon Sep 17 00:00:00 2001 From: Nicu Hodos Date: Fri, 18 Oct 2024 09:14:49 +0200 Subject: [PATCH 2/3] - avoid possible buffer overflows by using snprintf - uniqueId is a field now - optimizes string copy - store long strings in FLASH memory --- src/esp.h | 4 ++-- src/ha.h | 27 +++++++++++++-------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/esp.h b/src/esp.h index 7180f6b..cfa0034 100644 --- a/src/esp.h +++ b/src/esp.h @@ -17,8 +17,8 @@ namespace HaESP { uint8_t hfrag; ESP.getHeapStats(&hfree, &hmax, &hfrag); - char value[256]; - sprintf(value, "{\"fragmentation\":%d,\"heap\":{\"Heap free\":\"%d B\",\"Heap max free block\":\"%d B\"}}", hfrag, hfree, hmax); + char value[128]; + snprintf_P(value, sizeof(value), PSTR("{\"fragmentation\":%d,\"heap\":{\"Heap free\":\"%d B\",\"Heap max free block\":\"%d B\"}}"), hfrag, hfree, hmax); Sensor::mapSensors["heap_fragmentation"]->updateState(value); Sensor::mapSensors["heap_free"]->updateState(to_string(hfree).c_str()); Sensor::mapSensors["heap_max_free_block"]->updateState(to_string(hmax).c_str()); diff --git a/src/ha.h b/src/ha.h index 364fef8..0299def 100644 --- a/src/ha.h +++ b/src/ha.h @@ -7,6 +7,7 @@ using namespace std; #define JSON_SIZE 512 #define TOPIC_SIZE 255 +#define BASE_TOPIC "homeassistant/%s/%s/%s" namespace Ha { uint16_t(*publisher)(const char* topic, const char* message); @@ -116,12 +117,13 @@ namespace Ha { } protected: - virtual void buildUniqueId(char* uniqueId) { - sprintf(uniqueId, "%s_%s", MAIN_DEVICE_ID, id); + char uniqueId[50]; + virtual void buildUniqueId() { + snprintf(uniqueId, sizeof(uniqueId), "%s_%s", MAIN_DEVICE_ID, id); } virtual void buildConfigTopic() { - sprintf(configTopic, "homeassistant/%s/%s/%s/config", type, MAIN_DEVICE_ID, id); + snprintf(configTopic, sizeof(configTopic), BASE_TOPIC"/config", type, MAIN_DEVICE_ID, id); } virtual void buildComponentConfig(JsonDocument& jsonDoc) = 0; @@ -131,8 +133,7 @@ namespace Ha { if (entityCategory) jsonDoc["entity_category"] = entityCategory; if (deviceClass) jsonDoc["device_class"] = deviceClass; jsonDoc["name"] = name; - char uniqueId[50]; - buildUniqueId(uniqueId); + buildUniqueId(); jsonDoc["unique_id"] = uniqueId; buildConfigTopic(); @@ -261,7 +262,7 @@ namespace Ha { static unordered_map mapCommands; Command(const char* name, const char* id, const char* type, onMessage f) : Component(name, id, type), f(f) { - sprintf(commandTopic, "homeassistant/%s/%s/%s/set", type, MAIN_DEVICE_ID, id); + snprintf(commandTopic, sizeof(commandTopic), BASE_TOPIC"/set", type, MAIN_DEVICE_ID, id); mapCommands.insert({ string(commandTopic), this }); } @@ -297,7 +298,7 @@ namespace Ha { StateConfig(Component* cmp) : cmp(cmp) {} void withStateTopic() { - sprintf(stateTopic, "homeassistant/%s/%s/%s/state", cmp->type, MAIN_DEVICE_ID, cmp->id); + snprintf(stateTopic, sizeof(stateTopic), BASE_TOPIC"/state", cmp->type, MAIN_DEVICE_ID, cmp->id); } void updateState(const char* message) { @@ -336,9 +337,7 @@ namespace Ha { : Command(name, id, "number", f), StateConfig(this), min(min), max(max), step(step) {} void updateState(unsigned int value) { - char message[32]; - sprintf(message, "%u", value); - StateConfig::updateState(message); + StateConfig::updateState(to_string(value).c_str()); } void restoreFromState() { @@ -366,17 +365,17 @@ namespace Ha { } protected: - void buildUniqueId(char* uniqueId) override { + void buildUniqueId() override { if (multiValueComponent && deviceClass) { - sprintf(uniqueId, "%s_%s_%s", MAIN_DEVICE_ID, deviceClass, id); + snprintf(uniqueId, sizeof(uniqueId), "%s_%s_%s", MAIN_DEVICE_ID, deviceClass, id); } else { - Component::buildUniqueId(uniqueId); + Component::buildUniqueId(); } } void buildConfigTopic() override { if (multiValueComponent && deviceClass) { - sprintf(configTopic, "homeassistant/%s/%s/%s_%s/config", type, MAIN_DEVICE_ID, deviceClass, id); + snprintf(configTopic, sizeof(configTopic), BASE_TOPIC"_%s""/config", type, MAIN_DEVICE_ID, deviceClass, id); } else { Component::buildConfigTopic(); } From fe8b33df72de8d1de8a514132d91bc6687a4290b Mon Sep 17 00:00:00 2001 From: Nicu Hodos Date: Fri, 18 Oct 2024 12:18:19 +0200 Subject: [PATCH 3/3] optimize ArduinoJson serialization: avoid copies of strings --- src/ha.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/ha.h b/src/ha.h index 0299def..f1840c1 100644 --- a/src/ha.h +++ b/src/ha.h @@ -88,7 +88,7 @@ namespace Ha { const char* entityCategory = nullptr; const char* deviceClass = nullptr; const char* name = nullptr; - char* id = nullptr; + const char* id = nullptr; const char* type = nullptr; char configTopic[TOPIC_SIZE]; JsonPairs jsonBooleans; @@ -98,7 +98,7 @@ namespace Ha { static List components; bool multiValueComponent = false; - Component(const char* name, const char* id, const char* type) : name(name), id((char*)id), type(type) { + Component(const char* name, const char* id, const char* type) : name(name), id(id), type(type) { components.add(this); } @@ -134,7 +134,7 @@ namespace Ha { if (deviceClass) jsonDoc["device_class"] = deviceClass; jsonDoc["name"] = name; buildUniqueId(); - jsonDoc["unique_id"] = uniqueId; + jsonDoc["unique_id"] = (const char*)uniqueId; buildConfigTopic(); buildComponentConfig(jsonDoc); @@ -276,7 +276,7 @@ namespace Ha { virtual void buildCommandConfig(JsonDocument& jsonDoc) = 0; void buildComponentConfig(JsonDocument& jsonDoc) override { - jsonDoc["command_topic"] = commandTopic; + jsonDoc["command_topic"] = (const char*)commandTopic; if (retain) jsonDoc["retain"] = true; buildCommandConfig(jsonDoc); } @@ -308,7 +308,7 @@ namespace Ha { protected: Component* cmp; void buildConfig(JsonDocument& jsonDoc) { - if (stateTopic[0]) jsonDoc["state_topic"] = stateTopic; + if (stateTopic[0]) jsonDoc["state_topic"] = (const char*)stateTopic; } };