Re: [libgpiod][RFC 0/6] first draft of libgpiod v2.0 API

[Kent Gibson <warthog618@gmail.com>]

On Sun, Apr 18, 2021 at 11:12:24PM +0200, Bartosz Golaszewski wrote:
> On Sun, Apr 18, 2021 at 5:48 AM Kent Gibson <warthog618@gmail.com> wrote:
> >

[snip!]

> >
> > I forgot to add that wrt the config mutators, you need to allow
> > overriding of existing config, rather than returning an error on
> > conflict, so that you can change config for the set_config ioctl().
> > Hence the last-in-wins approach.  And as a consequence the mutator is
> > always right and so needs no return code.
> >
> 
> This sounds good in theory but how do we handle a situation that
> requires more than 10 attributes? Override the first one? The last
> one? What if the line offsets passed to the request config repeat
> themselves? I think some sanitization of input is in order.
> 

Repeating of lines is equivalent to repeatedly setting a bit, so the
subsequent instances are ignored. In practice I don't even need to check
- if the user includes the line multiple times then it gets set multiple
times - to the same thing.

The case where a complex config can't be mapped to the uAPI, e.g. due to
too many attributes on too many lines, is handled at the time of the
request_lines() or set_config() itself when that mapping is performed.
Those will return an "overly complex config" error.

> Regarding offsets: I was thinking about how to approach referring to
> lines in configs and requests by offsets only (in order to hide the
> whole masking logic) and while for a request (for example: when
> setting/reading line) this is straightforward (as long as we make sure
> the offsets are never duplicated), the line config structure doesn't
> really know the concept of offsets. So when we set a config option for
> a specific line, we need to carry the offset information somehow in
> the structure until the request is actually made. How do you deal with
> this in your library? Did you expose any of the bitmap details in your
> API? Can we really avoid dealing with indexing of lines in a request?
> 

In the request config I use a map of offset to line config to avoid 
duplication. A config change that alters any existing setting just
overwrites the old.

The line config is similar to your struct gpiod_line_config.
The line config for a particular line is only created and added to the
map if there is a config change specific to that line.
Each attribute has a "not set" value, in which case the request-wide
default is used.

The request-wide default config is stored separately from the map.
And there is a function to reset a line config back to the default,
i.e. drop that line config from the map.

The request_lines() and set_config(), that accept the config, also have
the list of offsets available (provided to the request_lines() and
subsequently stored as part of the request struct for the set_config())
and so can map from offsets to indices to build the bitmap.
The bitmap and indices themselves are never exposed.

That is a high level description - the details are actually a little
different as the Go implementation uses functional options, so the
initial config settings become parameters to the request, and bundles
the config into the request object itself.

> > And you might want to add a copy() for config to allow the user to
> > easily create two slightly different configurations.
> >
> > > I was on the fence wrt reference counting but then realized that in
> > > C++ or Python we still need to provide a mechanism for unconditional
> > > closing of chips and releasing of requests. For the former it's
> > > because otherwise we'd need to make the object go out of scope
> > > manually (probably by storing it in another object that would be
> > > "closed" -> pointless abstraction) and in the latter case: Python
> > > doesn't even guarantee that the destructor will be called at any
> > > specific point.
> > >
> >
> > Hmmm, ok, I was assuming the C++ bindings would wrap the C objects in C++
> > objects, and the C++ destructor would release any associated resources.
> >
> 
> Yes, but what if the user wants to close the chip or release the
> request without the underlying object going out of scope? I think we
> need to keep that possibility.
> 

Then you also provide a close() method.  They aren't mutually exclusive.

Cheers,
Kent.