Re: [libgpiod 1.6.3] Do different lines have to be in the same scope?

[Kent Gibson <warthog618@xxxxxxxxx>]

On Fri, Feb 18, 2022 at 07:33:16PM +0100, Bartosz Golaszewski wrote:
> On Tue, Feb 15, 2022 at 5:11 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> > On Mon, Feb 14, 2022 at 02:01:29PM +0000, Hummrich, Tobias wrote:
> > > Hi,
> > >
> > > last week I ported part of our gpio related code from sysfs to libgpiod. I use the C++ bindings. I had some problems polling two different lines on different gpio chips and finally realized that all was OK if both lines were defined in the same scope. Out of curiosity I'm asking: Is that really the case in version 1.6.3 and was this intended?
> > >
> >
> > No.  I'm guessing you are doing something wrong, but I'd have to see
> > real code to be sure.  Code > 1000 words.
> >
> > As the lines are on different chips they need to be requested
> > separately, they can't be combinied into a bulk request, but that is the
> > only restriction.
> >
> > > The problem was this: When I declared lines locally in a method and called this method to get the file descriptor, the file descriptor was the same for both lines. Like:
> > >
> > > int MyClass::getFiledescriptor(const std::string &linename)
> > > {
> > >     auto currentLine = gpiod::find_line(linename);
> > >     return currentLine.event_get_fd();
> > > }
> > >
> > > ... returned 23 for both parameters "in1" and "in2" while gpioinfo told me that these names where unique.
> >
> > I'm very surprised this works for you.  Note that find_line() and
> > get_line() methods return an unrequested line object.  You need to
> > request the line from the kernel using request() before calling
> > event_get_fd() - and that should throw if you haven't, which makes
> > me think your actual code is different or something very weird is
> > going on here.
> >
> > Have a look at the gpiogetcxx and gpiomoncxx examples to see how a
> > line is requested.
> >
> > What does gpioinfo show for those lines?
> > Why do you need the fd, anyway?  Are you confusing it with the offset?
> > And why poll when you can get edge events?
> >
> > Bart - the need to request lines is frequently misunderstood - Gasai Maple
> > had a similar problem, thinking get_line() and set_direction_output()
> > would be sufficient to set the line output value.
> > Consider updating the documentation to highlight which methods require
> > that the line is requested before they are usable - even if they do
> > return an error or throw in that case.
> 
> Make sense, thanks for the heads-up. Will do.
> 
> > This also applies to v2.
> >
> 
> I don't think so - in v2 you can't do anything without the request
> handle and you only get it when you call gpiod_chip_request_lines(). I
> will add some code examples in a dedicated directory though like many
> other projects do. Code > 1000 words. :)
> 

Yeah, my bad.  I was refering to the cxx bindings and eyeballing [1],
but the v2 cxx binding patches haven't been applied there yet, have they?
So that is still at v1, unlike my local branch that has the patches
applied :-(.

On that point, when you do merge patches into the libgpiod-2.0 branch
could you not squash them into one revision?  I'm losing track of where
we're at.  No problem with doing it just prior to release if you want to
remove all the development cruft, but having the full history is helpful
while things are unstable.

Cheers,
Kent.

> Bart
> 
> > >
> > > It is OK for me now, the two lines I'm polling are members of one class now, it works as intended, and I'm fine with that. But still I wonder if I misunderstood something or just did it wrong.
> > >
> > > Is a new version of libgpiod published soon? Then this whole text may be obsolete.
> > >
> >
> > Indeed, if I were you I would be looking at the libgpiod v2 branch[1]
> > that will soon obsolete v1 and has a slightly different API.  e.g.
> > gpiod::find_line() is gone as it is problematic - instead you need to
> > call find_line() on the appropriate chip and it now returns the line
> > offset.
> > That is assuming you are on a reasonably recent kernel - libgpiod v2
> > requires kernel v5.10 or later.
> >
> > Cheers,
> > Kent.
> >
> > [1] https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/?h=next/libgpiod-2.0
> >