From a538dcd8f8db5758ffe89f58c63a821416369b6f Mon Sep 17 00:00:00 2001 From: Peter Bigot Date: Mon, 22 Jun 2020 09:56:19 -0500 Subject: [PATCH] shell: refactor device_name_get implementation Several shell modules use cloned code to iterate over all devices and identify the nth instance that meets some criteria. The code was repetitive and included various errors. Abstract to a helper function that performs the check consistently. Signed-off-by: Peter Bigot --- drivers/flash/flash_shell.c | 16 ++-------------- drivers/i2c/i2c_shell.c | 17 ++--------------- drivers/sensor/sensor_shell.c | 16 ++-------------- include/shell/shell.h | 18 ++++++++++++++++++ subsys/shell/shell_utils.c | 27 +++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 43 deletions(-) diff --git a/drivers/flash/flash_shell.c b/drivers/flash/flash_shell.c index 577755fa73d..69cc9e31689 100644 --- a/drivers/flash/flash_shell.c +++ b/drivers/flash/flash_shell.c @@ -225,24 +225,12 @@ SHELL_DYNAMIC_CMD_CREATE(dsub_device_name, device_name_get); static void device_name_get(size_t idx, struct shell_static_entry *entry) { - int device_idx = 0; - struct device *dev; + struct device *dev = shell_device_lookup(idx, NULL); - entry->syntax = NULL; + entry->syntax = (dev != NULL) ? dev->name : NULL; entry->handler = NULL; entry->help = NULL; entry->subcmd = &dsub_device_name; - - for (dev = __device_start; dev != __device_end; dev++) { - if ((dev->driver_api != NULL) && - strcmp(dev->name, "") && (dev->name != NULL)) { - if (idx == device_idx) { - entry->syntax = dev->name; - break; - } - device_idx++; - } - } } SHELL_STATIC_SUBCMD_SET_CREATE(flash_cmds, diff --git a/drivers/i2c/i2c_shell.c b/drivers/i2c/i2c_shell.c index fea137b5d25..96d32137b51 100644 --- a/drivers/i2c/i2c_shell.c +++ b/drivers/i2c/i2c_shell.c @@ -224,25 +224,12 @@ SHELL_DYNAMIC_CMD_CREATE(dsub_device_name, device_name_get); static void device_name_get(size_t idx, struct shell_static_entry *entry) { - int device_idx = 0; - struct device *dev; + struct device *dev = shell_device_lookup(idx, I2C_DEVICE_PREFIX); - entry->syntax = NULL; + entry->syntax = (dev != NULL) ? dev->name : NULL; entry->handler = NULL; entry->help = NULL; entry->subcmd = &dsub_device_name; - - for (dev = __device_start; dev != __device_end; dev++) { - if ((dev->driver_api != NULL) && (dev->name != NULL) && - strstr(dev->name, I2C_DEVICE_PREFIX) != NULL && - strcmp(dev->name, "")) { - if (idx == device_idx) { - entry->syntax = dev->name; - break; - } - device_idx++; - } - } } SHELL_STATIC_SUBCMD_SET_CREATE(sub_i2c_cmds, diff --git a/drivers/sensor/sensor_shell.c b/drivers/sensor/sensor_shell.c index 24049d53ad5..4119c8cd156 100644 --- a/drivers/sensor/sensor_shell.c +++ b/drivers/sensor/sensor_shell.c @@ -173,24 +173,12 @@ SHELL_DYNAMIC_CMD_CREATE(dsub_device_name, device_name_get); static void device_name_get(size_t idx, struct shell_static_entry *entry) { - int device_idx = 0; - struct device *dev; + struct device *dev = shell_device_lookup(idx, NULL); - entry->syntax = NULL; + entry->syntax = (dev != NULL) ? dev->name : NULL; entry->handler = NULL; entry->help = NULL; entry->subcmd = &dsub_channel_name; - - for (dev = __device_start; dev != __device_end; dev++) { - if ((dev->driver_api != NULL) && - strcmp(dev->name, "") && (dev->name != NULL)) { - if (idx == device_idx) { - entry->syntax = dev->name; - break; - } - device_idx++; - } - } } SHELL_STATIC_SUBCMD_SET_CREATE(sub_sensor, diff --git a/include/shell/shell.h b/include/shell/shell.h index 2e138172e9a..99b54eeddd9 100644 --- a/include/shell/shell.h +++ b/include/shell/shell.h @@ -104,6 +104,24 @@ struct shell_static_args { uint8_t optional; /*!< Number of optional arguments. */ }; +/** + * @brief Get by index a device that matches . + * + * This can be used, for example, to identify I2C_1 as the second I2C + * device. + * + * Devices that failed to initialize or do not have a non-empty name + * are excluded from the candidates for a match. + * + * @param idx the device number starting from zero. + * + * @param prefix optional name prefix used to restrict candidate + * devices. Indexing is done relative to devices with names that + * start with this text. Pass null if no prefix match is required. + */ +struct device *shell_device_lookup(size_t idx, + const char *prefix); + /** * @brief Shell command handler prototype. * diff --git a/subsys/shell/shell_utils.c b/subsys/shell/shell_utils.c index 4cca3f4a7c9..245030cab47 100644 --- a/subsys/shell/shell_utils.c +++ b/subsys/shell/shell_utils.c @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ #include +#include #include "shell_utils.h" #include "shell_wildcard.h" @@ -459,3 +460,29 @@ void shell_cmd_trim(const struct shell *shell) buffer_trim(shell->ctx->cmd_buff, &shell->ctx->cmd_buff_len); shell->ctx->cmd_buff_pos = shell->ctx->cmd_buff_len; } + +struct device *shell_device_lookup(size_t idx, + const char *prefix) +{ + size_t match_idx = 0; + struct device *dev; + size_t len = z_device_get_all_static(&dev); + struct device *dev_end = dev + len; + + while (dev < dev_end) { + if ((dev->driver_api != NULL) + && (dev->name != NULL) + && (strlen(dev->name) != 0) + && ((prefix == NULL) + || (strncmp(prefix, dev->name, + strlen(prefix)) == 0))) { + if (match_idx == idx) { + return dev; + } + ++match_idx; + } + ++dev; + } + + return NULL; +}