Re: [libgpiod][PATCH] bindings: python: fix LineRequest.set_value only works for the last entry

[Kent Gibson <warthog618@xxxxxxxxx>]

On Tue, Jun 25, 2024 at 09:15:22AM +0800, chuang+git@xxxxxxxxxx wrote:
> From: Chuang Zhu <git@xxxxxxxxx>
>
> When multiple entries are requested using line names in
> Chip.request_lines, only the the last entry is added to
> LineRequest._name_map, causing a ValueError when trying to use
> LineRequest.set_value on any former entries.
>
> >>> import gpiod
> >>> lr = gpiod.Chip('/dev/gpiochip0').request_lines(
> ...     config={
> ...         'LINE0': gpiod.LineSettings(direction=gpiod.line.Direction.OUTPUT,
> ...                                     output_value=gpiod.line.Value.INACTIVE),
> ...         'LINE2': gpiod.LineSettings(direction=gpiod.line.Direction.OUTPUT, active_low=True,
> ...                                     output_value=gpiod.line.Value.ACTIVE),
> ...     }
> ... )
> >>> lr._name_map
> {'LINE2': 2}
> >>> lr.set_value('LINE0', gpiod.line.Value.ACTIVE)
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
>   File ".../gpiod/line_request.py", line 126, in set_value
>     self.set_values({line: value})
>   File ".../gpiod/line_request.py", line 138, in set_values
>     mapped = {
>              ^
>   File ".../gpiod/line_request.py", line 139, in <dictcomp>
>     self._name_map[line] if self._check_line_name(line) else line: values[line]
>                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>   File ".../gpiod/line_request.py", line 82, in _check_line_name
>     raise ValueError("unknown line name: {}".format(line))
> ValueError: unknown line name: LINE0
>

Rather than including example code and the error, put this, or similar, in a
test case to both demonstrate the problem and the correctness of the fix.

And, as well as describing the problem, you should describe what the fix
is that you have implemented.

The change itself looks ok to me.

Cheers,
Kent.

> Signed-off-by: Chuang Zhu <git@xxxxxxxxx>
> ---
>  bindings/python/gpiod/chip.py | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/bindings/python/gpiod/chip.py b/bindings/python/gpiod/chip.py
> index b3d8e61..ce77d27 100644
> --- a/bindings/python/gpiod/chip.py
> +++ b/bindings/python/gpiod/chip.py
> @@ -279,11 +279,12 @@ class Chip:
>          else:
>              mapped_output_values = None
>
> +        offsets = list()
> +        name_map = dict()
> +        offset_map = dict()
> +        global_output_values = list()
> +
>          for lines, settings in config.items():
> -            offsets = list()
> -            name_map = dict()
> -            offset_map = dict()
> -            global_output_values = list()
>
>              if isinstance(lines, int) or isinstance(lines, str):
>                  lines = (lines,)
> --
> 2.44.0
>