-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sys/shell_commands: convert to SHELL_COMMAND() #18152
Conversation
If you wonder why I dropped the abuse of the Also, it is IMO super confusing to see shell command output without the shell. I assumed the application to be crashed when I was unable to invoke shell commands via serial until I realized that no shell is actually running. |
How did this ever work? RIOT/sys/shell/commands/shell_commands.c Lines 109 to 112 in 6e41c68
RIOT/sys/shell/commands/sc_openwsn.c Lines 96 to 99 in 6e41c68
|
e3149fa
to
f4630a4
Compare
No idea 😕, sorry about that |
f4630a4
to
818ac85
Compare
818ac85
to
6db5836
Compare
The shell command `id` was dropped in b3a061e but somehow the command entry was left it. This cleans it up.
Make use of XFA for shell commands
6db5836
to
97f2033
Compare
rebased to solve merge conflict |
All green now 🎉 |
Thanks for the review! |
To fix warnings breaking compilation using GCC 12. Remove `#include "shell_comands.h"` as in RIOT-OS/RIOT#18152 Signed-off-by: Eric Courtois <[email protected]>
Contribution description
This drops the manually managed "shell command registry" and uses the XFA based
SHELL_COMMAND()
mechanism for registration instead.Open ToDos
Adding a
static
in front of the handler function should now be possible in most cases, except when the command handler is actually used externally. I'd rather do this in a follow up and get his in as quickly and smoothly as possible to avoid merge conflicts.Testing procedure
Except for alphabetical ordering, the shell command should still all registered as before.
Issues/PRs references
Same as #16095 but already moved the
SHELL_COMMAND()
registration next to the implementation.