drivers: gpio: shell: Improved error reporting

[aasinclair]

Invalid device now generates message and errno return
Arguments checked with shell_strtoul
gpio function call return values checked and passed on

Device name completion and usage text added

SHELL_GPIO enabled when SHELL and GPIO both enabled

Configuration command now accepts integer argument as well and named modes

[henrikbrixandersen]

Looks good, thanks! One minor nit:

[WalkingTalkingPotato]

I think it would be better for readability to make local variables for arguments instead of using arbitrary array elements directly, something like this:

`char *port = argv[1];

...

dev = device_get_binding(port);`

[carlescufi]

Why?

[aasinclair]

This way you get GPIO shell automatically if you have GPIO and SHELL enabled.
Otherwise you have to enable it explicitly.

Most of the other shell implementations have done it this way.

[nixward]

Hi @aasinclair, apologies up front from me, I forgot to check if anyone else had been working on this shell and ended up recently putting together a PR for it also see PR 63019. The lack of tab suggestion and completion functionality was the bad user experience I'd recently had which was the trigger for me to try to find a solution. In the PR I used the dynamic subcommand and the dictionary subcommand APIs to enable tab suggestion/complete feature for all gpio_shell subcommands. The benefit of re-structuring the gpio_shell to utilise the dynamic subcommand API for device name and pin sub commands, and the dictionary subcommand API for pin config options and pin set values is that if the user manually enters something not in the subcommand set the shell will automatically reject it and will print out the valid subcommand options available which removes the need to write error checking code for these subcommands as it's already built into these subcommand APIs.

In addition I can now see there's a way forward with this tab suggestion/completion strategy to use the Devicetree to limit the device suggestions to only gpio devices similar to the adc_shell (link) and to use the Devicetree to limit the pin subcommands to only those indicated as available by these gpio bindings properties.

So in summary I think tab suggestion/completion functionality in the gpio_shell would be a good base line for it to have so I implore you and others to have a look at PR 63019 as an alternate way forward. If there's consensus PR 63019 is a desirable way forward I'm of course happy to modify with ideas from this PR or if alternatively this PR is deemed the preferred way feel free to pull in any ideas from PR 63019. Thanks.

[aasinclair]

Hi @aasinclair, apologies up front from me, I forgot to check if anyone else had been working on this shell and ended up recently putting together a PR for it also see PR 63019. The lack of tab suggestion and completion functionality was the bad user experience I'd recently had which was the trigger for me to try to find a solution. In the PR I used the dynamic subcommand and the dictionary subcommand APIs to enable tab suggestion/complete feature for all gpio_shell subcommands. The benefit of re-structuring the gpio_shell to utilise the dynamic subcommand API for device name and pin sub commands, and the dictionary subcommand API for pin config options and pin set values is that if the user manually enters something not in the subcommand set the shell will automatically reject it and will print out the valid subcommand options available which removes the need to write error checking code for these subcommands as it's already built into these subcommand APIs.

In addition I can now see there's a way forward with this tab suggestion/completion strategy to use the Devicetree to limit the device suggestions to only gpio devices similar to the adc_shell (link) and to use the Devicetree to limit the pin subcommands to only those indicated as available by these gpio bindings properties.

So in summary I think tab suggestion/completion functionality in the gpio_shell would be a good base line for it to have so I implore you and others to have a look at PR 63019 as an alternate way forward. If there's consensus PR 63019 is a desirable way forward I'm of course happy to modify with ideas from this PR or if alternatively this PR is deemed the preferred way feel free to pull in any ideas from PR 63019. Thanks.

Hi @nixward.
I've just tested your PR and the autocompletion is great, so I think it would be prudent to take your PR as a starting point.
The only features I would really like to see from this PR are:

• Add default y to the Kconfig, so you don't have to manually specify CONFIG_GPIO_SHELL (Just CONFIG_SHELL is enough).
• Add the option for custom configuration of the pin (in addition to in, inu, ind, out).

[WalkingTalkingPotato]

In theory it is possible that an API call will return an error even if all the arguments were correct. Example - if we're controlling a GPIO extender, connected through an external bus (say SPI or I2C), call to it could fail if the bus connection got disrupted. So, I disagree that autocompletion "removes the need to write error checking code".

[nixward]

@WalkingTalkingPotato yes I agree, sorry I should have been more specific, it removes the need for error handling of some incorrect input, i.e. the gpio_pin_set() value will always be 0 or 1 as that is all the subcommand will allow otherwise the dictionary subcommand API will inform the user they need to select 0 or 1. But yes if gpio_pin_set() returns an error it should be reported via the shell so I'll double check that is handled. Please feel free to point out anything I've missed.