Re: [libgpiod v2][PATCH v2 5/5] bindings: python: add the implementation for v2 API

[Kent Gibson <warthog618@xxxxxxxxx>]

On Fri, Jul 08, 2022 at 11:49:59AM +0200, Bartosz Golaszewski wrote:
> On Fri, Jul 8, 2022 at 3:38 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> > On Thu, Jul 07, 2022 at 10:09:44PM +0200, Bartosz Golaszewski wrote:
> > > On Thu, Jul 7, 2022 at 3:10 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > > >
> > > > On Thu, Jul 07, 2022 at 02:19:17PM +0200, Bartosz Golaszewski wrote:
> > > > > On Tue, Jul 5, 2022 at 4:09 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > >
> > > [snip]
> > >
> > > > > >
> > > > > > How about merging the _default and _offset forms by adding an offsets
> > > > > > kwarg?
> > > > > > offsets=None (or unspecified) -> default
> > > > > > offsets=int -> offset
> > > > > > offsets=iterable -> offsets
> > > > > >
> > > > > > Off on a bit of a tangent... why should the end user care about
> > > > > > defaults and overrides?
> > > > > > For a higher level abstraction I'd prefer to see the whole "default"
> > > > > > concept disappear in favour of the config for each line.  That would
> > > > > > remove a lot of the complexity from the LineConfig interface.
> > > > > > Though it would add complexity to the binding internals.
> > > > > >
> > > > >
> > > > > What would that look like (in python code) if I wanted to request 5
> > > > > lines and use the same config for them?
> > > > >
> > > >
> > > > That is the trivial case - you use the module level
> > > > gpiod.request_lines() as is and pass in the config parameters and list of
> > > > lines you want.
> > > >
> > > > req = gpiod.request_lines(chip="gpiochip0", offsets=[1,2,3,4,5],
> > > >                           direction="output", values=[1,0,1,0,0])
> > > >
> > >
> > > This is close to what I have now in my v3 branch. Except that values
> > > is called output_values and takes a dictionary like its counterpart in
> > > LineConfig but that can be extended to interpreting a list as
> > > providing the values for corresponding offsets/lines. Current version
> > > of request_lines() takes all LineConfig options and uses them as the
> > > defaults.
> > >
> > > > The more complicated case is where the lines config differs.
> > > > Then you have to build the LineConfig by adding the config for each set
> > > > of lines in a separate call to set_props().
> > > > Then you provide that LineConfig to the request_lines(), along with the
> > > > set of lines.
> > > >
> > > > lc.set_props(offsets=[1,2,3], direction="input")
> > > > lc.set_props(offsets=[4,5], direction="output", values=[1,0])
> > > > req = gpiod.request_lines(chip="gpiochip0", line_cfg=lc)
> > > >
> > > > (simplified examples using stringified prop values etc - hope you get
> > > > the idea)
> > > >
> > > > Building that on top of the C API, you would determine the "default"
> > > > config based on the most common attribute values, then override the
> > > > config for the lines that differ from that default.
> > > > That is the internal complexity I mentioned.
> > > >
> > >
> > > Internal complexity is fine - it's the implicitness of the defaults
> > > that make me not like this idea. I think we discussed something
> > > similar for the C API and I was against it too. Your examples are fine
> > > but the defaults for lines not mentioned in set_props() would be
> > > filled by a freshly created LineConfig with its well defined default
> > > values. In other words I prefer to keep the override mechanism visible
> > > in python but unification of the property setters is something I will
> > > consider.
> > >
> >
> > I suspect you are right that we've been here before and I'm flogging a
> > dead horse, but you get that - I must think there is still a bit of life
> > in the old nag ;-).
> >
> > I find it ironic that a feature of the uAPI that is there due to
> > the constraints on the uAPI, i.e. to keep the line_config to a
> > manageable size, gets propagated this highly.  In my mind the
> > configuration for each line has always been distinct, and the uAPI
> > line_config is just a reduced form.
> >
> 
> The limitation of the uAPI is what keeps us from making it true in
> user-space (that each line can have its own config). As it is, only up
> to 9-10 lines can have distinct configs and making the API look and
> behave as if it wasn't the case is more confusing (E2BIG errors) than
> simply admitting we have the concept of defaults and overrides (to
> which the interface is greatly simplified in the high-level
> libraries). The idea about making the most common config attributes
> become the defaults is simply bad. It would require the user to
> anticipate how the library will behave for every attribute and lead to

It requires nothing from the user.  They are not even aware of the
concept of "defaults" or "overrides".  They just set config on lines.
If that is too complicated, which is quite unlikely, then they get
E2BIG and they need to repartition their lines into multiple requests.

Anyway, that horse is dead.

Cheers,
Kent.