linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
	Linus Walleij <linus.walleij@linaro.org>,
	Darrien <darrien@freenet.de>,
	Viresh Kumar <viresh.kumar@linaro.org>, Jiri Benc <jbenc@upir.cz>,
	Joel Savitz <joelsavitz@gmail.com>,
	linux-gpio@vger.kernel.org
Subject: Re: [libgpiod v2][PATCH 3/5] bindings: python: add examples for v2 API
Date: Tue, 7 Jun 2022 09:52:20 +0800	[thread overview]
Message-ID: <20220607015220.GA9430@sol> (raw)
In-Reply-To: <Yp3TmNg2uBlC0XzI@smile.fi.intel.com>

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.

Cheers,
Kent.

  reply	other threads:[~2022-06-07  1:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-25 14:06 [libgpiod v2][PATCH 0/5] bindings: implement python bindings for libgpiod v2 Bartosz Golaszewski
2022-05-25 14:07 ` [libgpiod v2][PATCH 1/5] bindings: python: remove old version Bartosz Golaszewski
2022-05-25 14:07 ` [libgpiod v2][PATCH 2/5] bindings: python: enum: add a piece of common code for using python's enums from C Bartosz Golaszewski
2022-05-25 14:07 ` [libgpiod v2][PATCH 3/5] bindings: python: add examples for v2 API Bartosz Golaszewski
2022-06-03 12:46   ` Kent Gibson
2022-06-04  2:41     ` Kent Gibson
2022-06-06 10:14       ` Andy Shevchenko
2022-06-07  1:52         ` Kent Gibson [this message]
2022-06-07 10:43           ` Andy Shevchenko
2022-06-08 15:39           ` Bartosz Golaszewski
2022-06-09  4:49             ` Kent Gibson
2022-06-09  8:42               ` Bartosz Golaszewski
2022-06-09 13:21                 ` Jiri Benc
2022-06-09 16:06                   ` Bartosz Golaszewski
2022-06-10  4:23                     ` Kent Gibson
2022-06-10  6:57                       ` Bartosz Golaszewski
2022-05-25 14:07 ` [libgpiod v2][PATCH 4/5] bindings: python: add tests " Bartosz Golaszewski
2022-05-25 14:07 ` [libgpiod v2][PATCH 5/5] bindings: python: add the implementation " Bartosz Golaszewski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220607015220.GA9430@sol \
    --to=warthog618@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=brgl@bgdev.pl \
    --cc=darrien@freenet.de \
    --cc=jbenc@upir.cz \
    --cc=joelsavitz@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).