shell: utils: Improve utility by addding non-ready devices functions #52
Conversation
| if (!device_is_ready(dev)) { | ||
| shell_info(sh, "Device %s is already initialized", argv[1]); |
There was a problem hiding this comment.
I think the logic is flipped here. You want if (device_is_ready(dev)) without negating it
There was a problem hiding this comment.
good catch, I did not test this module (as I used the beo-shell-device one intstead in the end)
subsys/shell/shell_utils.c
Outdated
| const char *prefix, | ||
| shell_device_filter_t filter) | ||
| shell_device_filter_t filter, | ||
| bool ready) |
There was a problem hiding this comment.
| bool ready) | |
| bool show_all) |
If you flip the logically intention here, I think the conditional later on simplifies.
Alternatively keep the current logic, but then I suggest renaming to something like only_ready to show the intended use a bit more from the get-go.
There was a problem hiding this comment.
I first agreed on this, and then I remembered that I wanted a way to auto-complete on non-ready devices... which means I miss a function to do so like shell_device_lookup_non_ready() of some sort.
So looks like I forgot that feature at some point (which also means the logic I made is wrong anyway...)
There was a problem hiding this comment.
Actually what I did is pretty stupid...
subsys/shell/shell_utils.c
Outdated
|
|
||
| while (dev < dev_end) { | ||
| if (device_is_ready(dev) | ||
| if (((ready && device_is_ready(dev)) || !ready) |
There was a problem hiding this comment.
If you did flip the logic of ready to be a show_all flag:
| if (((ready && device_is_ready(dev)) || !ready) | |
| if ((show_all || device_is_ready(dev)) |
| const struct device *dev_end = dev + len; | ||
|
|
||
| while (dev < dev_end) { | ||
| if ((dev->name != NULL) && |
There was a problem hiding this comment.
| if ((dev->name != NULL) && | |
| if (dev->name && |
Isn't this the common approach in zephyr as well when checking non-null?
There was a problem hiding this comment.
I c/p shell_device_internal().
In linux it is the way yes (like testing any integer if it has a value or 0) but in zephyr... I thought there might be some MISRA rule for this, but looking at https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html I am not so sure anymore (could not find anything related to this)
subsys/shell/shell_utils.c
Outdated
| size_t len = z_device_get_all_static(&dev); | ||
| const struct device *dev_end = dev + len; | ||
|
|
||
| while (dev < dev_end) { |
There was a problem hiding this comment.
Alternatively you can substitute a for-loop to get the ++dev right here :)
tbursztyka
force-pushed
the
tobu/improve_shell_utils_on_devices
branch
2 times, most recently
from
ef5b993 to
accdb4d
Compare
This are necessary changes to handle non-initialized devices. I.e. devices which 'zephyr,deferred-init' DTS attribute was set. Later on, such utility will prove to be useful on improving devices shell module. Signed-off-by: Tomasz Bursztyka <[email protected]>
tbursztyka
force-pushed
the
tobu/improve_shell_utils_on_devices
branch
from
accdb4d to
572f3de
Compare
This are necessary changes to handle non-initialized devices. I.e. devices which 'zephyr,deferred-init' DTS attribute was set.
Later on, such utility will prove to be useful on improving devices shell module.