From fa54cb6a48a13729135121c2c80f568816557262 Mon Sep 17 00:00:00 2001 From: proddy Date: Wed, 24 Jul 2024 13:22:34 +0200 Subject: [PATCH] more minor changes to clean up messaging --- src/analogsensor.cpp | 2 +- src/command.cpp | 27 +++++++++++----------- src/console.cpp | 13 +++++------ src/emsdevice.cpp | 30 +++++++++++++----------- src/emsesp.cpp | 10 +++----- src/locale_translations.h | 2 +- src/system.cpp | 20 ++++++++-------- src/temperaturesensor.cpp | 2 +- src/test/test.cpp | 37 ++++++++++++++++++++++-------- src/web/WebCustomEntityService.cpp | 3 ++- src/web/WebSchedulerService.cpp | 2 +- 11 files changed, 85 insertions(+), 63 deletions(-) diff --git a/src/analogsensor.cpp b/src/analogsensor.cpp index c07135672..f36231f73 100644 --- a/src/analogsensor.cpp +++ b/src/analogsensor.cpp @@ -711,7 +711,7 @@ bool AnalogSensor::get_value_info(JsonObject output, const char * cmd, const int } } - return EMSESP::return_not_found(output, cmd, F_(analogsensor)); // not found + return false; // not found } void AnalogSensor::addSensorJson(JsonObject output, const Sensor & sensor) { diff --git a/src/command.cpp b/src/command.cpp index fda02bb98..a0825de1d 100644 --- a/src/command.cpp +++ b/src/command.cpp @@ -63,7 +63,6 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec int8_t id_n = -1; // default hc // check for a device as first item in the path - // if its not a known device (thermostat, boiler etc) look for any special MQTT subscriptions const char * device_s = nullptr; if (!num_paths) { // we must look for the device in the JSON body @@ -72,7 +71,7 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec } } else { // extract it from the path - device_s = p.paths().front().c_str(); // get the device (boiler, thermostat, system etc) + device_s = p.paths().front().c_str(); // get the device type name (boiler, thermostat, system etc) } // validate the device, make sure it exists @@ -347,16 +346,13 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * if (output["message"]) { LOG_WARNING("Command failed: %s", output["message"].as()); } else { - std::string err = "no command " + std::string(cmd) + " in " + dname; + std::string err = "no " + std::string(cmd) + " in " + dname; output["message"] = err; LOG_WARNING("Command failed: %s", err.c_str()); } return CommandRet::ERROR; } - // TODO here - output.clear(); // we have a command function, clear messages from device_value_info - // before calling the command, check permissions and abort if not authorized if (cf->has_flags(CommandFlag::ADMIN_ONLY) && !is_admin) { LOG_WARNING("Command failed: authentication failed"); @@ -404,12 +400,15 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * LOG_WARNING(error); } else { if (single_command) { - LOG_DEBUG(("%sCalled command %s"), ro.c_str(), info_s); + // TODO not sure if these should go to INFO as there may be a lot of calls, e.g. ioBroker + // TODO for now using debug (regarles if compiled with EMSESP_DEBUG) + // TODO mvdp ? + logger_.debug(("%sCalled command %s"), ro.c_str(), info_s); } else { if (id > 0) { - LOG_INFO(("%sCalled command %s with value %s and id %d on device 0x%02X"), ro.c_str(), info_s, value, id, device_id); + logger_.debug(("%sCalled command %s with value %s and id %d on device 0x%02X"), ro.c_str(), info_s, value, id, device_id); } else { - LOG_INFO(("%sCalled command %s with value %s"), ro.c_str(), info_s, value); + logger_.debug(("%sCalled command %s with value %s"), ro.c_str(), info_s, value); } } } @@ -549,7 +548,7 @@ void Command::show(uuid::console::Shell & shell, uint8_t device_type, bool verbo } } - // non EMS devices always have an info and commands command + // non EMS devices always have an info, commands and values bool show_info = (device_type == EMSdevice::DeviceType::TEMPERATURESENSOR || device_type == EMSdevice::DeviceType::ANALOGSENSOR || device_type == EMSdevice::DeviceType::SCHEDULER || device_type == EMSdevice::DeviceType::CUSTOM); @@ -576,9 +575,11 @@ void Command::show(uuid::console::Shell & shell, uint8_t device_type, bool verbo // we hard code 'info' and 'commands' commands so print them first if (show_info) { - shell.printf(" info:\t\t\t\t%slists all values %s*", COLOR_BRIGHT_CYAN, COLOR_BRIGHT_GREEN); + shell.printf(" info:\t\t\t\t%slist all values %s*", COLOR_BRIGHT_CYAN, COLOR_BRIGHT_GREEN); shell.println(COLOR_RESET); - shell.printf(" commands:\t\t\t%slists all commands %s*", COLOR_BRIGHT_CYAN, COLOR_BRIGHT_GREEN); + shell.printf(" commands:\t\t\t%slist all commands %s*", COLOR_BRIGHT_CYAN, COLOR_BRIGHT_GREEN); + shell.println(COLOR_RESET); + shell.printf(" values:\t\t\t%slist all values %s*", COLOR_BRIGHT_CYAN, COLOR_BRIGHT_GREEN); shell.println(COLOR_RESET); } @@ -618,7 +619,6 @@ bool Command::device_has_commands(const uint8_t device_type) { return true; // we always have System } - // if there are no entries to scheduler/custom/temperaturesensor/analogsensor, don't error but return a message if (device_type == EMSdevice::DeviceType::SCHEDULER) { return true; } @@ -637,6 +637,7 @@ bool Command::device_has_commands(const uint8_t device_type) { for (const auto & emsdevice : EMSESP::emsdevices) { if (emsdevice && (emsdevice->device_type() == device_type)) { + // TODO will this work for info, values etc?? // device found, now see if it has any commands for (const auto & cf : cmdfunctions_) { if (cf.device_type_ == device_type) { diff --git a/src/console.cpp b/src/console.cpp index 6d3eb7387..53eb61691 100644 --- a/src/console.cpp +++ b/src/console.cpp @@ -575,18 +575,17 @@ static void setup_commands(std::shared_ptr & commands) { return; } else { // show message if no data returned (e.g. for analogsensor, temperaturesensor, custom) - shell.println("No data returned."); + shell.println("Command executed. Check log for messages."); return; } - } - - - if (return_code == CommandRet::NOT_FOUND) { + } else if (return_code == CommandRet::NOT_FOUND) { shell.println("Unknown command"); shell.print("Available commands are: "); Command::show(shell, device_type, false); // non-verbose mode - } else if (return_code != CommandRet::OK) { - shell.printfln("Bad syntax (error code %d)", return_code); + } else if ((return_code == CommandRet::ERROR) || (return_code == CommandRet::FAIL)) { + shell.printfln("Bad syntax. Check arguments."); + } else { + shell.printfln("Command failed with error code %d", return_code); } }, [](Shell & shell, const std::vector & current_arguments, const std::string & next_argument) -> std::vector { diff --git a/src/emsdevice.cpp b/src/emsdevice.cpp index b3d65fdd5..ac8902250 100644 --- a/src/emsdevice.cpp +++ b/src/emsdevice.cpp @@ -212,9 +212,6 @@ uint8_t EMSdevice::device_name_2_device_type(const char * topic) { if (!strcmp(lowtopic, F_(thermostat))) { return DeviceType::THERMOSTAT; } - if (!strcmp(lowtopic, F_(system))) { - return DeviceType::SYSTEM; - } if (!strcmp(lowtopic, F_(scheduler))) { return DeviceType::SCHEDULER; } @@ -227,12 +224,6 @@ uint8_t EMSdevice::device_name_2_device_type(const char * topic) { if (!strcmp(lowtopic, F_(mixer))) { return DeviceType::MIXER; } - if (!strcmp(lowtopic, F_(temperaturesensor))) { - return DeviceType::TEMPERATURESENSOR; - } - if (!strcmp(lowtopic, F_(analogsensor))) { - return DeviceType::ANALOGSENSOR; - } if (!strcmp(lowtopic, F_(switch))) { return DeviceType::SWITCH; } @@ -248,9 +239,6 @@ uint8_t EMSdevice::device_name_2_device_type(const char * topic) { if (!strcmp(lowtopic, F_(heatsource))) { return DeviceType::HEATSOURCE; } - if (!strcmp(lowtopic, F_(custom))) { - return DeviceType::CUSTOM; - } if (!strcmp(lowtopic, F_(ventilation))) { return DeviceType::VENTILATION; } @@ -261,6 +249,23 @@ uint8_t EMSdevice::device_name_2_device_type(const char * topic) { return DeviceType::POOL; } + // non EMS + if (!strcmp(lowtopic, F_(custom))) { + return DeviceType::CUSTOM; + } + if (!strcmp(lowtopic, F_(temperaturesensor))) { + return DeviceType::TEMPERATURESENSOR; + } + if (!strcmp(lowtopic, F_(analogsensor))) { + return DeviceType::ANALOGSENSOR; + } + if (!strcmp(lowtopic, F_(scheduler))) { + return DeviceType::SCHEDULER; + } + if (!strcmp(lowtopic, F_(system))) { + return DeviceType::SYSTEM; + } + return DeviceType::UNKNOWN; } @@ -1606,7 +1611,6 @@ bool EMSdevice::get_value_info(JsonObject output, const char * cmd, const int8_t } return EMSESP::return_not_found(output, attribute_s, command_s); // not found } - return true; } } diff --git a/src/emsesp.cpp b/src/emsesp.cpp index 31ac33dca..c49316807 100644 --- a/src/emsesp.cpp +++ b/src/emsesp.cpp @@ -794,7 +794,7 @@ bool EMSESP::get_device_value_info(JsonObject root, const char * cmd, const int8 bool EMSESP::return_not_found(JsonObject output, const char * msg, const char * cmd) { output.clear(); char error[100]; - snprintf(error, sizeof(error), "%s not found in %s", msg, cmd); + snprintf(error, sizeof(error), "no %s in %s", msg, cmd); output["message"] = error; return false; } @@ -1363,13 +1363,9 @@ bool EMSESP::add_device(const uint8_t device_id, const uint8_t product_id, const device_type, F_(values), [device_type](const char * value, const int8_t id, JsonObject output) { - return EMSdevice::export_values(device_type, - output, - id, - EMSdevice::OUTPUT_TARGET::API_SHORTNAMES); // HIDDEN command showing short names, used in e.g. /api/boiler + return EMSdevice::export_values(device_type, output, id, EMSdevice::OUTPUT_TARGET::API_SHORTNAMES); }, - nullptr, - CommandFlag::HIDDEN); // this command is hidden + FL_(values_cmd)); Command::add( device_type, F_(commands), diff --git a/src/locale_translations.h b/src/locale_translations.h index 360d67ea4..baf5c6100 100644 --- a/src/locale_translations.h +++ b/src/locale_translations.h @@ -59,7 +59,7 @@ MAKE_WORD_TRANSLATION(water_device, "Water Module", "Wassermodul", "", "", "Modu MAKE_WORD_TRANSLATION(pool_device, "Pool Module", "Poolmodul", "", "", "Moduł basenu", "", "", "", "", "") // TODO translate // commands -MAKE_WORD_TRANSLATION(info_cmd, "list all values", "Liste aller Werte", "lijst van alle waardes", "", "wyświetl wszystkie wartości", "Viser alle verdier", "", "Tüm değerleri listele", "elenca tutti i valori", "zobraziť všetky hodnoty") // TODO translate +MAKE_WORD_TRANSLATION(info_cmd, "list all values (verbose)", "Liste aller Werte", "lijst van alle waardes", "", "wyświetl wszystkie wartości", "Viser alle verdier", "", "Tüm değerleri listele", "elenca tutti i valori", "zobraziť všetky hodnoty") // TODO translate MAKE_WORD_TRANSLATION(commands_cmd, "list all commands", "Liste aller Kommandos", "lijst van alle commando's", "", "wyświetl wszystkie komendy", "Viser alle kommandoer", "", "Tüm komutları listele", "elencaa tutti i comandi", "zobraziť všetky príkazy") // TODO translate MAKE_WORD_TRANSLATION(entities_cmd, "list all entities", "Liste aller Entitäten", "lijst van alle entiteiten", "", "wyświetl wszsytkie encje", "Viser alle enheter", "", "Tüm varlıkları listele", "elenca tutte le entità", "zobraziť všetky entity") // TODO translate MAKE_WORD_TRANSLATION(send_cmd, "send a telegram", "Sende EMS-Telegramm", "stuur een telegram", "", "wyślij telegram", "send et telegram", "", "Bir telegram gönder", "invia un telegramma", "poslať telegram") // TODO translate diff --git a/src/system.cpp b/src/system.cpp index 9fd089d53..98f5e8555 100644 --- a/src/system.cpp +++ b/src/system.cpp @@ -1297,8 +1297,18 @@ bool System::get_value_info(JsonObject root, const char * command) { return false; } + // cmd is lower case of the command char cmd[COMMAND_MAX_LENGTH]; strlcpy(cmd, Helpers::toLower(command).c_str(), sizeof(cmd)); + + // fetch all the data from the system + (void)command_info("", 0, root); + + // check for hardcoded "info" + if (!strcmp(cmd, F_(info))) { + return true; + } + char * val = strstr(cmd, "/value"); if (val) { val[0] = '\0'; @@ -1310,14 +1320,6 @@ bool System::get_value_info(JsonObject root, const char * command) { slash++; } - // fetch all the data from the system - (void)command_info("", 0, root); - - // check for hardcoded "info" - if (Helpers::toLower(cmd) == F_(info)) { - return true; - } - std::string s; // Loop through all the key-value pairs in root to find the key, case independent if (slash) { // search the top level first @@ -1340,7 +1342,7 @@ bool System::get_value_info(JsonObject root, const char * command) { } } - root.clear(); // empty json output, as it may have the default output from an info command earlier + root.clear(); // clear json, we only one a single value if (!s.empty()) { if (val) { diff --git a/src/temperaturesensor.cpp b/src/temperaturesensor.cpp index d94ce1255..f27111c51 100644 --- a/src/temperaturesensor.cpp +++ b/src/temperaturesensor.cpp @@ -408,7 +408,7 @@ bool TemperatureSensor::get_value_info(JsonObject output, const char * cmd, cons } } - return EMSESP::return_not_found(output, cmd, F_(temperaturesensor)); // not found + return false; // not found } void TemperatureSensor::addSensorJson(JsonObject output, const Sensor & sensor) { diff --git a/src/test/test.cpp b/src/test/test.cpp index 187843d14..2c5e4ed73 100644 --- a/src/test/test.cpp +++ b/src/test/test.cpp @@ -953,8 +953,8 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd, const bool single; - single = true; - // single = false; + // single = true; + single = false; AsyncWebServerRequest request; JsonDocument doc; @@ -963,21 +963,39 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd, const // load devices test("boiler"); - // test("thermostat"); - test("2thermostats"); + test("thermostat"); + // test("2thermostats"); if (single) { // run dedicated tests only - // EMSESP::webCustomEntityService.test(); // custom entities - // EMSESP::webCustomizationService.test(); // set customizations - this will overwrite any settings in the FS - // EMSESP::temperaturesensor_.test(); // add temperature sensors - // EMSESP::webSchedulerService.test(); // run scheduler tests, and conditions + EMSESP::webCustomEntityService.test(); // custom entities + EMSESP::webCustomizationService.test(); // set customizations - this will overwrite any settings in the FS + EMSESP::temperaturesensor_.test(); // add temperature sensors + EMSESP::webSchedulerService.test(); // run scheduler tests, and conditions shell.invoke_command("call system fetch"); - request.method(HTTP_GET); request.url("/api/system/fetch"); EMSESP::webAPIService.webAPIService(&request); + // request.url("/api/system"); + // EMSESP::webAPIService.webAPIService(&request); + // request.url("/api/system/system/version"); + // EMSESP::webAPIService.webAPIService(&request); + // request.url("/api/system/bad"); + // EMSESP::webAPIService.webAPIService(&request); + + // request.url("/api/boiler"); + // EMSESP::webAPIService.webAPIService(&request); + // request.url("/api/boiler/bad"); + // EMSESP::webAPIService.webAPIService(&request); + + // request.url("/api/custom"); + // EMSESP::webAPIService.webAPIService(&request); + // request.url("/api/custom/seltemp"); + // EMSESP::webAPIService.webAPIService(&request); + // request.url("/api/custom/bad"); + // EMSESP::webAPIService.webAPIService(&request); + } else { EMSESP::webCustomEntityService.test(); // custom entities EMSESP::webCustomizationService.test(); // set customizations - this will overwrite any settings in the FS @@ -1062,6 +1080,7 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd, const request.method(HTTP_POST); // these next 3 should return empty JSON in their response + // but there will be a log message char data[] = "{\"cmd\":\"send\",\"data\":\"0B 90 FF 13 01 01 B9 01\"}"; deserializeJson(doc, data); json = doc.as(); diff --git a/src/web/WebCustomEntityService.cpp b/src/web/WebCustomEntityService.cpp index f174d0885..9eac1425b 100644 --- a/src/web/WebCustomEntityService.cpp +++ b/src/web/WebCustomEntityService.cpp @@ -278,6 +278,7 @@ bool WebCustomEntityService::get_value_info(JsonObject output, const char * cmd) } // if it's info or values... + // TODO make a function if (strlen(cmd) == 0 || Helpers::toLower(cmd) == F_(values) || Helpers::toLower(cmd) == F_(info)) { // list all names for (const CustomEntityItem & entity : *customEntityItems_) { @@ -336,7 +337,7 @@ bool WebCustomEntityService::get_value_info(JsonObject output, const char * cmd) } } - return EMSESP::return_not_found(output, cmd, F_(custom)); // not found + return false; // not found } // publish single value diff --git a/src/web/WebSchedulerService.cpp b/src/web/WebSchedulerService.cpp index 52b44c5fa..ad3fc6cb7 100644 --- a/src/web/WebSchedulerService.cpp +++ b/src/web/WebSchedulerService.cpp @@ -215,7 +215,7 @@ bool WebSchedulerService::get_value_info(JsonObject output, const char * cmd) { return true; } - return EMSESP::return_not_found(output, cmd, F_(scheduler)); // not found + return false; // not found } // publish single value