From c18d8452215c5db90a095ef70931d1f8cff195f9 Mon Sep 17 00:00:00 2001 From: nhjschulz Date: Tue, 17 Sep 2024 10:11:45 +0200 Subject: [PATCH 1/3] Added help terminal command Added new command help to print the supported commands list. Side changes: * Store cmd list in a table instead of if-else chain testing. * Avoid unneeded dynamic construction of static variables (strlen calls) * Read up to 12 bytes in process ( 32bit aligned to avoid gab byte loss of no gain). --- README.md | 2 + src/Common/MiniTerminal.cpp | 117 +++++++++++++++++------------------- src/Common/MiniTerminal.h | 21 ++++++- 3 files changed, 77 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 9842fd51..b78d379d 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,8 @@ Connect PIXELIX with your PC via usb and start a terminal. Use the following com * A status of 0 means everything is ok. * Other than 0, see their meaning in the low [low level error code table](#the-display-only-shows-a-error-code-like-e4-what-does-that-mean). Note, the status of 1 is equal to E1 in the error code table and etc. +Enter ```help``` to get a list of all supported commands. + ## PIXELIX Is Ready After configuration, restart again and voila, PIXELIX will be available in your wifi network. diff --git a/src/Common/MiniTerminal.cpp b/src/Common/MiniTerminal.cpp index a43bdd9a..7440c80f 100644 --- a/src/Common/MiniTerminal.cpp +++ b/src/Common/MiniTerminal.cpp @@ -62,40 +62,35 @@ *****************************************************************************/ /** Command: ping */ -static const char* PING = "ping"; - -/** Command length: ping */ -static const size_t PING_LEN = strlen(PING); +static const char PING[] = "ping"; /** Command: reset */ -static const char* RESET = "reset"; - -/** Command length: reset */ -static const size_t RESET_LEN = strlen(RESET); +static const char RESET[] = "reset"; /** Command: write wifi passphrase */ -static const char* WRITE_WIFI_PASSPHRASE = "write wifi passphrase "; - -/** Command length: write wifi passphrase */ -static const size_t WRITE_WIFI_PASSPHRASE_LEN = strlen(WRITE_WIFI_PASSPHRASE); +static const char WRITE_WIFI_PASSPHRASE[] = "write wifi passphrase "; /** Command: write wifi ssid */ -static const char* WRITE_WIFI_SSID = "write wifi ssid "; - -/** Command length: write wifi ssid */ -static const size_t WRITE_WIFI_SSID_LEN = strlen(WRITE_WIFI_SSID); +static const char WRITE_WIFI_SSID[] = "write wifi ssid "; /** Command: get ip */ -static const char* GET_IP = "get ip"; - -/** Command length: get ipaddress */ -static const size_t GET_IP_LEN = strlen(GET_IP); +static const char GET_IP[] = "get ip"; /** Command: status */ -static const char* GET_STATUS = "get status"; +static const char GET_STATUS[] = "get status"; -/** Command length: status */ -static const size_t GET_STATUS_LEN = strlen(GET_STATUS); +/** Command: help */ +static const char HELP[] = "help"; + +const MiniTerminal::CmdTableEntry MiniTerminal::m_cmdTable[] = { + { PING, sizeof(PING) - 1U, &MiniTerminal::cmdPing }, + { RESET, sizeof(RESET) - 1U, &MiniTerminal::cmdReset }, + { WRITE_WIFI_PASSPHRASE, sizeof(WRITE_WIFI_PASSPHRASE) - 1U, &MiniTerminal::cmdWriteWifiPassphrase }, + { WRITE_WIFI_SSID, sizeof(WRITE_WIFI_SSID) - 1U, &MiniTerminal::cmdWriteWifiSSID }, + { GET_IP, sizeof(GET_IP) - 1U, &MiniTerminal::cmdGetIPAddress }, + { GET_STATUS, sizeof(GET_STATUS) - 1U, &MiniTerminal::cmdGetStatus }, + { HELP, sizeof(HELP) - 1U, &MiniTerminal::cmdHelp }, +}; /****************************************************************************** * Public Methods @@ -110,15 +105,15 @@ void MiniTerminal::process() /* Process the read input data. */ while(read > idx) { - bool echoOn = false; + char currentChar = buffer[idx]; /* Command finished? */ - if (ASCII_LF == buffer[idx]) + if (ASCII_LF == currentChar) { /* Don't echo mechanism, because its too late in case the * command may write a result too. */ - (void)m_stream.write(buffer[idx]); + (void)m_stream.write(currentChar); m_input[m_writeIndex] = '\0'; @@ -133,8 +128,8 @@ void MiniTerminal::process() m_input[m_writeIndex] = '\0'; } /* Remove the last character from command line? */ - else if ((ASCII_DEL == buffer[idx]) || - (ASCII_BS == buffer[idx])) + else if ((ASCII_DEL == currentChar) || + (ASCII_BS == currentChar)) { if (0 < m_writeIndex) { @@ -153,21 +148,14 @@ void MiniTerminal::process() else if (INPUT_BUFFER_SIZE > (m_writeIndex + 1U)) { /* Valid character? */ - if ((' ' <= buffer[idx]) && - ('~' >= buffer[idx])) + if ((' ' <= currentChar) && + ('~' >= currentChar)) { - m_input[m_writeIndex] = buffer[idx]; - ++m_writeIndex; - - echoOn = true; + m_input[m_writeIndex++] = currentChar; + (void)m_stream.write(currentChar); } } - if (true == echoOn) - { - (void)m_stream.write(buffer[idx]); - } - ++idx; } } @@ -202,31 +190,20 @@ void MiniTerminal::writeError(const char* result) void MiniTerminal::executeCommand(const char* cmdLine) { - if (0 == strcmp(cmdLine, PING)) - { - cmdPing(&cmdLine[PING_LEN]); - } - else if (0 == strcmp(cmdLine, RESET)) - { - cmdReset(&cmdLine[RESET_LEN]); - } - else if (0 == strncmp(cmdLine, WRITE_WIFI_PASSPHRASE, WRITE_WIFI_PASSPHRASE_LEN)) - { - cmdWriteWifiPassphrase(&cmdLine[WRITE_WIFI_PASSPHRASE_LEN]); - } - else if (0 == strncmp(cmdLine, WRITE_WIFI_SSID, WRITE_WIFI_SSID_LEN)) - { - cmdWriteWifiSSID(&cmdLine[WRITE_WIFI_SSID_LEN]); - } - else if (0 == strncmp(cmdLine, GET_IP, GET_IP_LEN)) - { - cmdGetIPAddress(&cmdLine[GET_IP_LEN]); - } - else if (0 == strncmp(cmdLine, GET_STATUS, GET_STATUS_LEN)) + uint32_t idx = 0U; + + for (idx = 0U; UTIL_ARRAY_NUM(m_cmdTable) > idx; ++idx) { - cmdGetStatus(&cmdLine[GET_STATUS_LEN]); + const CmdTableEntry& entry(m_cmdTable[idx]); + + if (0 == strncmp(cmdLine, entry.cmdStr, entry.cmdLen)) + { + (this->*entry.handler)(&cmdLine[entry.cmdLen]); + break; + } } - else + + if (UTIL_ARRAY_NUM(m_cmdTable) == idx) { writeError("Unknown command.\n"); } @@ -324,6 +301,22 @@ void MiniTerminal::cmdGetStatus(const char* par) } } +void MiniTerminal::cmdHelp(const char* par) +{ + UTIL_NOT_USED(par); + + m_stream.write("Supported commands:\n"); + + for (size_t idx = 0U; UTIL_ARRAY_NUM(m_cmdTable) > idx; ++idx) + { + (void)m_stream.write(" "); + (void)m_stream.write(m_cmdTable[idx].cmdStr); + (void)m_stream.write("\n"); + } + + writeSuccessful(); +} + /****************************************************************************** * External Functions *****************************************************************************/ diff --git a/src/Common/MiniTerminal.h b/src/Common/MiniTerminal.h index 0707ddca..b33ecdb3 100644 --- a/src/Common/MiniTerminal.h +++ b/src/Common/MiniTerminal.h @@ -107,13 +107,23 @@ class MiniTerminal return isRestartRequested; } + /** + * @brief Table entry for known terminal commands + * + */ + struct CmdTableEntry { + const char *cmdStr; /**< Command string. */ + size_t cmdLen; /**< Length of command string. */ + void (MiniTerminal::*handler)(const char *); /**< Command handler function. */ + }; + private: static const char ASCII_BS = 8; /**< ASCII backspace value */ static const char ASCII_LF = 10; /**< ASCII line feed value */ static const char ASCII_SP = 32; /**< ASCII space value */ static const char ASCII_DEL = 127; /**< ASCII delete value */ - static const size_t LOCAL_BUFFER_SIZE = 10U; /**< Buffer size in byte to read during processing. */ + static const size_t LOCAL_BUFFER_SIZE = 12U; /**< Buffer size in byte to read during processing. */ static const size_t INPUT_BUFFER_SIZE = 80U; /**< Buffer size of one input command line in byte. */ Stream& m_stream; /**< In/Out-stream. */ @@ -121,6 +131,8 @@ class MiniTerminal size_t m_writeIndex; /**< Write index to the command line buffer. */ bool m_isRestartRequested; /**< Restart requested? */ + static const CmdTableEntry m_cmdTable[]; /**< Table with supported commands. */ + /** * Write successful response. * @@ -183,6 +195,13 @@ class MiniTerminal * @param[in] par Parameter */ void cmdGetStatus(const char* par); + + /** + * print command help message. + * + * @param[in] par Parameter + */ + void cmdHelp(const char* par); }; /****************************************************************************** From c074ae9eacbb6b9a3441b8890a5330164e4bbc58 Mon Sep 17 00:00:00 2001 From: nhjschulz Date: Tue, 24 Sep 2024 20:19:58 +0200 Subject: [PATCH 2/3] MinTerm: Reduce rom consumption * Remove ping command (now there is help instead) * Don't store cmd string length, but calc it on enter key * Remove unneeded if's This version need ~80 bytes less flash space then before. --- README.md | 1 - src/Common/MiniTerminal.cpp | 76 ++++++++++++++++--------------------- src/Common/MiniTerminal.h | 10 +---- 3 files changed, 34 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index b78d379d..60275bd9 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,6 @@ Use the following default credentials to get access to the PIXELIX web interface ## Variant 2: Configure wifi station SSID and passphrase with the terminal Connect PIXELIX with your PC via usb and start a terminal. Use the following commands to set the wifi SSID and passphrase of your home wifi network: -* Test: ```ping``` * Write wifi passphrase: ```write wifi passphrase ``` * Write wifi SSID: ```write wifi ssid ``` * Restart PIXELIX: ```reset``` diff --git a/src/Common/MiniTerminal.cpp b/src/Common/MiniTerminal.cpp index 7440c80f..12dc7480 100644 --- a/src/Common/MiniTerminal.cpp +++ b/src/Common/MiniTerminal.cpp @@ -61,9 +61,6 @@ * Local Variables *****************************************************************************/ -/** Command: ping */ -static const char PING[] = "ping"; - /** Command: reset */ static const char RESET[] = "reset"; @@ -83,13 +80,12 @@ static const char GET_STATUS[] = "get status"; static const char HELP[] = "help"; const MiniTerminal::CmdTableEntry MiniTerminal::m_cmdTable[] = { - { PING, sizeof(PING) - 1U, &MiniTerminal::cmdPing }, - { RESET, sizeof(RESET) - 1U, &MiniTerminal::cmdReset }, - { WRITE_WIFI_PASSPHRASE, sizeof(WRITE_WIFI_PASSPHRASE) - 1U, &MiniTerminal::cmdWriteWifiPassphrase }, - { WRITE_WIFI_SSID, sizeof(WRITE_WIFI_SSID) - 1U, &MiniTerminal::cmdWriteWifiSSID }, - { GET_IP, sizeof(GET_IP) - 1U, &MiniTerminal::cmdGetIPAddress }, - { GET_STATUS, sizeof(GET_STATUS) - 1U, &MiniTerminal::cmdGetStatus }, - { HELP, sizeof(HELP) - 1U, &MiniTerminal::cmdHelp }, + { RESET, &MiniTerminal::cmdReset }, + { WRITE_WIFI_PASSPHRASE, &MiniTerminal::cmdWriteWifiPassphrase }, + { WRITE_WIFI_SSID, &MiniTerminal::cmdWriteWifiSSID }, + { GET_IP, &MiniTerminal::cmdGetIPAddress }, + { GET_STATUS, &MiniTerminal::cmdGetStatus }, + { HELP, &MiniTerminal::cmdHelp }, }; /****************************************************************************** @@ -133,7 +129,7 @@ void MiniTerminal::process() { if (0 < m_writeIndex) { - char removeSeq[] = + static char removeSeq[] = { ASCII_BS, ASCII_SP, @@ -151,7 +147,8 @@ void MiniTerminal::process() if ((' ' <= currentChar) && ('~' >= currentChar)) { - m_input[m_writeIndex++] = currentChar; + m_input[m_writeIndex] = currentChar; + ++m_writeIndex; (void)m_stream.write(currentChar); } } @@ -194,11 +191,12 @@ void MiniTerminal::executeCommand(const char* cmdLine) for (idx = 0U; UTIL_ARRAY_NUM(m_cmdTable) > idx; ++idx) { - const CmdTableEntry& entry(m_cmdTable[idx]); + const CmdTableEntry entry = m_cmdTable[idx]; + const size_t len = strlen(entry.cmdStr); - if (0 == strncmp(cmdLine, entry.cmdStr, entry.cmdLen)) + if (0 == strncmp(cmdLine, entry.cmdStr, len)) { - (this->*entry.handler)(&cmdLine[entry.cmdLen]); + (this->*entry.handler)(&cmdLine[len]); break; } } @@ -209,12 +207,6 @@ void MiniTerminal::executeCommand(const char* cmdLine) } } -void MiniTerminal::cmdPing(const char* par) -{ - UTIL_NOT_USED(par); - writeSuccessful("pong\n"); -} - void MiniTerminal::cmdReset(const char* par) { UTIL_NOT_USED(par); @@ -268,37 +260,35 @@ void MiniTerminal::cmdWriteWifiSSID(const char* par) void MiniTerminal::cmdGetIPAddress(const char* par) { - if (nullptr != par) - { - String result; - - if (WIFI_MODE_AP == WiFi.getMode()) - { - result = WiFi.softAPIP().toString(); - } - else - { - result = WiFi.localIP().toString(); - } + UTIL_NOT_USED(par); - result += "\n"; + String result; - writeSuccessful(result.c_str()); + if (WIFI_MODE_AP == WiFi.getMode()) + { + result = WiFi.softAPIP().toString(); } + else + { + result = WiFi.localIP().toString(); + } + + result += "\n"; + + writeSuccessful(result.c_str()); } void MiniTerminal::cmdGetStatus(const char* par) { - if (nullptr != par) - { - ErrorState::ErrorId status = ErrorState::getInstance().getErrorId(); - String result; + UTIL_NOT_USED(par); - result += static_cast(status); - result += "\n"; + ErrorState::ErrorId status = ErrorState::getInstance().getErrorId(); + String result; - writeSuccessful(result.c_str()); - } + result += static_cast(status); + result += "\n"; + + writeSuccessful(result.c_str()); } void MiniTerminal::cmdHelp(const char* par) diff --git a/src/Common/MiniTerminal.h b/src/Common/MiniTerminal.h index b33ecdb3..4a567795 100644 --- a/src/Common/MiniTerminal.h +++ b/src/Common/MiniTerminal.h @@ -113,7 +113,6 @@ class MiniTerminal */ struct CmdTableEntry { const char *cmdStr; /**< Command string. */ - size_t cmdLen; /**< Length of command string. */ void (MiniTerminal::*handler)(const char *); /**< Command handler function. */ }; @@ -154,13 +153,6 @@ class MiniTerminal */ void executeCommand(const char* cmdLine); - /** - * Ping command. - * - * @param[in] par Parameter - */ - void cmdPing(const char* par); - /** * Reset the device. * @@ -197,7 +189,7 @@ class MiniTerminal void cmdGetStatus(const char* par); /** - * print command help message. + * Print command help message. * * @param[in] par Parameter */ From 8d3f478ce22fba8af46e22df247efbd30eb2678e Mon Sep 17 00:00:00 2001 From: nhjschulz Date: Wed, 25 Sep 2024 19:14:25 +0200 Subject: [PATCH 3/3] MiniTermimal: Make backspace char sequence const Review feedback --- src/Common/MiniTerminal.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/MiniTerminal.cpp b/src/Common/MiniTerminal.cpp index 12dc7480..c4d6b601 100644 --- a/src/Common/MiniTerminal.cpp +++ b/src/Common/MiniTerminal.cpp @@ -129,7 +129,7 @@ void MiniTerminal::process() { if (0 < m_writeIndex) { - static char removeSeq[] = + static const char removeSeq[] = { ASCII_BS, ASCII_SP,