sys/shell: reduce overhead of XFA shell commands

.cargo/config.toml

@@ -6,4 +6,4 @@
 
 [patch.crates-io]
 riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" }
-riot-wrappers = { git = "https://github.com/RIOT-OS/rust-riot-wrappers" }
+riot-wrappers = { git = "https://github.com/maribu/rust-riot-wrappers", branch = "fix-riot-pr20958" }

makefiles/pseudomodules.inc.mk

@@ -463,6 +463,7 @@ PSEUDOMODULES += shell_cmd_udptty
 PSEUDOMODULES += shell_cmd_vfs
 PSEUDOMODULES += shell_cmds_default
 PSEUDOMODULES += shell_hooks
+PSEUDOMODULES += shell_json
 PSEUDOMODULES += shell_lock_auto_locking
 PSEUDOMODULES += shield_llcc68
 PSEUDOMODULES += shield_w5100

sys/include/shell.h

@@ -19,6 +19,25 @@
  * there is no expectation of security of the system when an attacker gains
  * access to the shell.
  *
+ * ## Usage
+ *
+ * Enable the `shell` module e.g. by adding the following snippet to your
+ * applications `Makefile`.
+ *
+ * ```
+ * USEMODULE += shell
+ * ```
+ *
+ * And run the shell using @ref shell_run_forever e.g. from the `main` thread
+ * after everything is set up. This call will never return.
+ *
+ * ## Builtin Commands
+ *
+ * The commands `help` and `cmds_json` are builtins that print the list of
+ * available commands: The former prints a human readable table and is always
+ * available, the latter requires module `shell_json` to be used and will
+ * give the same info machine readable.
+ *
  * @{
  *
  * @file
@@ -283,15 +302,14 @@ int shell_parse_file(const shell_command_t *commands,
  * ```
  */
 #define SHELL_COMMAND(cmd, help, func) \
-    XFA_USE_CONST(shell_command_xfa_t*, shell_commands_xfa); \
+    XFA_USE_CONST(shell_command_xfa_t, shell_commands_xfa_v2); \
     static FLASH_ATTR const char _xfa_ ## cmd ## _cmd_name[] = #cmd; \
     static FLASH_ATTR const char _xfa_ ## cmd ## _cmd_desc[] = help; \
-    static const shell_command_xfa_t _xfa_ ## cmd ## _cmd = { \
+    XFA_CONST(shell_command_xfa_t, shell_commands_xfa_v2, 0) _xfa_ ## cmd ## _cmd = { \
         .name = _xfa_ ## cmd ## _cmd_name, \
         .desc = _xfa_ ## cmd ## _cmd_desc, \
         .handler = &func \
-    }; \
-    XFA_ADD_PTR(shell_commands_xfa, cmd, cmd, &_xfa_ ## cmd ## _cmd)
+    };
 #endif /* __cplusplus */
 
 #ifdef __cplusplus

sys/shell/shell.c

@@ -47,7 +47,7 @@
 #endif
 
 /* define shell command cross file array */
-XFA_INIT_CONST(shell_command_xfa_t*, shell_commands_xfa);
+XFA_INIT_CONST(shell_command_xfa_t, shell_commands_xfa_v2);
 
 #define ETX '\x03'  /** ASCII "End-of-Text", or Ctrl-C */
 #define EOT '\x04'  /** ASCII "End-of-Transmission", or Ctrl-D */
@@ -102,10 +102,10 @@ static shell_command_handler_t search_commands(const shell_command_t *entry,
 
 static shell_command_handler_t search_commands_xfa(char *command)
 {
-    unsigned n = XFA_LEN(shell_command_t*, shell_commands_xfa);
+    unsigned n = XFA_LEN(shell_command_t, shell_commands_xfa_v2);
 
     for (unsigned i = 0; i < n; i++) {
-        const volatile shell_command_xfa_t *entry = shell_commands_xfa[i];
+        const volatile shell_command_xfa_t *entry = &shell_commands_xfa_v2[i];
         if (flash_strcmp(command, entry->name) == 0) {
             return entry->handler;
         }
@@ -129,6 +129,38 @@ static shell_command_handler_t find_handler(
     return handler;
 }
 
+static void print_commands_json(const shell_command_t *cmd_list)
+{
+    bool first = true;
+
+    printf("{\"cmds\": [");
+
+    if (cmd_list) {
+        for (const shell_command_t *entry = cmd_list; entry->name != NULL; entry++) {
+            if (first) {
+                first = false;
+            }
+            else {
+                printf(", ");
+            }
+            printf("{\"cmd\": \"%s\", \"desc\": \"%s\"}", entry->name, entry->desc);
+        }
+    }
+
+    unsigned n = XFA_LEN(shell_command_xfa_t*, shell_commands_xfa);
+    for (unsigned i = 0; i < n; i++) {
+        if (first) {
+            first = false;
+        }
+        else {
+            printf(", ");
+        }
+        const volatile shell_command_xfa_t *entry = shell_commands_xfa[i];
+        printf("{\"cmd\": \"%s\", \"desc\": \"%s\"}", entry->name, entry->desc);
+    }
+    puts("]}");
+}
+
 static void print_commands(const shell_command_t *entry)
 {
     for (; entry->name != NULL; entry++) {
@@ -138,9 +170,9 @@ static void print_commands(const shell_command_t *entry)
 
 static void print_commands_xfa(void)
 {
-    unsigned n = XFA_LEN(shell_command_xfa_t*, shell_commands_xfa);
+    unsigned n = XFA_LEN(shell_command_xfa_t, shell_commands_xfa_v2);
     for (unsigned i = 0; i < n; i++) {
-        const volatile shell_command_xfa_t *entry = shell_commands_xfa[i];
+        const volatile shell_command_xfa_t *entry = &shell_commands_xfa_v2[i];
         printf("%-20" PRIsflash " %" PRIsflash "\n",
                      entry->name, entry->desc);
     }
@@ -343,6 +375,9 @@ int shell_handle_input_line(const shell_command_t *command_list, char *line)
             print_help(command_list);
             return 0;
         }
+        else if (IS_USED(MODULE_SHELL_JSON) && !strcmp("cmds_json", argv[0])) {
+            print_commands_json(command_list);
+        }
         else {
             printf("shell: command not found: %s\n", argv[0]);
         }

tests/rust_libs/Makefile

@@ -2,6 +2,7 @@ include ../Makefile.tests_common
 
 USEMODULE += shell
 USEMODULE += shell_democommands
+USEMODULE += shell_json # for automated testing
 USEMODULE += ztimer_msec
 
 FEATURES_REQUIRED += rust_target

tests/rust_libs/tests/01-run.py

@@ -7,34 +7,25 @@
 # General Public License v2.1. See the file LICENSE in the top level
 # directory for more details.
 
+import json
 import sys
 from testrunner import run
 
 
-EXPECTED_HELP = (
-    'Command              Description',
-    '---------------------------------------',
-    'bufsize              Get the shell\'s buffer size',
-    'start_test           starts a test',
-    'end_test             ends a test',
-    'echo                 prints the input command',
-    'empty                print nothing on command',
-    'hello_world          Print a greeting',
-    'xfa_test1            xfa test command 1',
-    'xfa_test2            xfa test command 2',
-)
+EXPECTED_CMDS = {
+        'bufsize': 'Get the shell\'s buffer size',
+        'start_test': 'starts a test',
+        'end_test': 'ends a test',
+        'echo': 'prints the input command',
+        'empty': 'print nothing on command',
+        'periodic': 'periodically print command',
+        'hello_world': 'Print a greeting',
+        'xfa_test1': 'xfa test command 1',
+        'xfa_test2': 'xfa test command 2',
+}
 
 PROMPT = '> '
 
-CMDS = (
-    ('start_test', '[TEST_START]'),
-
-    # test default commands
-    ('help', EXPECTED_HELP),
-
-    ('end_test', '[TEST_END]'),
-)
-
 CMDS_REGEX = {'ps.rs'}
 
 
@@ -48,11 +39,25 @@
         else:
             child.expect_exact(line)
 
+def check_cmd_list(child):
+    child.expect(PROMPT)
+    child.sendline('cmds_json')
+    child.expect(r"(\{[^\n\r]*\})\r\n")
+    cmdlist = json.loads(child.match.group(1))["cmds"]
+    cmds = set(EXPECTED_CMDS)
+    for item in cmdlist:
+        assert item['cmd'] in EXPECTED_CMDS, f"command {item['cmd']} not expected"
+        assert item['cmd'] in cmds, f"command {item['cmd']} listed twice"
+        assert item['desc'] == EXPECTED_CMDS[item['cmd']], f"description of {item['cmd']} not expected"
+        cmds.remove(item['cmd'])
+
+    assert len(cmds) == 0, f"commands {cmds} missing"
 
 def testfunc(child):
     # loop other defined commands and expected output
-    for cmd, expected in CMDS:
-        check_cmd(child, cmd, expected)
+    check_cmd(child, 'start_test', '[TEST_START]')
+    check_cmd_list(child)
+    check_cmd(child, 'end_test', '[TEST_END]')
 
 
 if __name__ == "__main__":