Re: [libgpiod] Bug in python binding when requesting output?

[Pedro Botella <pbotella@xxxxxxxxx>]

Okay, got it. I have understood the function incorrectly then.

What I wanted to achieve was for the output to keep its current state
if it was already configured as an output, which I thought would be a
reasonable behavior. So I will instead wrap my requests with this:

def gpiod_safe_request_out(line, consumer):
    if line.direction() == gpiod.Line.DIRECTION_OUTPUT:
        # already an output, request as is and the output value won't
be modified
        line.request(consumer=consumer, type=gpiod.LINE_REQ_DIR_AS_IS)
    else:
        # Read current value
        line.request(consumer=consumer, type=gpiod.LINE_REQ_DIR_IN)
        value = line.get_value()
        line.release()
        # Request as output current value as default value
        line.request(consumer=consumer, type=gpiod.LINE_REQ_DIR_OUT,
default_val=value)

Which won't modify outputs, and if it is currently an input, will keep
the value at the pin.

I don't think I'm the most suitable for providing a patch for the
uninitialized default_vals, I'm not very well versed in providing
patches to the linux source tree, but I can give it a try if you want
me to.

Thanks for your help Kent!

Pedro

On Sat, Feb 13, 2021 at 1:23 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Thu, Feb 11, 2021 at 09:54:22PM +0100, Pedro Botella wrote:
> > Hi,
> >
> > I'm experiencing what I think is a bug in the python bindings for libgpiod.
> > I believe a line.request with type gpiod.LINE_REQ_DIR_OUT always
> > results in that line being set to '0'.
>
> That is correct - when requesting a line as output at the kernel uAPI
> the initial value must always be provided.  If you do not provide
> default_vals via the Python API then the output should be defaulted to
> '0' by the Python binding.
>
> > To reproduce:
> > 1. request a line with type gpiod.LINE_REQ_DIR_OUT
> > 2. set the line to '1'
> > 3. release the line
> > 4. request the same line with type gpiod.LINE_REQ_DIR_OUT
> > 5. get the value, it should now be '0'
> >
>
> To clarify, the expected behaviour is that the output is defaulted
> to '0' if default values are not provided.
> So the problem you are seeing is that the output is not consistently '0'?
>
> If you are expecting to see a '1' then you are expecting the lack of
> default_vals in the kwds to leave the output value as is, but that is
> not the case - it should default to '0'.
>
> > I think the issue is in "gpiod_LineBulk_request" in
> > https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/python/gpiodmodule.c
> > There a call to "gpiod_line_request_bulk" with default_vals being
> > passed as a pointer. Later on in the code, this parameter is checked
> > for NULL, if it is not NULL then the values in the array are used as
> > default_vals.
> > I believe that a NULL pointer should be passed instead if no
> > default_vals have been requested when doing a Line.request from
> > Python.
> >
>
> Agreed - passing default_vals uninitialized to gpiod_line_request_bulk()
> is a bug.
> It should be zeroed, or a NULL pointer should be passed if the
> default_vals were not provided in the kwds. Otherwise the output
> value will be set based on the uninitializezd contents of default_vals.
>
> Would you like to provide a patch?
>
> In the meantime the obvious workaround is to always provide default_vals
> in the kwds.
>
> Cheers,
> Kent.