Re: [libgpiod] Python bindings don't allow to wait on events indefinitely

[Kent Gibson <warthog618@xxxxxxxxx>]

On Fri, May 19, 2023 at 04:32:32PM +0200, Nicolas Frattaroli wrote:
> On Freitag, 19. Mai 2023 07:17:27 CEST Kent Gibson wrote:
> > On Thu, May 11, 2023 at 10:28:34PM +0200, Nicolas Frattaroli wrote:
> > > Hello,
> > > 
> > > in libgpiod 1.6.x, Line.event_wait's codepath had no path where ts
> > > as passed to ppoll could ever be NULL. This means waiting indefinitely
> > > was impossible.
> > > 
> > > I thought hey, maybe the new Python bindings in libgpiod 2.x fixed this,
> > > but no, it has made it worse by explicitly setting timeout to 0 seconds
> > > if it's None[1]. Obviously, this behaviour can't be changed now, because
> > > people depend on this API to return immediately now with None as the
> > > parameter, and changing it to wait indefinitely would no doubt break
> > > actual programs.
> > > 
> > > So I'm left wondering if there's a particular reason users of these
> > > bindings shouldn't wait on events indefinitely or if that same mistake
> > > was just made twice in a row.
> > > 
> > > Is there some way the API could be enhanced to support waiting for
> > > events indefinitely without having to slap a While True with
> > > an arbitrarily high timeout around every single invocation?
> > > 
> 
> Hello Kent,
>  
> > That does sound like a bug to me, but the rest of your mail isn't worth
> > responding to.
> 
> I'm not quite sure what you mean. Was my tone this off? I apologise if
> you took my displeasure with libgpiod's bindings as a personal attack,
> it wasn't intended as such.
> 

Not a personal attack on me, but offensive none the less.
Assume you are the code owner and see how it parses to you.

> > A more productive approach could be to submit a patch that describes the
> > problem and suggests a fix, say:
> > 
> >  def poll_fd(fd: int, timeout: Optional[Union[timedelta, float]] = None) -> bool:
> > -    if timeout is None:
> > -        timeout = 0.0
> > -
> > 
> > and see where that goes.
> 
> That would go nowhere, as this makes the API behave differently for current
> users calling the function without an argument, as I've mentioned.
> 

By that logic any bug that has visible effects cannot be fixed - in case
some user depends on those effects.
And all bugs have visible effects.

The function docs don't specify the behaviour when None is passed -
despite None being the default.

> One solution would be to pass float("inf") and then check for that, this
> wouldn't break the API, merely extend it, but I'm not sure how good of
> an idea that is to do if someone uses an older libgpiod that doesn't
> have an explicit check for inf. I don't even know what passing inf does
> right now, but it's probably worth looking into.
> 

Please no.

I would expect the API should stick to standard Python conventions,
similar to select itself - " When the timeout argument is omitted the
function blocks until at least one file descriptor is ready."
so wait_edge_event() should block, as should wait_edge_event(None)

> > to submit a patch
> 
> Move the project back to a git forge and I will.
> 

And there you go being confrontational again. GFY.

Read the README - you submit patches to this list.

And not my project, so not my call on where it is hosted.
I'm just trying to help you out here, but I'm left wondering why.

Cheers,
Kent.