diff --git a/src/command.cpp b/src/command.cpp index 1db2aff02..dea6575c6 100644 --- a/src/command.cpp +++ b/src/command.cpp @@ -36,7 +36,7 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json if (!p.paths().size()) { output.clear(); - output["message"] = "error: invalid path"; // path is empty + output["message"] = "invalid path"; // path is empty return CommandRet::ERROR; } @@ -52,7 +52,7 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json } else { // error output.clear(); - output["message"] = "error: unrecognized path"; + output["message"] = "unrecognized path"; return CommandRet::ERRORED; } } @@ -63,7 +63,7 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json std::string cmd_s; int8_t id_n = -1; // default hc - // check for a device + // 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) { @@ -81,16 +81,17 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json if (device_type == EMSdevice::DeviceType::UNKNOWN) { output.clear(); char error[100]; - snprintf(error, sizeof(error), "error: unknown device %s", device_s); + snprintf(error, sizeof(error), "unknown device %s", device_s); output["message"] = error; return CommandRet::NOT_FOUND; } + // the next value on the path should be the command const char * command_p = nullptr; if (num_paths == 2) { command_p = p.paths()[1].c_str(); } else if (num_paths >= 3) { - // concatenate the path into one string + // concatenate the path into one string as it could be in the format 'hc/XXX' char command[50]; snprintf(command, sizeof(command), "%s/%s", p.paths()[1].c_str(), p.paths()[2].c_str()); command_p = command; @@ -103,12 +104,13 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json } } - // some commands may be prefixed with hc. or wwc. so extract these + // some commands may be prefixed with hc. or wwc. so extract these if they exist + // parse_command_string returns the extracted command // exit if we don't have a command command_p = parse_command_string(command_p, id_n); if (command_p == nullptr) { output.clear(); - output["message"] = "error: missing command"; + output["message"] = "missing or bad command"; return CommandRet::NOT_FOUND; } @@ -133,36 +135,48 @@ uint8_t Command::process(const char * path, const bool authenticated, const Json } // call the command based on the type - uint8_t cmd_return = CommandRet::ERROR; + uint8_t return_code = CommandRet::ERROR; if (data.is()) { - cmd_return = Command::call(device_type, command_p, data.as(), authenticated, id_n, output); + return_code = Command::call(device_type, command_p, data.as(), authenticated, id_n, output); } else if (data.is()) { char data_str[10]; - cmd_return = Command::call(device_type, command_p, Helpers::itoa(data_str, (int16_t)data.as()), authenticated, id_n, output); + return_code = Command::call(device_type, command_p, Helpers::itoa(data_str, (int16_t)data.as()), authenticated, id_n, output); } else if (data.is()) { char data_str[10]; - cmd_return = Command::call(device_type, command_p, Helpers::render_value(data_str, (float)data.as(), 2), authenticated, id_n, output); + return_code = Command::call(device_type, command_p, Helpers::render_value(data_str, (float)data.as(), 2), authenticated, id_n, output); } else if (data.isNull()) { // empty - cmd_return = Command::call(device_type, command_p, "", authenticated, id_n, output); + return_code = Command::call(device_type, command_p, "", authenticated, id_n, output); } else { // can't process - LOG_ERROR(F("Cannot parse command")); + output.clear(); + output["message"] = "cannot parse command"; return CommandRet::ERROR; } - // write debug to log - if (cmd_return == CommandRet::OK) { - LOG_DEBUG(F("Command %s was executed successfully"), command_p); - } else { - if (!output.isNull()) { - LOG_ERROR(F("Command failed with %s (%d)"), (const char *)output["message"], cmd_return); - } else { - LOG_ERROR(F("Command failed with code %d"), cmd_return); - } - } + return return_code; +} - return cmd_return; +const std::string Command::return_code_string(const uint8_t return_code) { + switch (return_code) { + case CommandRet::ERROR: + return read_flash_string(F("Error")); + break; + case CommandRet::OK: + return read_flash_string(F("OK")); + break; + case CommandRet::NOT_FOUND: + return read_flash_string(F("Not found")); + break; + case CommandRet::NOT_ALLOWED: + return read_flash_string(F("Not authorized")); + break; + case CommandRet::ERRORED: + return read_flash_string(F("Critical error")); + break; + } + char s[4]; + return Helpers::smallitoa(s, return_code); } // takes a string like "hc1/seltemp" or "seltemp" or "wwc2.seltemp" and tries to get the id and cmd @@ -176,11 +190,9 @@ const char * Command::parse_command_string(const char * command, int8_t & id) { char command_s[100]; strncpy(command_s, command, sizeof(command_s)); - char * p; - char * breakp; // look for a delimeter and split the string - p = command_s; - breakp = strchr(p, '.'); + char * p = command_s; + char * breakp = strchr(p, '.'); if (!breakp) { p = command_s; // reset and look for / breakp = strchr(p, '/'); @@ -193,17 +205,17 @@ const char * Command::parse_command_string(const char * command, int8_t & id) { } } + // extract the hc or wwc number uint8_t start_pos = breakp - p + 1; - - // extra the hc or wwc number if (!strncmp(command, "hc", 2) && start_pos == 4) { id = command[start_pos - 2] - '0'; } else if (!strncmp(command, "wwc", 3) && start_pos == 5) { id = command[start_pos - 2] - '0'; } else { #if defined(EMSESP_DEBUG) - LOG_DEBUG(F("command parse error, unknown hc/wwc in %s"), command_s); + LOG_DEBUG(F("[DEBUG] Command parse error, unknown hc/wwc in %s"), command_s); #endif + return nullptr; } return (command + start_pos); @@ -249,7 +261,7 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * // check permissions if (cf->has_flags(CommandFlag::ADMIN_ONLY) && !authenticated) { - output["message"] = "error: authentication failed"; + output["message"] = "authentication failed"; return CommandRet::NOT_ALLOWED; // command not allowed } @@ -264,7 +276,7 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * // report error if call failed if (return_code != CommandRet::OK) { output.clear(); - output["message"] = "error: function failed"; + output["message"] = "callback function failed"; } return return_code; @@ -272,7 +284,7 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char * // we didn't find the command and its not an endpoint, report error char error[100]; - snprintf(error, sizeof(error), "error: invalid command %s", cmd); + snprintf(error, sizeof(error), "invalid command '%s'", cmd); output["message"] = error; return CommandRet::NOT_FOUND; } diff --git a/src/command.h b/src/command.h index 3000992a7..b72d3a833 100644 --- a/src/command.h +++ b/src/command.h @@ -134,6 +134,8 @@ class Command { static const char * parse_command_string(const char * command, int8_t & id); + static const std::string return_code_string(const uint8_t return_code); + private: static uuid::log::Logger logger_; diff --git a/src/console.cpp b/src/console.cpp index fc193179c..54fe8eae3 100644 --- a/src/console.cpp +++ b/src/console.cpp @@ -384,37 +384,37 @@ void EMSESPShell::add_console_commands() { const char * cmd = arguments[1].c_str(); - uint8_t cmd_return = CommandRet::OK; + uint8_t return_code = CommandRet::OK; JsonObject json = doc.to(); if (arguments.size() == 2) { // no value specified, just the cmd - cmd_return = Command::call(device_type, cmd, nullptr, true, -1, json); + return_code = Command::call(device_type, cmd, nullptr, true, -1, json); } else if (arguments.size() == 3) { if (strncmp(cmd, "info", 4) == 0) { // info has a id but no value - cmd_return = Command::call(device_type, cmd, nullptr, true, atoi(arguments.back().c_str()), json); + return_code = Command::call(device_type, cmd, nullptr, true, atoi(arguments.back().c_str()), json); } else { // has a value but no id so use -1 - cmd_return = Command::call(device_type, cmd, arguments.back().c_str(), true, -1, json); + return_code = Command::call(device_type, cmd, arguments.back().c_str(), true, -1, json); } } else { // use value, which could be an id or hc - cmd_return = Command::call(device_type, cmd, arguments[2].c_str(), true, atoi(arguments[3].c_str()), json); + return_code = Command::call(device_type, cmd, arguments[2].c_str(), true, atoi(arguments[3].c_str()), json); } - if (cmd_return == CommandRet::OK && json.size()) { + if (return_code == CommandRet::OK && json.size()) { serializeJsonPretty(doc, shell); shell.println(); return; } - if (cmd_return == CommandRet::NOT_FOUND) { + if (return_code == CommandRet::NOT_FOUND) { shell.println(F("Unknown command")); shell.print(F("Available commands are: ")); Command::show(shell, device_type, false); // non-verbose mode - } else if (cmd_return != CommandRet::OK) { + } else if (return_code != CommandRet::OK) { shell.println(F("Bad syntax")); } }, diff --git a/src/emsdevice.cpp b/src/emsdevice.cpp index a092cbf57..9d4799c0c 100644 --- a/src/emsdevice.cpp +++ b/src/emsdevice.cpp @@ -905,7 +905,9 @@ bool EMSdevice::get_value_info(JsonObject & output, const char * cmd, const int8 } } - emsesp::EMSESP::logger().err(F("Can't get values for entity '%s'"), cmd); + char error[100]; + snprintf(error, sizeof(error), "cannot find values for entity '%s'", cmd); + json["message"] = error; return false; } diff --git a/src/mqtt.cpp b/src/mqtt.cpp index 22318c504..64aee6bcc 100644 --- a/src/mqtt.cpp +++ b/src/mqtt.cpp @@ -270,16 +270,22 @@ void Mqtt::on_message(const char * topic, const char * payload, size_t len) { JsonObject output = output_doc.to(); uint8_t return_code = Command::process(topic, true, input.as(), output); // mqtt is always authenticated - - // send MQTT response if enabled - if (!send_response_ || output.isNull()) { - return; - } - if (return_code != CommandRet::OK) { - Mqtt::publish(F_(response), (const char *)output["message"]); + char error[100]; + if (output.size()) { + snprintf(error, sizeof(error), "Call failed with error: %s (%s)", (const char *)output["message"], Command::return_code_string(return_code).c_str()); + } else { + snprintf(error, sizeof(error), "Call failed with error code (%s)", Command::return_code_string(return_code).c_str()); + } + LOG_ERROR(error); + if (send_response_) { + Mqtt::publish(F_(response), error); + } } else { - Mqtt::publish(F_(response), output); // output response from call + // all good, send back json output from call + if (send_response_) { + Mqtt::publish(F_(response), output); + } } } diff --git a/src/test/test.cpp b/src/test/test.cpp index c0d2468d2..dc44b9ccc 100644 --- a/src/test/test.cpp +++ b/src/test/test.cpp @@ -498,6 +498,11 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { /* AsyncWebServerRequest request2; request2.method(HTTP_GET); + + request2.url("/api/thermostat/mode/auto"); // check if defaults to info + EMSESP::webAPIService.webAPIService_get(&request2); + return; + request2.url("/api/thermostat"); // check if defaults to info EMSESP::webAPIService.webAPIService_get(&request2); request2.url("/api/thermostat/info"); @@ -524,6 +529,18 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { return; */ + /* + AsyncWebServerRequest request3; + request3.method(HTTP_POST); + DynamicJsonDocument docX(2000); + JsonVariant jsonX; + char dataX[] = "{}"; + deserializeJson(docX, dataX); + jsonX = docX.as(); + request3.url("/api/thermostat/mode/auto"); // should fail + EMSESP::webAPIService.webAPIService_post(&request3, jsonX); + */ + // test command parse int8_t id_n; const char * cmd; @@ -564,8 +581,9 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13"); // MQTT bad tests - EMSESP::mqtt_.incoming("ems-esp/thermostate/mode", "auto"); // unknown device - EMSESP::mqtt_.incoming("ems-esp/thermostat/modee", "auto"); // unknown command + EMSESP::mqtt_.incoming("ems-esp/thermostate/mode", "auto"); // unknown device + EMSESP::mqtt_.incoming("ems-esp/thermostat/modee", "auto"); // unknown command + EMSESP::mqtt_.incoming("ems-esp/thermostat/mode/auto", "auto"); // invalid // check long base Mqtt::base("home/cellar/heating"); @@ -649,6 +667,13 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) { request.url("/rest/writeValue"); EMSESP::webDataService.write_value(&request, json); + // should fail + char data8[] = "{}"; + deserializeJson(doc, data8); + json = doc.as(); + request.url("/api/thermostat/mode/auto"); + EMSESP::webAPIService.webAPIService_post(&request, json); + #endif } diff --git a/src/web/WebAPIService.cpp b/src/web/WebAPIService.cpp index 11b23f456..aa5b59406 100644 --- a/src/web/WebAPIService.cpp +++ b/src/web/WebAPIService.cpp @@ -80,16 +80,16 @@ void WebAPIService::parse(AsyncWebServerRequest * request, JsonObject & input) { JsonObject output = response->getRoot(); // call command - uint8_t command_ret = Command::process(request->url().c_str(), authenticated, input, output); + uint8_t return_code = Command::process(request->url().c_str(), authenticated, input, output); // handle response codes // the output will be populated with a message key if an error occurred int ret_code; - if (command_ret == CommandRet::NOT_ALLOWED) { + if (return_code == CommandRet::NOT_ALLOWED) { ret_code = 401; // Unauthorized - } else if (command_ret == CommandRet::NOT_FOUND) { + } else if (return_code == CommandRet::NOT_FOUND) { ret_code = 400; // Bad request - } else if (command_ret == CommandRet::OK) { + } else if (return_code == CommandRet::OK) { ret_code = 200; // OK // if there was not json output from the call, default to the message 'OK'. // this will be used for example when calling endpoints e.g. /api/thermostat/temp @@ -106,9 +106,20 @@ void WebAPIService::parse(AsyncWebServerRequest * request, JsonObject & input) { response->setContentType("application/json"); request->send(response); // send json response + // do a log entry if there is an error + if (return_code != CommandRet::OK) { + char error[100]; + if (output.size()) { + snprintf(error, sizeof(error), "Call failed with error: %s (%s)", (const char *)output["message"], Command::return_code_string(return_code).c_str()); + } else { + snprintf(error, sizeof(error), "Call failed with error code (%s)", Command::return_code_string(return_code).c_str()); + } + emsesp::EMSESP::logger().err(error); + } + #if defined(EMSESP_STANDALONE) Serial.print(COLOR_YELLOW); - Serial.print("return code: "); + Serial.print("web response code: "); Serial.println(ret_code); if (output.size()) { serializeJsonPretty(output, Serial); diff --git a/src/web/WebDataService.cpp b/src/web/WebDataService.cpp index 6d5b0a569..f906c0e33 100644 --- a/src/web/WebDataService.cpp +++ b/src/web/WebDataService.cpp @@ -146,28 +146,28 @@ void WebDataService::write_value(AsyncWebServerRequest * request, JsonVariant & // the data could be in any format, but we need string // authenticated is always true JsonVariant data = dv["v"]; // the value in any format - uint8_t command_ret = CommandRet::OK; + uint8_t return_code = CommandRet::OK; uint8_t device_type = emsdevice->device_type(); if (data.is()) { - command_ret = Command::call(device_type, cmd, data.as(), true, id, output); + return_code = Command::call(device_type, cmd, data.as(), true, id, output); } else if (data.is()) { char s[10]; - command_ret = Command::call(device_type, cmd, Helpers::render_value(s, data.as(), 0), true, id, output); + return_code = Command::call(device_type, cmd, Helpers::render_value(s, data.as(), 0), true, id, output); } else if (data.is()) { char s[10]; - command_ret = Command::call(device_type, cmd, Helpers::render_value(s, (float)data.as(), 1), true, id, output); + return_code = Command::call(device_type, cmd, Helpers::render_value(s, (float)data.as(), 1), true, id, output); } else if (data.is()) { - command_ret = Command::call(device_type, cmd, data.as() ? "true" : "false", true, id, output); + return_code = Command::call(device_type, cmd, data.as() ? "true" : "false", true, id, output); } // write debug - if (command_ret != CommandRet::OK) { - EMSESP::logger().err(F("Write command failed %s (%d)"), (const char *)output["message"], command_ret); + if (return_code != CommandRet::OK) { + EMSESP::logger().err(F("Write command failed %s (%s)"), (const char *)output["message"], Command::return_code_string(return_code).c_str()); } else { EMSESP::logger().debug(F("Write command successful")); } - response->setCode((command_ret == CommandRet::OK) ? 200 : 204); + response->setCode((return_code == CommandRet::OK) ? 200 : 204); response->setLength(); request->send(response); return;