Skip to contentSkip to conversation

bindings: python: fix line request by name with multiple entries

PATCH Applied Bartosz Golaszewski 2 participants 5 messages Jul 10, 2024 – Jul 11, 2024 GPIO

lore
Share & tools
lore

Bartosz Golaszewski

author
#0 lore raw

v3 cover bindings: python: fix line request by name with multiple entries

From: Bartosz Golaszewski <[email protected]>

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(-)
Signature · 1 line
2.43.0
author
#1 lore raw

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

From: Chuang Zhu <[email protected]>

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 <[email protected]>
Co-developed-by Bartosz Golaszewski <[email protected]>
[Bartosz: tweak the commit message]
Signed-off-by Bartosz Golaszewski <[email protected]>
---
 bindings/python/gpiod/chip.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
bindings/python/gpiod/chip.py +4 -3
index 47bda11..7692ba4 100644
@@ -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,)
Signature · 1 line
2.43.0
author
#2 lore raw

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

In reply to Bartosz Golaszewski

From: Chuang Zhu <[email protected]>

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

Signed-off-by Chuang Zhu <[email protected]>
Co-developed-by Bartosz Golaszewski <[email protected]>

[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 <[email protected]>
---
 bindings/python/tests/tests_line_request.py | 34 +++++++++++++++++++++
 1 file changed, 34 insertions(+)
bindings/python/tests/tests_line_request.py +34 -0
index 79167f1..c79a324 100644
@@ -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"})
Signature · 1 line
2.43.0
Reviewed-by on the v3 series
#3 lore raw
In reply to Bartosz Golaszewski
Quoted 5 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

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

Quoted 9 lines

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 <[email protected]>

for the series.

author Applied
#4 lore raw
In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Quoted 11 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

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,

Signature · 1 line
Bartosz Golaszewski <[email protected]>