Skip to content
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

Shell overdo argument parsing in some cases #23907

Closed
nordic-krch opened this issue Mar 30, 2020 · 3 comments · Fixed by #24329
Closed

Shell overdo argument parsing in some cases #23907

nordic-krch opened this issue Mar 30, 2020 · 3 comments · Fixed by #24329
Assignees
Labels
area: Shell Shell subsystem Enhancement Changes/Updates/Additions to existing features

Comments

@nordic-krch
Copy link
Contributor

Describe the scenario
If shell is used to implement AT commands, some commands are parsed incorrectly. It is because shell argument parser attempts to extract all parameters into separate strings and support quotation characters so it parses string like command arg1 "arg2 with spaces" into 3 arguments with last one containing spaces. Quotations must be escaped if they are just quotations (e.g. command arg1\"ABC\"). For AT case it would actually be more convenient if shell passes parameters as single string to the command handler.

Example (assuming that at is a registered command with handler):
command: at AT+ABC="some string","FOO", "foo"

Now is parsed as:
argv[0]=at
argv[1]=AT+ABC=some string,FOO,
argv[2]=foo

Expected:
argv[0]=at
argv[1]=AT+ABC="some string","FOO", "foo"

Proposed solution
Add flags to command registration and flag that will tell the parser to treat all characters following n'th argument as the last argument.
Add flag SHELL_CMD_MERGE_ARGS(offset). Offset written on 4 bits should be more than enough.
Extend registration command to allow passing flags.

@nordic-krch nordic-krch added Enhancement Changes/Updates/Additions to existing features area: Shell Shell subsystem labels Mar 30, 2020
@nordic-krch nordic-krch self-assigned this Mar 30, 2020
@nordic-krch
Copy link
Contributor Author

fyi @stig-bjorlykke

@nordic-krch
Copy link
Contributor Author

@jakub-uC @Vudentz what do you think about it? feedback welcomed. Other solution proposals or maybe different perspective to the problem.

@jakub-uC
Copy link
Contributor

jakub-uC commented Mar 30, 2020

@nordic-krch : The easiest way, in my opinion, would be to implement a command like select which you can use to dynamically modify parameter set by Kconfig's SHELL_ARGC_MAX.
And similar to select you could use some escape code to bring back default value.

By executing command:

set_argc_max 1  // new command to be implemented

Shell command at AT+ABC="some string","FOO", "foo" will be interpreted as expected:

argv[0]=at
argv[1]=AT+ABC="some string","FOO", "foo"

btw.
Your proposal also sounds reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Shell Shell subsystem Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants