diff mbox series

[libgpiod,v2,1/2] bindings: python: tests: add a test case for line request by name with multiple entries

Message ID 20240627153103.594107-1-chuang+git@melty.land
State New
Headers show
Series [libgpiod,v2,1/2] bindings: python: tests: add a test case for line request by name with multiple entries | expand

Commit Message

Chuang Zhu June 27, 2024, 3:31 p.m. UTC
From: Chuang Zhu <git@chuang.cz>

Signed-off-by: Chuang Zhu <git@chuang.cz>
---
 bindings/python/tests/tests_line_request.py | 34 +++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Kent Gibson June 28, 2024, 10:31 a.m. UTC | #1
On Thu, Jun 27, 2024 at 11:31:02PM +0800, Chuang Zhu wrote:
> From: Chuang Zhu <git@chuang.cz>
>

Add a commit comment.

> Signed-off-by: Chuang Zhu <git@chuang.cz>
> ---
>  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 f99b93d..c9a5c81 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 LineRequestComplexConfigByName(TestCase):

Nothing complex about the config, so LineRequestMixedConfigByName?

> +    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"]),
> +            [Value.INACTIVE, Value.ACTIVE],
> +        )
> +

Add "baz" to the lines to get so this test fails as well - expected to
be INACTIVE.

Cheers,
Kent.

> +
>  class RepeatingLinesInRequestConfig(TestCase):
>      def setUp(self):
>          self.sim = gpiosim.Chip(num_lines=4, line_names={0: "foo", 2: "bar"})
> --
> 2.44.0
>
Kent Gibson June 28, 2024, 10:44 a.m. UTC | #2
On Thu, Jun 27, 2024 at 11:31:03PM +0800, Chuang Zhu wrote:
> 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.
>

Chip.request_lines()

> This patch fixes that by moving the required variables to the correct
> scope.
>

Use imperitive, "Move the required...."

> Signed-off-by: Chuang Zhu <git@chuang.cz>

Use the same address to sign off and send the submission email.

With a multi-patch series you should include a cover letter that
describes the series the changes between versions.

e.g. here you have fixed my review comments, but also fixed not moving the
offsets list into the outer scope.

Cheers,
Kent.

> ---
>  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 19c62cd..55f0c3e 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.44.0
>
diff mbox series

Patch

diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
index f99b93d..c9a5c81 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 LineRequestComplexConfigByName(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"]),
+            [Value.INACTIVE, Value.ACTIVE],
+        )
+
+
 class RepeatingLinesInRequestConfig(TestCase):
     def setUp(self):
         self.sim = gpiosim.Chip(num_lines=4, line_names={0: "foo", 2: "bar"})