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

Enhancement request for console component help feature (IDFGH-7603) #9158

Closed
dalbert2 opened this issue Jun 14, 2022 · 7 comments
Closed

Enhancement request for console component help feature (IDFGH-7603) #9158

dalbert2 opened this issue Jun 14, 2022 · 7 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally

Comments

@dalbert2
Copy link

I have always implemented a command line interface for my projects that's similar to the console component. I'm trying to switch to IDF standard components whenever possible. but an obstacle with the console component is the implementation of the help feature which generates excessive output once the number of commands grows larger than a trivial example.

For my own component, I implement help as a two-tier function:

  1. the help command alone lists all commands, separated by commas and at the end suggests
    "use help for more details".

  2. help displays the detailed help for the specified command

This way a system with more than a few commands can generate reasonable amounts of output for any particular help request.

I'd be happy to modify the component code to do this if the maintainers agree it is desirable and permit me to do so. Otherwise, please consider this as an enhancement request.

@espressif-bot espressif-bot added the Status: Opened Issue is new label Jun 14, 2022
@github-actions github-actions bot changed the title Enhancement request for console component help feature Enhancement request for console component help feature (IDFGH-7603) Jun 14, 2022
@dalbert2
Copy link
Author

Another easy feature I have in my console implementation is an optional password. If passwordP is not null, a boolean authenticated is set false and the console task prompts with a "password>" prompt. When the user enters the configured password, authenticated is set true and normal console operation proceeds. Each failed password attempt bumps login_failure_cnt and whenever login_failure_cnt & 0x03 == 0 (every 4 failures), a brief vTaskDelay() prevents brute force attacks. An exit command sets the authenticated variable false again.

This is easy to implement and provides a minimum level of security on console interfaces.

@igrr
Copy link
Member

igrr commented Jun 15, 2022

Hi @dalbert2, the first suggestion (brief help and more verbose help <cmd>) makes sense, please feel free to open a PR for this.

For the 2nd option, I think this doesn't require changes to the console component itself. You can start the console with only one command registered, e.g. login. This command, implemented in your application, can verify the password. Only when the password is verified, your application registers the remaining commands in the console component. Here is the change you can apply to the "basic" console example in IDF to implement this.
0001-add-an-example-of-login-command.patch.txt

@dalbert2
Copy link
Author

dalbert2 commented Jun 15, 2022

Thanks @igrr, I mentioned the second enhancement (minimum password security) because it is so easy to implement and makes a significant improvement to the component. Virtually all CLI consoles on commercial devices have at least a minimum level of security. The workaround you propose is interesting, but there appears to be no way to un-register commands, so you could log in that way, but you would have to reset the unit to log out.

The enhancement proposed is both easy to implement and adds important functionality. I'm new to IDF so it will be my first PR; I'm happy to make these changes and submit for review; unless you object, I'd like to open PRs for both.

@igrr
Copy link
Member

igrr commented Jun 15, 2022

The workaround you propose is interesting, but there appears to be no way to un-register commands, so you could log in that way, but you would have to reset the unit to log out.

It is possible to un-initalize the console and re-initialize it again, as a lighter weight alternative to resetting the device.

We can also accept a PR for un-registering commands, this sounds like a reasonable request!

Typically we don't add additional functionality to core components, which

  1. can be easily implemented in the application, or
  2. different applications might require slightly different implementations of this functionality, or
  3. isn't required for common use cases or applications.

The request for a feature to log into the console hasn't been seen on the IDF feature tracker before. It is something that can be easily implemented in the application (just about 20 lines of code, as shown above). Different users may require different ways the passwords are validated. For these reasons I don't think we will accept a PR for this feature. Of course, if more developers chime in and say that they would like to see such functionality in IDF, we can reconsider this.

I'm sorry if my reply sounds discouraging. For your first request (help command) we definitely welcome a PR.

@igrr
Copy link
Member

igrr commented Jun 15, 2022

Here is the updated example which demonstrates logging out by de-initializing the console REPL.

0001-add-an-example-of-login-command.patch.txt

@KaeLL
Copy link
Contributor

KaeLL commented Jun 15, 2022

What about as an example?

@espressif-bot espressif-bot added Status: Selected for Development Issue is selected for development and removed Status: Opened Issue is new labels Apr 18, 2024
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Selected for Development Issue is selected for development labels May 17, 2024
@louielauanu
Copy link
Collaborator

Hi @dalbert2,

We develop a feature that enable users to decide the help command verbose level, and that will be available soon.

@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress labels May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

No branches or pull requests

5 participants