optimize device_info

src/analogsensor.cpp

@@ -700,13 +700,12 @@ bool AnalogSensor::get_value_info(JsonObject output, const char * cmd, const int
             // if we're filtering on an attribute, go find it
             if (attribute_s) {
                 if (output.containsKey(attribute_s)) {
-                    String data = output[attribute_s].as<String>();
+                    std::string data = output[attribute_s].as<std::string>();
                     output.clear();
-                    output["api_data"] = data;
+                    output["api_data"] = data; // always as a string
                     return true;
-                } else {
-                    return EMSESP::return_not_found(output, "attribute", sensor_name); // not found
                 }
+                return EMSESP::return_not_found(output, "attribute", sensor_name); // not found
             }
             return true; // found a match, exit
         }

src/command.cpp

@@ -173,14 +173,13 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
                     return CommandRet::INVALID;
                 }
 
-                // TODO refactor containsKey - not recommended to use
+                const char * api_data = output["api_data"];
-                if (!output.containsKey("api_data")) {
+                if (api_data) {
-                    return CommandRet::INVALID;
+                    output.clear();
+                    return Command::call(device_type, command_p, api_data, is_admin, id_n, output);
                 }
 
-                String dat = output["api_data"].as<String>();
+                return CommandRet::INVALID;
-                output.clear();
-                return Command::call(device_type, command_p, dat.c_str(), is_admin, id_n, output);
             }
         }
     }
@@ -304,16 +303,14 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char *
 
     auto dname = EMSdevice::device_type_2_device_name(device_type); // device name, not translated
 
-    // check first if there is a command given as it may be calling a device's attribute (e.g. /api/boiler/nrgheat)
+    // check first if there is only a command being called without a value
+    // it could be an endpoint like a device's entity or attribute e.g. api/boiler/nrgheat or /api/boiler/nrgheat/value
+    // or a special command like 'info', 'values', 'commands', 'entities' etc
     bool single_command = (!value || !strlen(value));
     if (single_command) {
         if (EMSESP::get_device_value_info(output, cmd, id, device_type)) { // entity = cmd
             LOG_DEBUG("Fetched device entity attributes for %s/%s", dname, cmd);
             return CommandRet::OK;
-        } else {
-            // char error[100];
-            // snprintf(error, sizeof(error), "no data for device %s", dname);
-            // output["message"] = error;
         }
     }
 
@@ -336,6 +333,10 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char *
     auto cf = find_command(device_type, device_id, cmd, flag);
     if (!cf) {
         LOG_WARNING("Command failed: invalid command '%s'", cmd ? cmd : "");
+        // if we don't alread have a message set, set it to invalid command
+        if (!output["message"]) {
+            output["message"] = "invalid command";
+        }
         return CommandRet::ERROR;
     }
 

src/emsdevice.cpp

@@ -1425,7 +1425,7 @@ void EMSdevice::dump_telegram_info(std::vector<TelegramFunctionDump> & telegram_
 }
 #endif
 
-// builds json for a specific device value / entity
+// builds json for a specific EMS device value / entity
 // cmd is the endpoint or name of the device entity
 // returns false if failed, otherwise true
 bool EMSdevice::get_value_info(JsonObject output, const char * cmd, const int8_t tag) {
@@ -1582,6 +1582,9 @@ bool EMSdevice::get_value_info(JsonObject output, const char * cmd, const int8_t
             json["writeable"] = dv.has_cmd && !dv.has_state(DeviceValueState::DV_READONLY);
             json["visible"]   = !dv.has_state(DeviceValueState::DV_WEB_EXCLUDE);
 
+            // TODO refactor to remove containsKey as it's costly and not advisable to use it
+            // https://arduinojson.org/v7/api/jsonobject/containskey/#avoid
+
             // if there is no value, mention it
             if (!json.containsKey(value)) {
                 json[value] = "not set";
@@ -1590,23 +1593,22 @@ bool EMSdevice::get_value_info(JsonObject output, const char * cmd, const int8_t
             // if we're filtering on an attribute, go find it
             if (attribute_s) {
 #if defined(EMSESP_DEBUG)
-                EMSESP::logger().debug("[DEBUG] single attribute '%s'", attribute_s);
+                EMSESP::logger().debug("[DEBUG] fetching single attribute '%s'", attribute_s);
 #endif
                 if (json.containsKey(attribute_s)) {
-                    String data = json[attribute_s].as<String>();
+                    std::string data = json[attribute_s].as<std::string>();
                     output.clear();
-                    output["api_data"] = data;
+                    output["api_data"] = data; // always as string
                     return true;
-                } else {
-                    return EMSESP::return_not_found(output, "attribute", command_s); // not found
                 }
+                return EMSESP::return_not_found(output, "attribute", command_s); // not found
             }
 
             return true;
         }
     }
 
-    return EMSESP::return_not_found(output, "entity data", cmd); // not found
+    return false; // not found, but don't return a message error yet
 }
 
 // mqtt publish all single values from one device (used for time schedule)
@@ -1934,7 +1936,7 @@ std::string EMSdevice::name() {
 // returns true on success.
 int EMSdevice::get_modbus_value(uint8_t tag, const std::string & shortname, std::vector<uint16_t> & result) {
     // find device value by shortname
-    // TODO linear search is inefficient
+    // TODO replace linear search which is inefficient
     const auto & it = std::find_if(devicevalues_.begin(), devicevalues_.end(), [&](const DeviceValue & x) { return x.tag == tag && x.short_name == shortname; });
     if (it == devicevalues_.end())
         return -1;

src/emsesp.cpp

@@ -748,13 +748,12 @@ bool EMSESP::get_device_value_info(JsonObject root, const char * cmd, const int8
     // check first for EMS devices
     for (const auto & emsdevice : emsdevices) {
         if (emsdevice->device_type() == devicetype) {
-            // found device, call its device info
+            return emsdevice->get_value_info(root, cmd, id);
-            if (emsdevice->get_value_info(root, cmd, id)) {
-                return true;
-            }
         }
     }
 
+    // check for other devices...
+
     // temperature sensor
     if (devicetype == DeviceType::TEMPERATURESENSOR) {
         return temperaturesensor_.get_value_info(root, cmd, id);
@@ -785,15 +784,11 @@ bool EMSESP::get_device_value_info(JsonObject root, const char * cmd, const int8
 
 // sends JSON error message, used with API calls
 bool EMSESP::return_not_found(JsonObject output, const char * msg, const char * cmd) {
-    // special case for "info" and "values" which are hardcoded
+    output.clear();
-    if ((strncmp(cmd, "info", 4)) && strncmp(cmd, "values", 6)) {
+    char error[100];
-        output.clear();
+    snprintf(error, sizeof(error), "cannot find %s in '%s'", msg, cmd);
-        char error[100];
+    output["message"] = error;
-        snprintf(error, sizeof(error), "cannot find %s for '%s'", msg, cmd);
+    return false;
-        output["message"] = error;
-    }
-
-    return false; // always return false
 }
 
 // search for recognized device_ids : Me, All, otherwise print hex value

src/system.cpp

@@ -1308,44 +1308,50 @@ bool System::get_value_info(JsonObject root, const char * command) {
         val[0] = '\0';
     }
 
-    char * dash = strchr(cmd, '/');
+    char * slash = strchr(cmd, '/');
-    if (dash) {
+    if (slash) {
-        *dash = '\0';
+        *slash = '\0';
-        dash++;
+        slash++;
     }
 
-    if (command_info("", 0, root)) {
+    // fetch all the data from the system
-        std::string s;
+    (void)command_info("", 0, root);
-        // Loop through all the key-value pairs in root to find the key, case independent
+
-        if (dash) { // search the nest first
+    // check for hardcoded "info"
-            for (JsonPair p : root) {
+    if (Helpers::toLower(cmd) == F_(info)) {
-                if (p.value().is<JsonObject>() && Helpers::toLower(p.key().c_str()) == cmd) {
+        return true;
-                    for (JsonPair p1 : p.value().as<JsonObject>()) {
+    }
-                        if (Helpers::toLower(p1.key().c_str()) == dash && !p1.value().is<JsonObject>()) {
+
-                            s = p1.value().as<std::string>();
+    std::string s;
-                            break;
+    // Loop through all the key-value pairs in root to find the key, case independent
-                        }
+    if (slash) { // search the top level first
+        for (JsonPair p : root) {
+            if (p.value().is<JsonObject>() && Helpers::toLower(p.key().c_str()) == cmd) {
+                for (JsonPair p1 : p.value().as<JsonObject>()) {
+                    if (Helpers::toLower(p1.key().c_str()) == slash && !p1.value().is<JsonObject>()) {
+                        s = p1.value().as<std::string>();
+                        break;
                     }
                 }
             }
-        } else {
+        }
-            for (JsonPair p : root) {
+    } else {
-                if (Helpers::toLower(p.key().c_str()) == cmd && !p.value().is<JsonObject>()) {
+        for (JsonPair p : root) {
-                    s = p.value().as<std::string>();
+            if (Helpers::toLower(p.key().c_str()) == cmd && !p.value().is<JsonObject>()) {
-                    break;
+                s = p.value().as<std::string>();
-                }
+                break;
             }
         }
+    }
 
-        if (!s.empty()) {
+    if (!s.empty()) {
-            root.clear();
+        root.clear();
-            if (val) {
+        if (val) {
-                root["api_data"] = s;
+            root["api_data"] = s;
-            } else {
+        } else {
-                root["value"] = s;
+            root["value"] = s;
-            }
-            return true;
         }
+        return true; // found
     }
 
     return EMSESP::return_not_found(root, "data", command); // not found
@@ -1609,7 +1615,7 @@ bool System::command_info(const char * value, const int8_t id, JsonObject output
         }
     }
 
-    return true;
+    return true; // this function always returns true!
 }
 
 #if defined(EMSESP_TEST)

src/temperaturesensor.cpp

@@ -348,8 +348,9 @@ bool TemperatureSensor::get_value_info(JsonObject output, const char * cmd, cons
         return Command::list(EMSdevice::DeviceType::TEMPERATURESENSOR, output);
     }
 
+    // return empty json if there are no sensors
     if (sensors_.empty()) {
-        return true; // no sensors, return true
+        return true;
     }
 
     uint8_t show_all = 0;
@@ -391,18 +392,17 @@ bool TemperatureSensor::get_value_info(JsonObject output, const char * cmd, cons
     for (const auto & sensor : sensors_) {
         // match custom name or sensor ID
         if (sensor_name == Helpers::toLower(sensor.name()) || sensor_name == Helpers::toLower(sensor.id())) {
-            // add values
+            // add all the data elements
             addSensorJson(output, sensor);
             // if we're filtering on an attribute, go find it
             if (attribute_s) {
                 if (output.containsKey(attribute_s)) {
-                    String data = output[attribute_s].as<String>();
+                    std::string data = output[attribute_s].as<std::string>();
                     output.clear();
-                    output["api_data"] = data;
+                    output["api_data"] = data; // always as string
                     return true;
-                } else {
-                    return EMSESP::return_not_found(output, "attribute", sensor_name); // not found
                 }
+                return EMSESP::return_not_found(output, "attribute", sensor_name); // not found
             }
             return true; // found a match, exit
         }
@@ -625,15 +625,15 @@ void TemperatureSensor::test() {
     uint8_t addr[ADDR_LEN] = {1, 2, 3, 4, 5, 6, 7, 8};
     sensors_.emplace_back(addr);
     sensors_.back().apply_customization();
-    sensors_.back().temperature_c = 123;
+    sensors_.back().temperature_c = 123; // 12.3
     sensors_.back().read          = true;
     publish_sensor(sensors_.back()); // call publish single
 
-    // Sensor ID: 0B-0C0D-0E0F-1011
+    // Sensor ID: 0B_0C0D_0E0F_1011
     uint8_t addr2[ADDR_LEN] = {11, 12, 13, 14, 15, 16, 17, 18};
     sensors_.emplace_back(addr2);
     sensors_.back().apply_customization();
-    sensors_.back().temperature_c = 456;
+    sensors_.back().temperature_c = 456; // 45.6
     sensors_.back().read          = true;
     publish_sensor(sensors_.back()); // call publish single
 }

src/temperaturesensor.h

@@ -63,7 +63,7 @@ class TemperatureSensor {
         bool apply_customization();
 
         // initial values
-        int16_t temperature_c = EMS_VALUE_INT16_NOTSET;
+        int16_t temperature_c = EMS_VALUE_INT16_NOTSET; // value is *10
         bool    read          = false;
         bool    ha_registered = false;
 

src/web/WebAPIService.cpp

@@ -142,12 +142,10 @@ void WebAPIService::parse(AsyncWebServerRequest * request, JsonObject input) {
     if (api_data) {
         request->send(200, "text/plain; charset=utf-8", api_data);
 #if defined(EMSESP_STANDALONE)
-        Serial.print(COLOR_YELLOW);
+        Serial.printf("%sweb output: %s[%s] %s(200)%s ", COLOR_WHITE, COLOR_BRIGHT_CYAN, request->url().c_str(), COLOR_BRIGHT_GREEN, COLOR_MAGENTA);
-        Serial.print("web output: ");
+        serializeJson(output, Serial);
-        if (output.size()) {
-            serializeJson(output, Serial);
-        }
         Serial.println(COLOR_RESET);
+        Serial.println();
 #endif
         api_count_++;
         delete response;
@@ -168,15 +166,11 @@ void WebAPIService::parse(AsyncWebServerRequest * request, JsonObject input) {
     api_count_++;
 
 #if defined(EMSESP_STANDALONE)
-    Serial.print(COLOR_YELLOW);
+    Serial.printf("%sweb output: %s[%s]", COLOR_WHITE, COLOR_BRIGHT_CYAN, request->url().c_str());
-    Serial.print("web output: ");
+    Serial.printf(" %s(%d)%s ", ret_codes[return_code] == 200 ? COLOR_BRIGHT_GREEN : COLOR_BRIGHT_RED, ret_codes[return_code], COLOR_YELLOW);
-    if (output.size()) {
+    serializeJson(output, Serial);
-        serializeJson(output, Serial);
-    }
-    Serial.print("  (response code ");
-    Serial.print(ret_codes[return_code]);
-    Serial.print(")");
     Serial.println(COLOR_RESET);
+    Serial.println();
 #endif
 }
 

src/web/WebCustomEntityService.cpp

@@ -237,13 +237,12 @@ void WebCustomEntityService::render_value(JsonObject output, CustomEntityItem en
         }
         break;
     case DeviceValueType::STRING:
+    default:
+        // if no type treat it as a string
         if (entity.data.length() > 0) {
             output[name] = entity.data;
         }
         break;
-    default:
-        // EMSESP::logger().warning("unknown value type");
-        break;
     }
 }
 
@@ -269,10 +268,10 @@ bool WebCustomEntityService::get_value_info(JsonObject output, const char * cmd)
         return true;
     }
 
-    // if no entries, return empty json
+    // if no custom entries, return empty json
+    // even if we're looking for a specific entity
     // https://github.com/emsesp/EMS-ESP32/issues/1297
     if (customEntityItems_->size() == 0) {
-        // TODO or should this report back the message[] ?
         return true;
     }
 
@@ -288,6 +287,7 @@ bool WebCustomEntityService::get_value_info(JsonObject output, const char * cmd)
     char command_s[COMMAND_MAX_LENGTH];
     strlcpy(command_s, Helpers::toLower(cmd).c_str(), sizeof(command_s));
     char * attribute_s = nullptr;
+
     // check specific attribute to fetch instead of the complete record
     char * breakp = strchr(command_s, '/');
     if (breakp) {
@@ -316,17 +316,16 @@ bool WebCustomEntityService::get_value_info(JsonObject output, const char * cmd)
                     output["bytes"] = (uint8_t)entity.factor;
                 }
             }
-            render_value(output, entity, true);
+            render_value(output, entity, true); // create the "value" field
 
             if (attribute_s) {
                 if (output.containsKey(attribute_s)) {
-                    String data = output[attribute_s].as<String>();
+                    std::string data = output[attribute_s].as<std::string>();
                     output.clear();
-                    output["api_data"] = data;
+                    output["api_data"] = data; // always as string
                     return true;
-                } else {
-                    return EMSESP::return_not_found(output, "attribute", command_s); // not found
                 }
+                return EMSESP::return_not_found(output, "attribute", command_s); // not found
             }
         }
 
@@ -637,8 +636,10 @@ bool WebCustomEntityService::get_value(std::shared_ptr<const Telegram> telegram)
 void WebCustomEntityService::test() {
     update([&](WebCustomEntity & webCustomEntity) {
         webCustomEntity.customEntityItems.clear();
+        auto entityItem = CustomEntityItem();
+
         // test 1
-        auto entityItem       = CustomEntityItem();
+        entityItem.id         = 1;
         entityItem.ram        = 0;
         entityItem.device_id  = 8;
         entityItem.type_id    = 24;
@@ -652,6 +653,7 @@ void WebCustomEntityService::test() {
         webCustomEntity.customEntityItems.push_back(entityItem);
 
         // test 2
+        entityItem.id         = 2;
         entityItem.ram        = 0;
         entityItem.device_id  = 24;
         entityItem.type_id    = 677;
@@ -664,7 +666,8 @@ void WebCustomEntityService::test() {
         entityItem.data       = "48";
         webCustomEntity.customEntityItems.push_back(entityItem);
 
-        // test 2
+        // test 3
+        entityItem.id         = 3;
         entityItem.ram        = 1;
         entityItem.device_id  = 0;
         entityItem.type_id    = 0;
@@ -677,6 +680,21 @@ void WebCustomEntityService::test() {
         entityItem.data       = "14";
         webCustomEntity.customEntityItems.push_back(entityItem);
 
+        // test 4
+        entityItem.id         = 4;
+        entityItem.ram        = 1;
+        entityItem.device_id  = 0;
+        entityItem.type_id    = 0;
+        entityItem.offset     = 0;
+        entityItem.factor     = 1;
+        entityItem.name       = "seltemp";
+        entityItem.uom        = 0;
+        entityItem.value_type = 8;
+        entityItem.writeable  = true;
+        entityItem.data       = "14";
+        entityItem.value      = 12;
+        webCustomEntity.customEntityItems.push_back(entityItem);
+
         return StateUpdateResult::CHANGED; // persist the changes
     });
 }

src/web/WebSchedulerService.cpp

@@ -165,7 +165,7 @@ bool WebSchedulerService::get_value_info(JsonObject output, const char * cmd) {
             return true;
         }
 
-        return EMSESP::return_not_found(output, "scheduler", cmd); // not found
+        return EMSESP::return_not_found(output, "schedule", cmd); // not found
     }
 
     char command_s[COMMAND_MAX_LENGTH];
@@ -200,16 +200,17 @@ bool WebSchedulerService::get_value_info(JsonObject output, const char * cmd) {
     }
 
     if (attribute_s && output.containsKey(attribute_s)) {
-        String data = output[attribute_s].as<String>();
+        std::string data = output[attribute_s].as<std::string>();
         output.clear();
-        output["api_data"] = data;
+        output["api_data"] = data; // always as a string
+        return true;
     }
 
     if (output.size()) {
         return true;
     }
 
-    return EMSESP::return_not_found(output, "scheduler", cmd); // not found
+    return EMSESP::return_not_found(output, "schedule", cmd); // not found
 }
 
 // publish single value
@@ -515,6 +516,31 @@ void WebSchedulerService::test() {
     // should output 'rssi is -23 dbm'
     test_value = "\"rssi is \"(system/network/rssi)\" dBm\"";
     command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "(custom/seltemp/value)";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "\"seltemp=\"(custom/seltemp/value)";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "(custom/seltemp)";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "(boiler/outdoortemp)";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "boiler/flowtempoffset";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "(boiler/flowtempoffset/value)";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    test_value = "(boiler/storagetemp1/value)";
+    command(test_cmd.c_str(), compute(test_value).c_str());
+
+    // (14 - 40) * 2.8 + 5 = -67.8
+    test_value = "(custom/seltemp - boiler/flowtempoffset) * 2.8 + 5";
+    command(test_cmd.c_str(), compute(test_value).c_str());
 }
 #endif