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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Thu, Jun 9, 2022 at 6:49 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Wed, Jun 08, 2022 at 05:39:16PM +0200, Bartosz Golaszewski wrote:
> > 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.
> >
>
> Agree that it would be easier to write a pythonic wrapper around the C
> API in Python, so no problem with that.
> However, the pythonic wrapper should the one named gpiod, as it is
> intended to be the primary interface for Python.  Rename your existing
> to gpiod_c or gpiod_core or something.
>

I don't agree. The module that wraps the C library should still be
called gpiod and be the primary interface. The pythonic module would
just offer helpers that would still use the gpiod data types for most
part.

Bart

> Btw, I've only mentioned a small part of the API so far, but the same
> applies to the remainder. e.g. the RequestConfig and LineConfig could use
> the lines kwarg treatment as well. Though I suspect implementing that will
> be a bit of a bear, in either language.
>