fix(esp_commands): Incorporate review comments

Author: SoucheSouche

esp_commands/.build-test-rules.yml

@@ -1,6 +1,3 @@
 esp_commands/test_apps:
   enable:
-    - if: (IDF_TARGET == "linux") and (IDF_VERSION_MAJOR == 5 and IDF_VERSION_MINOR >= 3)
-      reason: "Test the main logic of the component (not target dependent)"
-    - if: IDF_TARGET == "esp32"
-      reason: "Test the placement of statically registered commands in dedicated flash section"
+    - if: (IDF_TARGET in ["esp32", "linux"]) and (IDF_VERSION_MAJOR == 5 and IDF_VERSION_MINOR >= 3)

esp_commands/README.md

@@ -60,15 +60,15 @@ typedef struct esp_command {
 Use the `ESP_COMMAND_REGISTER` macro to register a command at compile time:
 
 ```c
-static int my_cmd(void *ctx, int argc, char **argv) {
+static int my_cmd(void *context, esp_commands_exec_arg_t *cmd_arg, int argc, char **argv) {
     printf("Hello from my_cmd!\n");
     return 0;
 }
 
 ESP_COMMAND_REGISTER(my_cmd, tools, "Prints hello", my_cmd, NULL, NULL, NULL);
 ```
 
-This places the command into the `.esp_commands` section.
+This places the command into the `.esp_commands` section in flash.
 
 ### Dynamic Registration
 

esp_commands/include/esp_commands.h

@@ -10,170 +10,10 @@ extern "C" {
 #endif
 
 #include <stdbool.h>
+#include "esp_commands_utils.h"
 #include "esp_heap_caps.h"
 #include "esp_err.h"
 
-#define _STRINGIFY(name) #name
-
-/**
- * @brief Function pointer type for writing bytes.
- *
- * @param fd File descriptor.
- * @param buf Buffer containing bytes to write.
- * @param count Number of bytes to write.
- * @return Number of bytes written, or -1 on error.
- */
-typedef ssize_t (*esp_commands_write_t)(int fd, const void *buf, size_t count);
-
-/**
- * @brief Structure containing dynamic argument necessary for the\
- * command callback to execute properly
- */
-typedef struct esp_commands_exec_arg {
-    int out_fd; /*!< file descriptor that the command function has to use to output data */
-    esp_commands_write_t write_func; /*!< write function the command function has to use to output datga */
-    void *dynamic_ctx; /*!< dynamic context passed to the command function */
-} esp_commands_exec_arg_t;
-
-/**
- * @brief Console command main function type with user context
- *
- * This function type is used to implement a console command.
- *
- * @param context User-defined context passed at invocation
- * @param cmd_arg Structure containing dynamic arguments necessary for the command
- * @param argc Number of arguments
- * @param argv Array of argc entries, each pointing to a null-terminated string argument
- * @return Return code of the console command; 0 indicates success
- */
-typedef int (*esp_command_func_t)(void *context, esp_commands_exec_arg_t *cmd_arg, int argc, char **argv);
-
-/**
- * @brief Callback to generate a command hint
- *
- * This function is called to retrieve a short hint for a command,
- * typically used for auto-completion or UI help.
- *
- * @param context Context registered when the command was registered
- * @return Persistent string containing the generated hint
- */
-typedef const char *(*esp_command_hint_t)(void *context);
-
-/**
- * @brief Callback to generate a command glossary entry
- *
- * This function is called to retrieve detailed description or glossary
- * information for a command.
- *
- * @param context Context registered when the command was registered
- * @return Persistent string containing the generated glossary
- */
-typedef const char *(*esp_command_glossary_t)(void *context);
-
-/**
- * @brief Structure describing a console command
- *
- * @note The `group` field allows categorizing commands into groups,
- * which can simplify filtering or listing commands.
- */
-typedef struct esp_command {
-    const char *name; /*!< Name of the command */
-    const char *group; /*!< Command group to which this command belongs */
-    const char *help; /*!< Short help text for the command */
-    esp_command_func_t func; /*!< Function implementing the command */
-    void *func_ctx; /*!< User-defined context for the command function */
-    esp_command_hint_t hint_cb; /*!< Callback returning the hint for the command */
-    esp_command_glossary_t glossary_cb; /*!< Callback returning the glossary for the command */
-} esp_command_t;
-
-/**
- * @brief Macro to define a forced inline accessor for a string field of esp_command_t
- *
- * @param NAME Field name of the esp_command_t structure
- */
-#define DEFINE_FIELD_ACCESSOR(NAME) \
-    static inline __attribute__((always_inline)) \
-    const char *get_##NAME(const esp_command_t *cmd) { \
-        if (!cmd) { \
-            return NULL; \
-        } \
-        return cmd->NAME; \
-    }
-
-/**
- * @brief Macro expanding to
- * static inline __attribute__((always_inline)) const char *get_name(esp_command_t *cmd) {
- *     if (!cmd) {
- *         return NULL;
- *     }
- *     return cmd->name;
- * }
- */
-DEFINE_FIELD_ACCESSOR(name)
-
-/**
- * @brief Macro expanding to
- * static inline __attribute__((always_inline)) const char *get_group(esp_command_t *cmd) {
- *     if (!cmd) {
- *         return NULL;
- *     }
- *     return cmd->group;
- * }
- */
-DEFINE_FIELD_ACCESSOR(group)
-
-/**
- * @brief Macro expanding to
- * static inline __attribute__((always_inline)) const char *get_help(esp_command_t *cmd) {
- *     if (!cmd) {
- *         return NULL;
- *     }
- *     return cmd->help;
- * }
- */
-DEFINE_FIELD_ACCESSOR(help)
-
-/**
- * @brief Macro to create the accessor function name for a field of esp_command_t
- *
- * @param NAME Field name of esp_command_t
- */
-#define FIELD_ACCESSOR(NAME) get_##NAME
-
-/**
- * @brief Configuration parameters for esp_commands_manager initialization
- */
-typedef struct esp_commands_config {
-    uint32_t heap_caps_used;            /*!< Set of heap capabilities to be used to perform internal allocations */
-    size_t max_cmdline_length;          /*!< Maximum length of the command line buffer, in bytes */
-    size_t max_cmdline_args;            /*!< Maximum number of command line arguments to parse */
-    int hint_color;                     /*!< ANSI color code used for hint text */
-    bool hint_bold;                     /*!< If true, display hint text in bold */
-} esp_commands_config_t;
-
-/**
- * @brief Callback for a completed command name
- *
- * This callback is called when a command is successfully completed.
- *
- * @param cb_ctx Opaque pointer pointing at the context passed to the callback
- * @param completed_cmd_name Completed command name
- */
-typedef void (*esp_command_get_completion_t)(void *cb_ctx, const char *completed_cmd_name);
-
-/**
- * @brief Callback to retrieve a string field of esp_command_t
- *
- * @param cmd Command object
- * @return Value of the requested string field
- */
-typedef const char *(*esp_commands_get_field_t)(const esp_command_t *cmd);
-
-/**
- * @brief Opaque handle to a set of commands
- */
-typedef struct esp_command_sets *esp_command_set_handle_t;
-
 /**
  * @brief Update the component configuration
  *
@@ -187,9 +27,6 @@ esp_err_t esp_commands_update_config(const esp_commands_config_t *config);
  * @brief macro registering a command and placing it in a specific section of flash.rodata
  * @note see the linker.lf file for more information concerning the section characteristics
  */
-#define _ESP_REPL_STRINGIFY(x) #x
-#define ESP_REPL_STRINGIFY(x) _ESP_REPL_STRINGIFY(x)
-
 #define _ESP_COMMAND_REGISTER(cmd_name, cmd_group, cmd_help, cmd_func, cmd_func_ctx, cmd_hint_cb, cmd_glossary_cb) \
     static_assert((cmd_func) != NULL); \
     static const esp_command_t cmd_name __attribute__((used, section(".esp_commands" "." _ESP_REPL_STRINGIFY(cmd_name)), aligned(4))) = { \

esp_commands/include/esp_commands_utils.h

@@ -0,0 +1,181 @@
+/*
+ * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+#pragma once
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdbool.h>
+#include <stdint.h>
+
+#define _ESP_REPL_STRINGIFY(x) #x
+#define ESP_REPL_STRINGIFY(x) _ESP_REPL_STRINGIFY(x)
+
+/**
+ * @brief Function pointer type for writing bytes.
+ *
+ * @param fd File descriptor.
+ * @param buf Buffer containing bytes to write.
+ * @param count Number of bytes to write.
+ * @return Number of bytes written, or -1 on error.
+ */
+typedef ssize_t (*esp_commands_write_t)(int fd, const void *buf, size_t count);
+
+/**
+ * @brief Structure containing dynamic argument necessary for the\
+ * command callback to execute properly
+ */
+typedef struct esp_commands_exec_arg {
+    int out_fd; /*!< file descriptor that the command function has to use to print data in the environment it was called from */
+    esp_commands_write_t write_func; /*!< write function the command function has to use to output datga */
+    void *dynamic_ctx; /*!< dynamic context passed to the command function */
+} esp_commands_exec_arg_t;
+
+/**
+ * @brief Console command main function type with user context
+ *
+ * This function type is used to implement a console command.
+ *
+ * @param context User-defined context passed at invocation
+ * @param cmd_arg Structure containing dynamic arguments necessary for the command
+ * @param argc Number of arguments
+ * @param argv Array of argc entries, each pointing to a null-terminated string argument
+ * @return Return code of the console command; 0 indicates success
+ */
+typedef int (*esp_command_func_t)(void *context, esp_commands_exec_arg_t *cmd_arg, int argc, char **argv);
+
+/**
+ * @brief Callback to generate a command hint
+ *
+ * This function is called to retrieve a short hint for a command,
+ * typically used for auto-completion or UI help.
+ *
+ * @param context Context registered when the command was registered
+ * @return Persistent string containing the generated hint
+ */
+typedef const char *(*esp_command_hint_t)(void *context);
+
+/**
+ * @brief Callback to generate a command glossary entry
+ *
+ * This function is called to retrieve detailed description or glossary
+ * information for a command.
+ *
+ * @param context Context registered when the command was registered
+ * @return Persistent string containing the generated glossary
+ */
+typedef const char *(*esp_command_glossary_t)(void *context);
+
+/**
+ * @brief Structure describing a console command
+ *
+ * @note The `group` field allows categorizing commands into groups,
+ * which can simplify filtering or listing commands.
+ */
+typedef struct esp_command {
+    const char *name; /*!< Name of the command */
+    const char *group; /*!< Command group to which this command belongs */
+    const char *help; /*!< Short help text for the command */
+    esp_command_func_t func; /*!< Function implementing the command */
+    void *func_ctx; /*!< User-defined context for the command function */
+    esp_command_hint_t hint_cb; /*!< Callback returning the hint for the command */
+    esp_command_glossary_t glossary_cb; /*!< Callback returning the glossary for the command */
+} esp_command_t;
+
+/**
+ * @brief Configuration parameters for esp_commands_manager initialization
+ */
+typedef struct esp_commands_config {
+    uint32_t heap_caps_used;            /*!< Set of heap capabilities to be used to perform internal allocations */
+    size_t max_cmdline_length;          /*!< Maximum length of the command line buffer, in bytes */
+    size_t max_cmdline_args;            /*!< Maximum number of command line arguments to parse */
+    int hint_color;                     /*!< ANSI color code used for hint text */
+    bool hint_bold;                     /*!< If true, display hint text in bold */
+} esp_commands_config_t;
+
+/**
+ * @brief Callback for a completed command name
+ *
+ * This callback is called when a command is successfully completed.
+ *
+ * @param cb_ctx Opaque pointer pointing at the context passed to the callback
+ * @param completed_cmd_name Completed command name
+ */
+typedef void (*esp_command_get_completion_t)(void *cb_ctx, const char *completed_cmd_name);
+
+/**
+ * @brief Callback to retrieve a string field of esp_command_t
+ *
+ * @param cmd Command object
+ * @return Value of the requested string field
+ */
+typedef const char *(*esp_commands_get_field_t)(const esp_command_t *cmd);
+
+/**
+ * @brief Opaque handle to a set of commands
+ */
+typedef struct esp_command_sets *esp_command_set_handle_t;
+
+/**
+ * @brief Macro to define a forced inline accessor for a string field of esp_command_t
+ *
+ * @param NAME Field name of the esp_command_t structure
+ */
+#define DEFINE_FIELD_ACCESSOR(NAME) \
+    static inline __attribute__((always_inline)) \
+    const char *get_##NAME(const esp_command_t *cmd) { \
+        if (!cmd) { \
+            return NULL; \
+        } \
+        return cmd->NAME; \
+    }
+
+/**
+ * @brief Macro expanding to
+ * static inline __attribute__((always_inline)) const char *get_name(esp_command_t *cmd) {
+ *     if (!cmd) {
+ *         return NULL;
+ *     }
+ *     return cmd->name;
+ * }
+ */
+DEFINE_FIELD_ACCESSOR(name)
+
+/**
+ * @brief Macro expanding to
+ * static inline __attribute__((always_inline)) const char *get_group(esp_command_t *cmd) {
+ *     if (!cmd) {
+ *         return NULL;
+ *     }
+ *     return cmd->group;
+ * }
+ */
+DEFINE_FIELD_ACCESSOR(group)
+
+/**
+ * @brief Macro expanding to
+ * static inline __attribute__((always_inline)) const char *get_help(esp_command_t *cmd) {
+ *     if (!cmd) {
+ *         return NULL;
+ *     }
+ *     return cmd->help;
+ * }
+ */
+DEFINE_FIELD_ACCESSOR(help)
+
+/**
+ * @brief Macro to create the accessor function name for a field of esp_command_t
+ *
+ * @note Those accessor functions are defined in esp_commands_internal.h
+ *
+ * @param NAME Field name of esp_command_t
+ */
+#define FIELD_ACCESSOR(NAME) get_##NAME
+
+#ifdef __cplusplus
+}
+#endif

esp_commands/test_apps/pytest_esp_commands.py

@@ -10,6 +10,6 @@
     not bool(glob.glob(f'{Path(__file__).parent.absolute()}/build*/')),
     reason="Skip the idf version that did not build"
 )
-@idf_parametrize('target', ['linux'], indirect=['target'])
+@idf_parametrize('target', ['linux', 'esp32'], indirect=['target'])
 def test_esp_commands(dut) -> None:
     dut.run_all_single_board_cases()