From 067055095f8700bf8134af6a703cd2286a32f682 Mon Sep 17 00:00:00 2001 From: Andrey G Date: Sun, 27 Feb 2022 00:58:10 +0300 Subject: [PATCH] cli registry: rework this hell, add FFF and FFFFF (#3964) * cli registry: rework this hell, add FFF and FFFFF * kinetis: no nodefaultlib * revert back token helpers * cli_registery: argument parser: do not eat qoutes * Unit test for FFF * cli_registry: remove debug --- firmware/config/boards/kinetis/config.mk | 2 +- firmware/controllers/algo/rusefi_types.h | 2 + firmware/util/cli_registry.cpp | 553 +++++++++++------------ firmware/util/cli_registry.h | 10 +- unit_tests/tests/test_util.cpp | 19 + 5 files changed, 301 insertions(+), 285 deletions(-) diff --git a/firmware/config/boards/kinetis/config.mk b/firmware/config/boards/kinetis/config.mk index 77c3c5e61e..c63f5fc139 100644 --- a/firmware/config/boards/kinetis/config.mk +++ b/firmware/config/boards/kinetis/config.mk @@ -8,4 +8,4 @@ KINETIS_CONTRIB = $(PROJECT_DIR)/config/boards/$(PROJECT_BOARD)/OS GENERATED_ENUMS_DIR = $(BOARD_DIR)/config/controllers/algo -EXTRA_PARAMS += -DFIRMWARE_ID=\"kinetis\" -nodefaultlibs -L$(PROJECT_DIR)/config/boards/$(PROJECT_BOARD)/libc -lgcc -ltinyc +EXTRA_PARAMS += -DFIRMWARE_ID=\"kinetis\" -L$(PROJECT_DIR)/config/boards/$(PROJECT_BOARD)/libc -lgcc -ltinyc diff --git a/firmware/controllers/algo/rusefi_types.h b/firmware/controllers/algo/rusefi_types.h index 23afea7af5..38e75054b4 100644 --- a/firmware/controllers/algo/rusefi_types.h +++ b/firmware/controllers/algo/rusefi_types.h @@ -120,6 +120,8 @@ typedef void (*VoidInt)(int); typedef void (*VoidIntVoidPtr)(int, void*); typedef void (*VoidFloat)(float); typedef void (*VoidFloatFloat)(float, float); +typedef void (*VoidFloatFloatFloat)(float, float, float); +typedef void (*VoidFloatFloatFloatFloatFloat)(float, float, float, float, float); typedef void (*VoidFloatFloatVoidPtr)(float, float, void*); typedef void (*VoidIntInt)(int, int); typedef void (*VoidIntIntVoidPtr)(int, int, void*); diff --git a/firmware/util/cli_registry.cpp b/firmware/util/cli_registry.cpp index bbedab7630..bb9f25cb14 100644 --- a/firmware/util/cli_registry.cpp +++ b/firmware/util/cli_registry.cpp @@ -21,6 +21,9 @@ #include "eficonsole.h" #endif /* ! EFI_UNIT_TEST */ +/* for isspace() */ +#include + // todo: support \t as well #define SPACE_CHAR ' ' @@ -38,7 +41,7 @@ static void doAddAction(const char *token, action_type_e type, Void callback, vo #if !defined(EFI_DISABLE_CONSOLE_ACTIONS) for (uint32_t i = 0; i < efiStrlen(token);i++) { char ch = token[i]; - if (ch != mytolower(ch)) { + if (isupper(ch)) { firmwareError(CUSTOM_ERR_COMMAND_LOWER_CASE_EXPECTED, "lowerCase expected [%s]", token); } } @@ -130,6 +133,14 @@ void addConsoleActionFF(const char *token, VoidFloatFloat callback) { doAddAction(token, FLOAT_FLOAT_PARAMETER, (Void) callback, NULL); } +void addConsoleActionFFF(const char *token, VoidFloatFloatFloat callback) { + doAddAction(token, FLOAT_FLOAT_FLOAT_PARAMETER, (Void) callback, NULL); +} + +void addConsoleActionFFFFF(const char *token, VoidFloatFloatFloatFloatFloat callback) { + doAddAction(token, FLOAT_FLOAT_FLOAT_FLOAT_FLOAT_PARAMETER, (Void) callback, NULL); +} + void addConsoleActionFFP(const char *token, VoidFloatFloatVoidPtr callback, void *param) { doAddAction(token, FLOAT_FLOAT_PARAMETER_P, (Void) callback, param); } @@ -153,8 +164,10 @@ static int getParameterCount(action_type_e parameterType) { case INT_FLOAT_PARAMETER: return 2; case STRING3_PARAMETER: + case FLOAT_FLOAT_FLOAT_PARAMETER: return 3; case STRING5_PARAMETER: + case FLOAT_FLOAT_FLOAT_FLOAT_FLOAT_PARAMETER: return 5; default: return -1; @@ -165,7 +178,6 @@ static int getParameterCount(action_type_e parameterType) { * @brief This function prints out a list of all available commands */ void helpCommand(void) { - #if EFI_PROD_CODE || EFI_SIMULATOR efiPrintf("%d actions available", consoleActionCount); for (int i = 0; i < consoleActionCount; i++) { @@ -183,17 +195,6 @@ static void echo(int value) { efiPrintf("got value: %d", value); } -char *unquote(char *line) { - if (line[0] == '"') { - int len = strlen(line); - if (line[len - 1] == '"') { - line[len - 1] = 0; - return line + 1; - } - } - return line; -} - int findEndOfToken(const char *line) { if (line[0] == '"') { /** @@ -214,243 +215,6 @@ int findEndOfToken(const char *line) { return indexOf(line, SPACE_CHAR); } -#define REPLACE_SPACES_WITH_ZERO { \ - while (parameter[spaceIndex + 1] == SPACE_CHAR) { \ - parameter[spaceIndex++] = 0; \ - } \ - parameter[spaceIndex] = 0; \ -} - -void handleActionWithParameter(TokenCallback *current, char *parameter) { - while (parameter[0] == SPACE_CHAR) { - parameter[0] = 0; - parameter++; - } - - switch (current->parameterType) { - case STRING_PARAMETER: - { - VoidCharPtr callbackS = (VoidCharPtr) current->callback; - (*callbackS)(parameter); - return; - } - case STRING_PARAMETER_P: - { - VoidCharPtrVoidPtr callbackS = (VoidCharPtrVoidPtr) current->callback; - (*callbackS)(parameter, current->param); - return; - } - default: - // todo: handle all cases explicitly - break; - } - - - - // todo: refactor this hell! - if (current->parameterType == STRING2_PARAMETER || current->parameterType == STRING2_PARAMETER_P) { - int spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) { - return; - } - REPLACE_SPACES_WITH_ZERO; - char * param0 = parameter; - - parameter += spaceIndex + 1; - char * param1 = parameter; - - if (current->parameterType == STRING2_PARAMETER) { - VoidCharPtrCharPtr callbackS = (VoidCharPtrCharPtr) current->callback; - (*callbackS)(param0, param1); - } else { - VoidCharPtrCharPtrVoidPtr callbackS = (VoidCharPtrCharPtrVoidPtr) current->callback; - (*callbackS)(param0, param1, current->param); - } - return; - } - - if (current->parameterType == STRING3_PARAMETER) { - int spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) { - return; - } - REPLACE_SPACES_WITH_ZERO; - char * param0 = parameter; - - parameter += spaceIndex + 1; - spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - char * param1 = parameter; - parameter += spaceIndex + 1; - char * param2 = parameter; - - VoidCharPtrCharPtrCharPtr callbackS = (VoidCharPtrCharPtrCharPtr) current->callback; - (*callbackS)(param0, param1, param2); - return; - - } - - // todo: refactor this hell! - if (current->parameterType == STRING5_PARAMETER) { - int spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) { - return; - } - REPLACE_SPACES_WITH_ZERO; - char * param0 = parameter; - - parameter += spaceIndex + 1; - spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - char * param1 = parameter; - - parameter += spaceIndex + 1; - spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - char * param2 = parameter; - - parameter += spaceIndex + 1; - spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - char * param3 = parameter; - - parameter += spaceIndex + 1; - char * param4 = parameter; - - VoidCharPtrCharPtrCharPtrCharPtrCharPtr callbackS = (VoidCharPtrCharPtrCharPtrCharPtrCharPtr) current->callback; - (*callbackS)(param0, param1, param2, param3, param4); - return; - - } - - if (current->parameterType == TWO_INTS_PARAMETER) { - int spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - int value1 = atoi(parameter); - if (absI(value1) == ERROR_CODE) { -#if EFI_PROD_CODE || EFI_SIMULATOR - efiPrintf("not an integer [%s]", parameter); -#endif - return; - } - parameter += spaceIndex + 1; - int value2 = atoi(parameter); - if (absI(value2) == ERROR_CODE) { -#if EFI_PROD_CODE || EFI_SIMULATOR - efiPrintf("not an integer [%s]", parameter); -#endif - return; - } - VoidIntInt callbackS = (VoidIntInt) current->callback; - (*callbackS)(value1, value2); - return; - } - - if (current->parameterType == FLOAT_PARAMETER_NAN_ALLOWED) { - float value = atoff(parameter); - VoidFloat callbackF = (VoidFloat) current->callback; - - // invoke callback function by reference - (*callbackF)(value); - return; - } - - if (current->parameterType == FLOAT_PARAMETER) { - float value = atoff(parameter); - if (cisnan(value)) { - efiPrintf("invalid float [%s]", parameter); - return; - } - VoidFloat callbackF = (VoidFloat) current->callback; - - // invoke callback function by reference - (*callbackF)(value); - return; - } - - if (current->parameterType == FLOAT_FLOAT_PARAMETER || current->parameterType == FLOAT_FLOAT_PARAMETER_P) { - int spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - float value1 = atoff(parameter); - if (cisnan(value1)) { - efiPrintf("invalid float [%s]", parameter); - return; - } - - parameter += spaceIndex + 1; - float value2 = atoff(parameter); - - if (cisnan(value2)) { - efiPrintf("invalid float [%s]", parameter); - return; - } - if (current->parameterType == FLOAT_FLOAT_PARAMETER) { - VoidFloatFloat callbackS = (VoidFloatFloat) current->callback; - (*callbackS)(value1, value2); - } else { - VoidFloatFloatVoidPtr callbackS = (VoidFloatFloatVoidPtr) current->callback; - (*callbackS)(value1, value2, current->param); - } - return; - } - - if (current->parameterType == INT_FLOAT_PARAMETER) { - int spaceIndex = findEndOfToken(parameter); - if (spaceIndex == -1) - return; - REPLACE_SPACES_WITH_ZERO; - int value1 = atoi(parameter); - if (absI(value1) == ERROR_CODE) { -#if EFI_PROD_CODE || EFI_SIMULATOR - efiPrintf("not an integer [%s]", parameter); -#endif - return; - } - parameter += spaceIndex + 1; - float value2 = atoff(parameter); - - if (cisnan(value2)) { - efiPrintf("invalid float [%s]", parameter); - return; - } - - VoidIntFloat callback = (VoidIntFloat) current->callback; - callback(value1, value2); - - return; - } - - int value = atoi(parameter); - if (absI(value) == ERROR_CODE) { - efiPrintf("invalid integer [%s]", parameter); - return; - } - - if (current->parameterType == ONE_PARAMETER_P) { - VoidIntVoidPtr callback1 = (VoidIntVoidPtr) current->callback; - // invoke callback function by reference - (*callback1)(value, current->param); - - } else { - VoidInt callback1 = (VoidInt) current->callback; - // invoke callback function by reference - (*callback1)(value); - } - -} - /** * @return Number of space-separated tokens in the string */ @@ -466,6 +230,236 @@ int tokenLength(const char *msgp) { return result; } +char *unquote(char *line) { + if (line[0] == '"') { + int len = strlen(line); + if (line[len - 1] == '"') { + line[len - 1] = 0; + return line + 1; + } + } + return line; +} + +static int setargs(char *args, char **argv, int max_args) +{ + int count = 0; + + while (isspace(*args)) { + *args = '\0'; + args++; + } + while ((*args) && (count < max_args)) { + if (argv) { + argv[count] = args; + } + while ((*args) && (!isspace(*args))) { + if (*args == '"') { + args++; + /* find closing quote */ + while ((*args) && (*args != '"')) { + args++; + } + /* failed? */ + if (*args == '\0') { + return -1; + } + } + args++; + } + if (*args) { + *args = '\0'; + args++; + } + while (isspace(*args)) { + *args = '\0'; + args++; + } + count++; + } + return count; +} + +int handleActionWithParameter(TokenCallback *current, char *argv[], int argc) { + (void)argc; + + switch (current->parameterType) { + case NO_PARAMETER: + { + (*current->callback)(); + return 0; + } + case NO_PARAMETER_P: + { + VoidPtr callbackS = (VoidPtr) current->callback; + (*callbackS)(current->param); + return 0; + } + case STRING_PARAMETER: + { + VoidCharPtr callbackS = (VoidCharPtr) current->callback; + (*callbackS)(argv[0]); + return 0; + } + case STRING_PARAMETER_P: + { + VoidCharPtrVoidPtr callbackS = (VoidCharPtrVoidPtr) current->callback; + (*callbackS)(argv[0], current->param); + return 0; + } + case STRING2_PARAMETER: + { + VoidCharPtrCharPtr callbackS = (VoidCharPtrCharPtr) current->callback; + (*callbackS)(argv[0], argv[1]); + return 0; + } + case STRING2_PARAMETER_P: + { + VoidCharPtrCharPtrVoidPtr callbackS = (VoidCharPtrCharPtrVoidPtr) current->callback; + (*callbackS)(argv[0], argv[1], current->param); + return 0; + } + case STRING3_PARAMETER: + { + VoidCharPtrCharPtrCharPtr callbackS = (VoidCharPtrCharPtrCharPtr) current->callback; + (*callbackS)(argv[0], argv[1], argv[2]); + return 0; + } + case STRING5_PARAMETER: + { + VoidCharPtrCharPtrCharPtrCharPtrCharPtr callbackS = (VoidCharPtrCharPtrCharPtrCharPtrCharPtr) current->callback; + (*callbackS)(argv[0], argv[1], argv[2], argv[3], argv[4]); + return 0; + } + case TWO_INTS_PARAMETER: + { + int value[2]; + for (int i = 0; i < 2; i++) { + value[i] = atoi(argv[i]); + if (absI(value[i]) == ERROR_CODE) { + #if EFI_PROD_CODE || EFI_SIMULATOR + efiPrintf("not an integer [%s]", argv[0]); + #endif + return -1; + } + } + VoidIntInt callbackS = (VoidIntInt) current->callback; + (*callbackS)(value[0], value[1]); + return 0; + } + case FLOAT_PARAMETER_NAN_ALLOWED: + { + float value = atoff(argv[0]); + VoidFloat callbackF = (VoidFloat) current->callback; + (*callbackF)(value); + return 0; + } + + case FLOAT_PARAMETER: + { + float value = atoff(argv[0]); + if (cisnan(value)) { + efiPrintf("invalid float [%s]", argv[0]); + return -1; + } + VoidFloat callbackF = (VoidFloat) current->callback; + (*callbackF)(value); + return 0; + } + + case FLOAT_FLOAT_PARAMETER: + case FLOAT_FLOAT_PARAMETER_P: + { + float value[2]; + for (int i = 0; i < 2; i++) { + value[i] = atoff(argv[i]); + if (cisnan(value[i])) { + efiPrintf("invalid float [%s]", argv[i]); + return -1; + } + } + if (current->parameterType == FLOAT_FLOAT_PARAMETER) { + VoidFloatFloat callbackS = (VoidFloatFloat) current->callback; + (*callbackS)(value[0], value[1]); + } else { + VoidFloatFloatVoidPtr callbackS = (VoidFloatFloatVoidPtr) current->callback; + (*callbackS)(value[0], value[1], current->param); + } + return 0; + } + case FLOAT_FLOAT_FLOAT_PARAMETER: + { + float value[3]; + for (int i = 0; i < 3; i++) { + value[i] = atoff(argv[i]); + if (cisnan(value[i])) { + efiPrintf("invalid float [%s]", argv[i]); + return -1; + } + } + VoidFloatFloatFloat callbackS = (VoidFloatFloatFloat) current->callback; + (*callbackS)(value[0], value[1], value[2]); + return 0; + } + case FLOAT_FLOAT_FLOAT_FLOAT_FLOAT_PARAMETER: + { + float value[5]; + for (int i = 0; i < 5; i++) { + value[i] = atoff(argv[i]); + if (cisnan(value[i])) { + efiPrintf("invalid float [%s]", argv[i]); + return -1; + } + } + VoidFloatFloatFloatFloatFloat callbackS = (VoidFloatFloatFloatFloatFloat) current->callback; + (*callbackS)(value[0], value[1], value[2], value[3], value[4]); + return 0; + } + case INT_FLOAT_PARAMETER: + { + int value1 = atoi(argv[0]); + if (absI(value1) == ERROR_CODE) { + #if EFI_PROD_CODE || EFI_SIMULATOR + efiPrintf("not an integer [%s]", argv[0]); + #endif + return -1; + } + float value2 = atoff(argv[1]); + if (cisnan(value2)) { + efiPrintf("invalid float [%s]", argv[1]); + return -1; + } + VoidIntFloat callback = (VoidIntFloat) current->callback; + callback(value1, value2); + return 0; + } + case ONE_PARAMETER_P: + case ONE_PARAMETER: + { + int value = atoi(argv[0]); + if (absI(value) == ERROR_CODE) { + #if EFI_PROD_CODE || EFI_SIMULATOR + efiPrintf("not an integer [%s]", argv[0]); + #endif + return -1; + } + if (current->parameterType == ONE_PARAMETER_P) { + VoidIntVoidPtr callback1 = (VoidIntVoidPtr) current->callback; + (*callback1)(value, current->param); + + } else { + VoidInt callback1 = (VoidInt) current->callback; + (*callback1)(value); + } + return 0; + } + default: + efiPrintf("Unsupported parameterType %d", current->parameterType); + break; + } + return -1; +} + void initConsoleLogic() { // resetConsoleActions(); addConsoleAction("help", helpCommand); @@ -509,41 +503,35 @@ char *validateSecureLine(char *line) { static char handleBuffer[200]; static bool handleConsoleLineInternal(const char *commandLine, int lineLength) { - strncpy(handleBuffer, commandLine, sizeof(handleBuffer) - 1); - handleBuffer[sizeof(handleBuffer) - 1] = 0; // we want this to be null-terminated for sure - char *line = handleBuffer; - int firstTokenLength = tokenLength(line); + int len = minI(lineLength, sizeof(handleBuffer) - 1); -// print("processing [%s] with %d actions\r\n", line, consoleActionCount); + strncpy(handleBuffer, commandLine, len); + handleBuffer[len] = 0; // we want this to be null-terminated for sure - if (firstTokenLength == lineLength) { - // no-param actions are processed here - for (int i = 0; i < consoleActionCount; i++) { - TokenCallback *current = &consoleActions[i]; - if (strEqual(line, current->token)) { - if (current->parameterType == NO_PARAMETER) { - (*current->callback)(); - } else if (current->parameterType == NO_PARAMETER_P) { - VoidPtr cb = (VoidPtr) current->callback; - (*cb)(current->param); - } - return true; + char *argv[10]; + int argc = setargs(handleBuffer, argv, 10); + + if (argc <= 0) { + efiPrintf("invalid input"); + return false; + } + + for (int i = 0; i < consoleActionCount; i++) { + TokenCallback *current = &consoleActions[i]; + if (strEqual(argv[0], current->token)) { + if ((argc - 1) != getParameterCount(current->parameterType)) { + efiPrintf("Incorrect argument count %d, expected %d", + (argc - 1), getParameterCount(current->parameterType)); + return false; } - } - } else { - char *ptr = line + firstTokenLength; - ptr[0] = 0; // change space into line end - ptr++; // start from next symbol - for (int i = 0; i < consoleActionCount; i++) { - TokenCallback *current = &consoleActions[i]; - if (strEqual(line, current->token)) { - handleActionWithParameter(current, ptr); - return true; - } + /* skip commant name */ + return handleActionWithParameter(current, argv + 1, argc - 1); } } - return false; + + /* unknown command */ + return 0; } /** @@ -562,11 +550,14 @@ void handleConsoleLine(char *line) { return; } - bool isKnownComman = handleConsoleLineInternal(line, lineLength); + int ret = handleConsoleLineInternal(line, lineLength); - if (!isKnownComman) { + if (ret == 0) { efiPrintf("unknown command [%s]", line); return; + } else if (ret < 0) { + efiPrintf("failed to handle command [%s]", line); + return; } #if EFI_PROD_CODE || EFI_SIMULATOR diff --git a/firmware/util/cli_registry.h b/firmware/util/cli_registry.h index 982ce5d4d9..8a5b8580b9 100644 --- a/firmware/util/cli_registry.h +++ b/firmware/util/cli_registry.h @@ -27,6 +27,8 @@ typedef enum { TWO_INTS_PARAMETER, TWO_INTS_PARAMETER_P, FLOAT_FLOAT_PARAMETER, + FLOAT_FLOAT_FLOAT_PARAMETER, + FLOAT_FLOAT_FLOAT_FLOAT_FLOAT_PARAMETER, FLOAT_FLOAT_PARAMETER_P, INT_FLOAT_PARAMETER, } action_type_e; @@ -40,6 +42,9 @@ typedef struct { int tokenLength(const char *msgp); +int findEndOfToken(const char *line); +char *unquote(char *line); + #ifdef __cplusplus extern "C" @@ -51,9 +56,6 @@ void resetConsoleActions(void); void helpCommand(void); void initConsoleLogic(); void handleConsoleLine(char *line); -int findEndOfToken(const char *line); -char *unquote(char *line); - void addConsoleAction(const char *token, Void callback); void addConsoleActionP(const char *token, VoidPtr callback, void *param); @@ -69,6 +71,8 @@ void addConsoleActionF(const char *token, VoidFloat callback); void addConsoleActionNANF(const char *token, VoidFloat callback); void addConsoleActionFF(const char *token, VoidFloatFloat callback); +void addConsoleActionFFF(const char *token, VoidFloatFloatFloat callback); +void addConsoleActionFFFFF(const char *token, VoidFloatFloatFloatFloatFloat callback); void addConsoleActionFFP(const char *token, VoidFloatFloatVoidPtr callback, void *param); void addConsoleActionS(const char *token, VoidCharPtr callback); diff --git a/unit_tests/tests/test_util.cpp b/unit_tests/tests/test_util.cpp index 25bb49f813..2f6413d604 100644 --- a/unit_tests/tests/test_util.cpp +++ b/unit_tests/tests/test_util.cpp @@ -239,6 +239,16 @@ static void testEchoSSS(const char *first, const char *second, const char *third lastThird = third; } +static float fFirst; +static float fSecond; +static float fThird; + +static void testEchoFFF(float first, float second, float third) { + fFirst = first; + fSecond = second; + fThird = third; +} + #define UNKNOWN_COMMAND "dfadasdasd" static loc_t GPSdata; @@ -361,6 +371,15 @@ TEST(misc, testConsoleLogic) { handleConsoleLine(buffer); ASSERT_TRUE(strEqual("\" 1\"", lastFirst)); + printf("\r\addConsoleActionFFF\r\n"); + addConsoleActionFFF("echofff", testEchoFFF); + strcpy(buffer, "echofff 1.0 2 00003.0"); + handleConsoleLine(buffer); + + ASSERT_EQ(1.0, fFirst); + ASSERT_EQ(2.0, fSecond); + ASSERT_EQ(3.0, fThird); + //addConsoleActionSSS("GPS", testGpsParser); }