minor improvements for mem fragmentation

src/devices/thermostat.cpp

@@ -1183,7 +1183,7 @@ void Thermostat::set_settings_calinttemp(const char * value, const int8_t id) {
         return;
     }
 
-    // TODO: Michael - does this value need to be multiple by 10?
+    // does this value need to be multiple by 10?
     if (((flags() & 0x0F) == EMS_DEVICE_FLAG_RC30_1) || ((flags() & 0x0F) == EMS_DEVICE_FLAG_RC35)) {
         LOG_INFO(F("Calibrating internal temperature to %d.%d"), ct / 10, ct < 0 ? -ct % 10 : ct % 10);
         write_command(EMS_TYPE_IBASettings, 2, ct);
@@ -1219,7 +1219,7 @@ void Thermostat::set_remotetemp(const char * value, const int8_t id) {
 
 // 0xA5 - Set the building settings
 void Thermostat::set_settings_building(const char * value, const int8_t id) {
-    std::string bd;
+    std::string bd(20, '\0');
     if (!Helpers::value2string(value, bd)) {
         return;
     }
@@ -1283,7 +1283,7 @@ void Thermostat::set_control(const char * value, const int8_t id) {
 
 // sets the thermostat ww working mode, where mode is a string
 void Thermostat::set_wwmode(const char * value, const int8_t id) {
-    std::string v;
+    std::string v(10, '\0');
     if (!Helpers::value2string(value, v)) {
         return;
     }
@@ -1307,7 +1307,7 @@ void Thermostat::set_wwmode(const char * value, const int8_t id) {
 
 // set the holiday as string dd.mm.yyyy-dd.mm.yyyy
 void Thermostat::set_holiday(const char * value, const int8_t id) {
-    std::string hd;
+    std::string hd(30, '\0');
     if (!Helpers::value2string(value, hd)) {
         return;
     }
@@ -1379,7 +1379,7 @@ void Thermostat::set_party(const char * value, const int8_t id) {
 // dw - day of week (0..6), dst- summertime (0/1)
 // id is ignored
 void Thermostat::set_datetime(const char * value, const int8_t id) {
-    std::string dt;
+    std::string dt(30, '\0');
     if (!Helpers::value2string(value, dt)) {
         return;
     }
@@ -1424,7 +1424,7 @@ void Thermostat::set_datetime(const char * value, const int8_t id) {
 // sets the thermostat working mode, where mode is a string
 // converts string mode to HeatingCircuit::Mode
 void Thermostat::set_mode(const char * value, const int8_t id) {
-    std::string mode;
+    std::string mode(10, '\0');
     if (!Helpers::value2string(value, mode)) {
         return;
     }

src/emsdevice.cpp

@@ -258,7 +258,7 @@ EMSdevice::TelegramFunction::TelegramFunction(uint16_t                    telegr
 
 // register a call back function for a specific telegram type
 void EMSdevice::register_telegram_type(const uint16_t telegram_type_id, const __FlashStringHelper * telegram_type_name, bool fetch, process_function_p f) {
-    telegram_functions_.emplace_back(telegram_type_id, telegram_type_name, fetch, f);
+    telegram_functions_.emplace_back(telegram_type_id, std::move(telegram_type_name), fetch, std::move(f));
 }
 
 // return the name of the telegram type

src/emsesp.cpp

@@ -206,7 +206,7 @@ void EMSESP::show_ems(uuid::console::Shell & shell) {
     } else {
         shell.printfln(F("Tx Queue (%ld telegram%s):"), tx_telegrams.size(), tx_telegrams.size() == 1 ? "" : "s");
 
-        std::string op;
+        std::string op(10, '\0');
         for (const auto & it : tx_telegrams) {
             if ((it.telegram_->operation) == Telegram::Operation::TX_RAW) {
                 op = read_flash_string(F("RAW  "));
@@ -295,9 +295,9 @@ std::string EMSESP::pretty_telegram(std::shared_ptr<const Telegram> telegram) {
     uint8_t offset = telegram->offset;
 
     // find name for src and dest by looking up known devices
-    std::string src_name;
+    std::string src_name(20, '\0');
-    std::string dest_name;
+    std::string dest_name(20, '\0');
-    std::string type_name;
+    std::string type_name(20, '\0');
     for (const auto & emsdevice : emsdevices) {
         if (emsdevice) {
             // get src & dest
@@ -541,9 +541,6 @@ void EMSESP::show_devices(uuid::console::Shell & shell) {
         // shell.printf(F("[factory ID: %d] "), device_class.first);
         for (const auto & emsdevice : emsdevices) {
             if ((emsdevice) && (emsdevice->device_type() == device_class.first)) {
-#if defined(EMSESP_DEBUG)
-                shell.printf(F("[id=%d] "), emsdevice->unique_id());
-#endif
                 shell.printf(F("%s: %s"), emsdevice->device_type_name().c_str(), emsdevice->to_string().c_str());
                 if ((emsdevice->device_type() == EMSdevice::DeviceType::THERMOSTAT) && (emsdevice->device_id() == actual_master_thermostat())) {
                     shell.printf(F(" ** master device **"));
@@ -613,12 +610,11 @@ bool EMSESP::add_device(const uint8_t device_id, const uint8_t product_id, std::
         LOG_NOTICE(F("Unrecognized EMS device with device ID 0x%02X with product ID %d. Please report on GitHub."), device_id, product_id);
         return false; // not found
     } else {
-        emsdevices.push_back(
+        std::string name = uuid::read_flash_string(device_p->name);
-            EMSFactory::add(device_p->device_type, device_id, device_p->product_id, version, uuid::read_flash_string(device_p->name), device_p->flags, brand));
+        emsdevices.push_back(EMSFactory::add(device_p->device_type, device_id, device_p->product_id, version, name, device_p->flags, brand));
         emsdevices.back()->unique_id(++unique_id_count_);
         LOG_DEBUG(F("Adding new device with device ID 0x%02X with product ID %d and version %s"), device_id, product_id, version.c_str());
-        // go and fetch its data,
+        fetch_device_values(device_id); // go and fetch its data,
-        fetch_device_values(device_id);
     }
 
     return true;

src/emsfactory.h

@@ -55,12 +55,12 @@ class EMSFactory {
     }
 
     // Construct derived class returning an unique ptr
-    static auto add(const uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string version, std::string name, uint8_t flags, uint8_t brand)
+    static auto add(const uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string &version, std::string &name, uint8_t flags, uint8_t brand)
         -> std::unique_ptr<EMSdevice> {
         return std::unique_ptr<EMSdevice>(EMSFactory::makeRaw(device_type, device_id, product_id, version, name, flags, brand));
     }
 
-    virtual auto construct(uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string version, std::string name, uint8_t flags, uint8_t brand) const
+    virtual auto construct(uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string &version, std::string &name, uint8_t flags, uint8_t brand) const
         -> EMSdevice * = 0;
 
   private:
@@ -72,7 +72,7 @@ class EMSFactory {
 
     // Construct derived class returning a raw pointer
     // find which EMS device it is and use that class
-    static auto makeRaw(const uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string version, std::string name, uint8_t flags, uint8_t brand)
+    static auto makeRaw(const uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string &version, std::string &name, uint8_t flags, uint8_t brand)
         -> EMSdevice * {
         auto it = EMSFactory::getRegister().find(device_type);
         if (it != EMSFactory::getRegister().end()) {
@@ -90,7 +90,7 @@ class ConcreteEMSFactory : EMSFactory {
         EMSFactory::registerFactory(device_type, this);
     }
 
-    auto construct(uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string version, std::string name, uint8_t flags, uint8_t brand) const
+    auto construct(uint8_t device_type, uint8_t device_id, uint8_t product_id, std::string &version, std::string &name, uint8_t flags, uint8_t brand) const
         -> EMSdevice * {
         return new DerivedClass(device_type, device_id, product_id, version, name, flags, brand);
     }

src/mqtt.cpp

@@ -47,10 +47,10 @@ Mqtt::QueuedMqttMessage::QueuedMqttMessage(uint16_t id, std::shared_ptr<MqttMess
     packet_id_   = 0;
 }
 
-MqttMessage::MqttMessage(const uint8_t operation, const std::string & topic, const std::string & payload, bool retain)
+MqttMessage::MqttMessage(const uint8_t operation, const std::string & topic, const std::string && payload, bool retain)
     : operation(operation)
     , topic(topic)
-    , payload(payload)
+    , payload(std::move(payload))
     , retain(retain) {
 }
 
@@ -84,19 +84,19 @@ void Mqtt::subscribe(const uint8_t device_type, const std::string & topic, mqtt_
 
     // register in our libary with the callback function.
     // We store both the original topic and the fully-qualified
-    mqtt_subfunctions_.emplace_back(device_type, std::move(topic), std::move(message->topic), cb);
+    mqtt_subfunctions_.emplace_back(device_type, std::move(topic), std::move(message->topic), std::move(cb));
 }
 
 // adds a command and callback function for a specific device
 void Mqtt::add_command(const uint8_t device_type, const __FlashStringHelper * cmd, mqtt_cmdfunction_p cb) {
     // subscribe to the command topic if it doesn't exist yet
     // create the cmd topic for a device like "<device_type>_cmd" e.g. "boiler_cmd"
-    // unless its a system MQTT command
+    // unless its a system MQTT command, then its system_cmd
     std::string cmd_topic(40, '\0');
     if (device_type == EMSdevice::DeviceType::SERVICEKEY) {
         cmd_topic = MQTT_SYSTEM_CMD; // hard-coded system
     } else {
-        snprintf_P(&cmd_topic[0], 40, PSTR("%s_cmd"), EMSdevice::device_type_topic_name(device_type).c_str());
+        snprintf_P(&cmd_topic[0], 41, PSTR("%s_cmd"), EMSdevice::device_type_topic_name(device_type).c_str());
     }
 
     bool exists = false;
@@ -112,7 +112,7 @@ void Mqtt::add_command(const uint8_t device_type, const __FlashStringHelper * cm
     }
 
     // add the function to our list
-    mqtt_cmdfunctions_.emplace_back(device_type, cmd, cb);
+    mqtt_cmdfunctions_.emplace_back(device_type, std::move(cmd), std::move(cb));
 }
 
 // subscribe to an MQTT topic, and store the associated callback function. For generic functions not tied to a specific device
@@ -451,11 +451,12 @@ std::shared_ptr<const MqttMessage> Mqtt::queue_message(const uint8_t operation,
     std::shared_ptr<MqttMessage> message;
     if ((strncmp(topic.c_str(), "homeassistant/", 13) == 0)) {
         // leave topic as it is
-        message = std::make_shared<MqttMessage>(operation, topic, payload, retain);
+        message = std::make_shared<MqttMessage>(operation, topic, std::move(payload), retain);
     } else {
         // prefix the hostname
-        std::string full_topic = Mqtt::hostname_ + "/" + topic;
+        std::string full_topic(20, '\0');
-        message                = std::make_shared<MqttMessage>(operation, full_topic, payload, retain);
+        snprintf_P(&full_topic[0], full_topic.capacity() + 1, PSTR("%s/%s"), Mqtt::hostname_.c_str(), topic.c_str());
+        message = std::make_shared<MqttMessage>(operation, full_topic, std::move(payload), retain);
     }
 
     // if the queue is full, make room but removing the last one

src/mqtt.h

@@ -49,7 +49,7 @@ using mqtt_cmdfunction_p = std::function<void(const char * data, const int8_t id
 using namespace std::placeholders; // for `_1`
 
 struct MqttMessage {
-    MqttMessage(const uint8_t operation, const std::string & topic, const std::string & payload, bool retain);
+    MqttMessage(const uint8_t operation, const std::string & topic, const std::string && payload, bool retain);
     ~MqttMessage() = default;
 
     const uint8_t     operation;

src/system.cpp

@@ -190,6 +190,18 @@ void System::loop() {
             send_heartbeat();
         }
     }
+
+#if defined(ESP8266)
+#if defined(EMSESP_DEBUG)
+    static uint32_t last_memcheck_ = 0;
+    if (currentMillis - last_memcheck_ > 8000) {
+        last_memcheck_ = currentMillis;
+        LOG_INFO(F("Free heap: %d%% (%lu), frag:%u%%"), free_mem(), (unsigned long)ESP.getFreeHeap(), ESP.getHeapFragmentation());
+    }
+#elif defined(ESP32)
+
+#endif
+#endif
 }
 
 // send periodic MQTT message with system information
@@ -324,7 +336,7 @@ void System::show_system(uuid::console::Shell & shell) {
     shell.printfln(F("Free heap:                %lu bytes"), (unsigned long)ESP.getFreeHeap());
     shell.printfln(F("Free mem:                 %d %%"), free_mem());
     shell.printfln(F("Maximum free block size:  %lu bytes"), (unsigned long)ESP.getMaxFreeBlockSize());
-    shell.printfln(F("Heap fragmentation:       %u%"), ESP.getHeapFragmentation());
+    shell.printfln(F("Heap fragmentation:       %u %%"), ESP.getHeapFragmentation());
     shell.printfln(F("Free continuations stack: %lu bytes"), (unsigned long)ESP.getFreeContStack());
 #elif defined(ESP32)
     shell.printfln(F("SDK version:   %s"), ESP.getSdkVersion());