Re: [libgpiod v2][PATCH 3/5] bindings: python: add examples for v2 API

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Jun 7, 2022 at 3:52 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Mon, Jun 06, 2022 at 01:14:48PM +0300, Andy Shevchenko wrote:
> > On Sat, Jun 04, 2022 at 10:41:31AM +0800, Kent Gibson wrote:
> > > On Fri, Jun 03, 2022 at 08:46:00PM +0800, Kent Gibson wrote:
> > > > On Wed, May 25, 2022 at 04:07:02PM +0200, Bartosz Golaszewski wrote:
> >
> > ...
> >
> > > > The focus of my comments above is to simplify the API for the most common
> > > > case, and to make it a little more Pythonic rather than mirroring the C
> > > > API, in both cases by hiding implementation details that the casual user
> > > > doesn't need to know about.
> > > >
> > >
> > > Further to this, and recalling our discussions on tool changes, it would
> > > be great if the Python API supported identification of line by name, not
> > > just (chip,offset).
> > >
> > > e.g.
> > >     with gpiod.request_lines(
> > >         lines=("GPIO17", "GPIO18"),
> > >         edge_detection=Edge.BOTH,
> > >     ) as request:
> > >         for event in request.edge_events():
> > >             print(event)
> > >
> > > with the returned event extended to contain the line name if the line
> > > was identified by name in request_lines().
> > >
> > > The lines kwarg replaces offsets, and could contain names (strings) or
> > > offsets (integers), or a combination.  If any offsets are present then
> > > the chip path kwarg must also be provided.  If the chip isn't provided,
> > > request_lines() would find the corresponding chip based on the line name.
> >
> > From Python programmer perspective it's a good idea, but from GPIO (ABI)
> > perspective, it may be confusing. Line name is not unique (globally) and
> > basically not a part of ABI.
> >
>
> "basically not a part of the ABI"???
> Damn - we should've removed it from the line info for uAPI v2 ;-).
>
> A common request from users is to be able to request lines by name.
> Of the libgpiod bindings, Python is the best suited to allow that
> possibility directly as part of its core API.
> It also happens to be the one most likely to be used by said users.
>
> While identifying line by name can't be guaranteed to work universally,
> that doesn't mean that we should automatically exclude the possibility.
> It is possible with the current ABI - it is awkward, but possible.
> In libgpiod v1, gpiod_ctxless_find_line(), gpiod_chip_find_line() et al.,
> and in v2 gpiod_chip_get_line_offset_from_name(), do just that -
> I'm merely suggesting that similar functionality be incorporated into
> request_lines().
>
> Line names should be unique in well configured systems, even if the
> kernel itself does not guarantee it.
> The binding would perform an exhaustive search to ensure the requested
> line name is unique, and throw if not (unlike the libgpiod v1 functions
> that return the first match - yikes).
> (We could always extend the GPIO uAPI to make the mapping process less
> painful, e.g. an ioctl to perform the name to offset mapping, including
> uniqueness check, for a chip.)
> For applications targetting systems that don't guarantee uniqueness, the
> (chip,offset) approach remains available.
> And if the line names are thought to be unique within a chip, the middle
> ground of (chip,name) is also available.
>
> Wrt confusion, the alternative would be to provide a separate name based
> API wrapper, or insist that the user jump through the name mapping hoops
> themselves prior to calling the offset based API.
> Are either of those less confusing?
>
> But if the purpose of the Python binding is purely to minimally wrap the
> C ABI, warts and all, then my suggestion should most certainly be ignored.
>

I actually have a third alternative. I would like the gpiod module to
only expose the C API functionality but how about a gpiod_extended or
something similar with all kinds of python helpers? Python users are
indeed used to modules making the work easier and I'm not against it
but writing it in C would be a PITA so I'm thinking about a secondary
pure python module with those kinds of extensions.

Bart