Re: [libgpiod PATCH] gpioget: Add --line-name to lookup GPIO line

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Fri, Dec 3, 2021 at 7:20 AM Andrew Jeffery <andrew@xxxxxxxx> wrote:
>
>
>
> On Fri, 3 Dec 2021, at 14:20, Zev Weiss wrote:
> > On Wed, Dec 01, 2021 at 08:29:47PM PST, Joel Stanley wrote:
> >>On Wed, 1 Dec 2021 at 08:29, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> >>>
> >>> On Wed, Dec 1, 2021 at 8:29 AM Joel Stanley <joel@xxxxxxxxx> wrote:
> >>> >
> >>> > Systems provide line names to make using GPIOs easier for userspace. Use
> >>> > this feature to make the tools user friendly by adding the ability to
> >>> > show the state of a named line.
> >>> >
> >>> >  $ gpioget --line-name power-chassis-good
> >>> >  1
> >>> >
> >>> >  $ gpioget -L pcieslot-power
> >>> >  0
> >>
> >>> I'm not very convinced to be honest. It's not like "gpioget gpiochip0
> >>> `gpiofind gpiochip0 line-name`" requires much more typing than
> >>> "gpioget gpiochip --line-name=name".
> >>
> >>I'm taking on feedback from people working in our labs, and
> >>implementing userspace applications. We've been building BMCs with
> >>mainline Linux for about six years now, and it's been a long road
> >>re-training them from "back in the day we just did devmem <this>
> >><that>" and "why can't we just do cat /sys/class/gpio/gpio305/value",
> >>and now "why does the level of the GPIO change back after I run the
> >>command?".
> >>
> >>This usability improvement is one more step towards them using and
> >>being happy with the "new world" of the gpiod API.
> >>
> >>Once we settle on a good API here, I plan on submitting a version of
> >>gpioget/gpioset added to busybox.
> >>
> >>> There are also other questions:
> >>> this uses getopt and only allows to specify a single line name. What
> >>> if we want to specify more lines like with offsets? Even if you allow
> >>> multiple names, getopt() doesn't guarantee ordering of arguments.
> >>
> >>If you're happy with the concept I'm happy to iterate on the implementation.
> >>

Alright, it looks like this has passed by popular vote.

> >>Yes, it only allows a single line name. That tends to be how the tool
> >>is used, both from the command line and in scripts.
> >>
> >>Can you give me an example of your proposed command line API, so I can
> >>understand what you're suggesting here?
> >>
> >
> > My two cents: like Jeremy, I would very much welcome the ability to
> > specify GPIOs by name instead of number, but the one-line-only
> > limitation does seem unfortunate.  How about making a command-line flag
> > that just means "line-specifier arguments should be interpreted as names
> > instead of numbers"?
> >
> > So you could do:
> >
> >   $ gpioget --by-name chassis-intrusion cpu1-prochot
> >   0 1
> >
> >   $ gpioset --by-name led-green=1 led-red=0

I like this more - that way we either allow offsets or names. Please
make sure corner cases are covered. If you can add this to other tools
too, I'm fine with that, but put the name lookup code into
tools-common.c please. I would also like to see bats test cases
covering this.

Thanks!
Bart

> >
>
> I came up with this approach as well (independently, just thinking
> about Joel's patch). I think it has good ergonomics. I hadn't figured
> out how we should interpret the arguments as line index vs line name,
> but your --by-name option solves that. I like it a lot.
>
> Andrew