[libgpiod][PATCH v3 0/2] bindings: python: fix line request by name with multiple entries

[libgpiod][PATCH v3 0/2] bindings: python: fix line request by name with multiple entries

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We haven't heard from Chuang in two weeks but I want to finalize this
series so I applied the hints from Kent.

Changes in v3:
- tweak the commit messages
- change the test class name
- improve the test case by testing one more line name

Chuang Zhu (2):
  bindings: python: fix line request by name with multiple entries
  bindings: python: tests: add a new test case

 bindings/python/gpiod/chip.py               |  7 +++--
 bindings/python/tests/tests_line_request.py | 34 +++++++++++++++++++++
 2 files changed, 38 insertions(+), 3 deletions(-)

-- 
2.43.0

[libgpiod][PATCH v3 1/2] bindings: python: fix line request by name with multiple entries

[Bartosz Golaszewski]

From: Chuang Zhu <git@chuang.cz>

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 functions
like LineRequest.set_value() on any former entries.

Move the required variables to the correct scope.

Signed-off-by: Chuang Zhu <git@chuang.cz>
Co-developed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
[Bartosz: tweak the commit message]
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 bindings/python/gpiod/chip.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/bindings/python/gpiod/chip.py b/bindings/python/gpiod/chip.py
index 47bda11..7692ba4 100644
--- a/bindings/python/gpiod/chip.py
+++ b/bindings/python/gpiod/chip.py
@@ -279,11 +279,12 @@ class Chip:
         else:
             mapped_output_values = None
 
+        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.43.0

[libgpiod][PATCH v3 2/2] bindings: python: tests: add a new test case

[Bartosz Golaszewski]

From: Chuang Zhu <git@chuang.cz>

Add a test-case for line request by name with multiple entries.

Signed-off-by: Chuang Zhu <git@chuang.cz>
Co-developed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
[Bartosz:
  - tweak the commit message
  - improve the test class name
  - extend the test assertion to test the 'baz' line too]
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 bindings/python/tests/tests_line_request.py | 34 +++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
index 79167f1..c79a324 100644
--- a/bindings/python/tests/tests_line_request.py
+++ b/bindings/python/tests/tests_line_request.py
@@ -310,6 +310,40 @@ class LineRequestComplexConfig(TestCase):
                 self.assertEqual(chip.get_line_info(3).edge_detection, Edge.BOTH)
 
 
+class LineRequestMixedConfigByName(TestCase):
+    def setUp(self):
+        self.sim = gpiosim.Chip(
+            num_lines=4, line_names={2: "foo", 3: "bar", 1: "baz", 0: "xyz"}
+        )
+        self.req = gpiod.request_lines(
+            self.sim.dev_path,
+            {
+                ("baz", "bar"): gpiod.LineSettings(direction=Direction.OUTPUT),
+                ("foo", "xyz"): gpiod.LineSettings(direction=Direction.INPUT),
+            },
+        )
+
+    def tearDown(self):
+        self.req.release()
+        del self.req
+        del self.sim
+
+    def test_set_values_by_name(self):
+        self.req.set_values({"bar": Value.ACTIVE, "baz": Value.INACTIVE})
+
+        self.assertEqual(self.sim.get_value(1), SimVal.INACTIVE)
+        self.assertEqual(self.sim.get_value(3), SimVal.ACTIVE)
+
+    def test_get_values_by_name(self):
+        self.sim.set_pull(0, Pull.UP)
+        self.sim.set_pull(2, Pull.DOWN)
+
+        self.assertEqual(
+            self.req.get_values(["foo", "xyz", "baz"]),
+            [Value.INACTIVE, Value.ACTIVE, Value.INACTIVE],
+        )
+
+
 class RepeatingLinesInRequestConfig(TestCase):
     def setUp(self):
         self.sim = gpiosim.Chip(num_lines=4, line_names={0: "foo", 2: "bar"})
-- 
2.43.0

Re: [libgpiod][PATCH v3 0/2] bindings: python: fix line request by name with multiple entries

[Kent Gibson]

On Wed, Jul 10, 2024 at 02:57:17PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We haven't heard from Chuang in two weeks but I want to finalize this
> series so I applied the hints from Kent.
>
> Changes in v3:
> - tweak the commit messages
> - change the test class name
> - improve the test case by testing one more line name
>
> Chuang Zhu (2):
>   bindings: python: fix line request by name with multiple entries
>   bindings: python: tests: add a new test case
>

I prefer adding the test case first, to demonstrate the problem before
fixing it, but it isn't a deal breaker.
Other than that, looks good to me.

Reviewed-by: Kent Gibson <warthog618@gmail.com>

for the series.

Re: [libgpiod][PATCH v3 0/2] bindings: python: fix line request by name with multiple entries

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Wed, 10 Jul 2024 14:57:17 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> We haven't heard from Chuang in two weeks but I want to finalize this
> series so I applied the hints from Kent.
> 
> Changes in v3:
> - tweak the commit messages
> - change the test class name
> - improve the test case by testing one more line name
> 
> [...]

Applied, thanks!

[1/2] bindings: python: fix line request by name with multiple entries
      commit: 237fd2c0597e720980023975250497d30aad1c12
[2/2] bindings: python: tests: add a new test case
      commit: 0c7ec9ee42912db00850c7570bbfd5325339da13

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>