Re: [libgpiod][RFC] helper functions for basic use cases

[Kent Gibson <warthog618@xxxxxxxxx>]

On Mon, May 13, 2024 at 01:28:19AM -0700, Bartosz Golaszewski wrote:
> On Sat, 11 May 2024 03:11:44 +0200, Kent Gibson <warthog618@xxxxxxxxx> said:
> > On Fri, May 10, 2024 at 08:37:08PM +0200, Bartosz Golaszewski wrote:
> >> On Tue, May 7, 2024 at 4:21 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >> >
> >
> > I understand - slippery slope and keeping minimal.
> > But I think we do need some solution for simple one line request cases that
> > can get users up and running with a smaller learning curve.
> > Then they can learn more if they want more.
> >
>
> I don't disagree on this point but I still think core libgpiod is not the right
> place for it.
>
> >> Do you think that users actually use it (core C libgpiod)?
> >
> > Yeah, they do, believe it or not.  I'm mainly talking the vocal Pi crowd,
> > many of whom are used to going bare metal to the hardware, and are now
> > searching for an alternative.  libgpiod as it stands is too complicated for
> > them - they just want to drive a pin up and down or read a button.
> > There are a few other alternatives that let them do that, and they will
> > switch on that basis alone, and never look back, though they will happily
> > spread their rather toxic opinions of libgpiod vs those alternatives.
> >
> >> I would think they stick to python and C++?
> >
> > They are actually less likely to go C++ - more learning curve.
> > And Python can be considered too heavyweight in situations with limited
> > resources.
> >
>
> I see. We're being held hostage by the RPi crowd. :)
>

You could look at it that way.

> > /**
> >  * @brief Set the bias of requested input line.
> >  * @param olr The request to reconfigure.
> >  * @param bias The new bias to apply to requested input line.
> >  * @return 0 on success, -1 on failure.
> >  */
> > int gpiod_olr_set_bias(struct gpiod_line_request * olr,
> > 		       enum gpiod_line_bias bias);
>
> For this to work, you'd most likely need a new struct wrapping the request
> and also storing the line config. Otherwise - how'd you keep the state of all
> the other line settings?
>

Yeah, I realised that when I went to implement it :(.

What I implemented was to read the line info and build the config from that.
So no impact on core.
Not the most efficient, but for this use case I wan't fussed.

> >
> > /**
> >  * @brief Set the debounce period of requested input line.
> >  * @param olr The request to reconfigure.
> >  * @param period The new debounce period to apply, in microseconds.
> >  * @return 0 on success, -1 on failure.
> >  */
> > int gpiod_olr_set_debounce_period_us(struct gpiod_line_request *olr,
> > 				     unsigned long period);
> >
> > /**
> >  * @brief Set the edge detection of requested input line.
> >  * @param olr The request to reconfigure.
> >  * @param edge The new edge detection setting.
> >  * @return 0 on success, -1 on failure.
> >  */
> > int gpiod_olr_set_edge_detection(struct gpiod_line_request * olr,
> > 				 enum gpiod_line_edge edge);
> >
> > /**
> >  * @}
> >  */
> >
> > I think those 5 functions cover the basics.  That is it.  Done. No more.
> > Unless you can think of something I've missed.
> >
>
> I'm afraid it never ends up being enough. We'd just open the door for all kinds
> of extensions to libgpiod. Very soon someone would want a callback-based API
> for reading edge events. Next you'll have people asking to be able to toggle
> the direction of a pin without touching the config structures which will require
> us to somehow store the context of the request. "That is it" never works.
>

At the end of the day you are the arbiter of where that line is drawn, so
that is up to you.

>
> In most cases bindings do allow you to do what you try to achieve here with
> one-liners already. I think this really only applies to C.
>

Oh, you would be surprised - the C++ "one-liners" are still considered
too verbose and difficult to understand.

> > Anyway, have a think about it.
> > And I'll go take a look at the GLib bindings.
> >
>
> I have thought about it. I agree we could use some simpler interfaces. I don't
> agree their place is in core libgpiod. I understand we want to make this new
> interface seamless to use for end-users of libgpiod.
>
> How about introducing a new header and a separate shared object: gpiod-ext.h
> and libgpiod-ext.so respectively? We could put all these new helpers in there,
> use the gpiod_ext_ prefix for all of them and distros could package the
> "extended" part of libgpiod together with the core library to avoid having to
> install multiple packages?
>

That sounds good to me.

> We'd keep the clear distinction between the low-level, core C library wrapping
> the kernel uAPI and the user-friendly C API. Though the user-friendly API in my
> book could be the GLib library but I understand not everyone wants to use GLib
> nor is familiar with GObject.
>

Sorry, still haven't had a chance to look at the GLib API.
But if it involves any additional overhead or learning curve then it
wont be well received.

Cheers,
Kent.