linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Jack Winch <sunt.un.morcov@gmail.com>,
	Helmut Grohne <helmut.grohne@intenta.de>,
	Ben Hutchings <ben.hutchings@essensium.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>
Subject: Re: [libgpiod v2.0][PATCH] core: extend config objects
Date: Mon, 9 Aug 2021 07:10:12 +0800	[thread overview]
Message-ID: <20210808231012.GA6224@sol> (raw)
In-Reply-To: <CAMRc=McQOcWDexBeWKcA9CosxfG_59quusnVLYN7qu-p86BPag@mail.gmail.com>

On Sun, Aug 08, 2021 at 09:11:14PM +0200, Bartosz Golaszewski wrote:
> On Sat, Aug 7, 2021 at 10:48 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Fri, Aug 06, 2021 at 03:28:10PM +0200, Bartosz Golaszewski wrote:
> > > Kent suggested that we may want to add getters for the config objects in
> > > his reviews under the C++ patches. Indeed when working on Python bindings
> > > I noticed it would be useful for implementing __str__ and __repr__
> > > callbacks. In C++ too we could use them for overloading stream operators.
> > >
> > > This extends the config objects with getters. They are straightforward for
> > > the request config but for the line config, they allow to only read
> > > per-offset values that would be used if the object was used in a request
> > > at this moment. We also add getters for the output values: both taking
> > > the line offset as argument as well as ones that take the index and allow
> > > to iterate over all configured output values.
> > >
> > > The sanitization of input for the getters has subsequently been changed
> > > so that we never return invalid values. The input values are verified
> > > immediately and if an invalid value is passed, it's silently replaced
> > > by the default value for given setting.
> > >
> > > This patch also adds the reset function for the line config object - it
> > > can be used to reset all stored configuration if - for example - the
> > > config has become too complex.
> > >
> > > As this patch will be squashed into the big v2 patch anyway, I allowed
> > > myself to include some additional changes: some variable renames and
> > > other minor tweaks.
> > >
> > > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> >
> > A few minor nit-picks in the gpiod.h documentation below...
> >
> > Cheers,
> > Kent.
> >
> 
> Thanks,
> 
> With that fixed, do you think it's good to be applied?
> 

Sure.

I was also wondering if anything could be done to simplify the
structures in line-config.c, but that isn't specific to this patch.
Not having access to the offsets, or even num_lines, and doing the
allocation up-front makes it rather painful.  Especially if the most
common case is only one or two lines.
But, as long as you are happy with the external API, that is just
implementation detail that can be optimised later.

Cheers,
Kent.

  reply	other threads:[~2021-08-08 23:10 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-06 13:28 [libgpiod v2.0][PATCH] core: extend config objects Bartosz Golaszewski
2021-08-07  8:48 ` Kent Gibson
2021-08-08 19:11   ` Bartosz Golaszewski
2021-08-08 23:10     ` Kent Gibson [this message]
2021-08-10  7:52       ` Bartosz Golaszewski
2021-08-10 10:31         ` Kent Gibson
2021-08-11  1:16           ` Kent Gibson
2021-08-12  7:24           ` Bartosz Golaszewski
2021-08-12 10:29             ` Kent Gibson
2021-08-12 12:51               ` Bartosz Golaszewski
2021-08-12 13:03                 ` Andy Shevchenko
2021-08-12 14:02                   ` Bartosz Golaszewski
2021-08-12 13:52                 ` Kent Gibson
2021-08-12 14:01                   ` Bartosz Golaszewski
2021-08-12 14:23                 ` Kent Gibson
2021-08-12 14:43                   ` Bartosz Golaszewski
2021-08-12 15:02                     ` Kent Gibson
2021-08-13 12:59                       ` Bartosz Golaszewski
2021-08-13 13:03                         ` Kent Gibson

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=20210808231012.GA6224@sol \
    --to=warthog618@gmail.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=ben.hutchings@essensium.com \
    --cc=brgl@bgdev.pl \
    --cc=helmut.grohne@intenta.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=sunt.un.morcov@gmail.com \
    /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).