[PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Bartosz Golaszewski]

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

Certain polling APIs in the standard library - most notably: the
select() function and the poll class - allow to poll any object that
implements the fileno() method returning the underlying file descriptor
number.

Implement fileno() for Chip and LineRequest which allows users to do:

  rd, _, _ = select([chip/request], [], [], 1)

where rd will contain the actual object passed to select which makes for
easier reading of events afterwards.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 bindings/python/gpiod/chip.py             |  6 +++++
 bindings/python/gpiod/line_request.py     |  6 +++++
 bindings/python/tests/tests_edge_event.py | 37 +++++++++++++++++++++++++++++++
 bindings/python/tests/tests_info_event.py | 18 +++++++++++++++
 4 files changed, 67 insertions(+)

diff --git a/bindings/python/gpiod/chip.py b/bindings/python/gpiod/chip.py
index 30201b5..ddd07b8 100644
--- a/bindings/python/gpiod/chip.py
+++ b/bindings/python/gpiod/chip.py
@@ -344,6 +344,12 @@ class Chip:
 
         return request
 
+    def fileno(self) -> int:
+        """
+        Return the underlying file descriptor.
+        """
+        return self.fd
+
     def __repr__(self) -> str:
         """
         Return a string that can be used to re-create this chip object.
diff --git a/bindings/python/gpiod/line_request.py b/bindings/python/gpiod/line_request.py
index 9471442..ef53e16 100644
--- a/bindings/python/gpiod/line_request.py
+++ b/bindings/python/gpiod/line_request.py
@@ -224,6 +224,12 @@ class LineRequest:
 
         return cast(_ext.Request, self._req).read_edge_events(max_events)
 
+    def fileno(self) -> int:
+        """
+        Return the underlying file descriptor.
+        """
+        return self.fd
+
     def __str__(self) -> str:
         """
         Return a user-friendly, human-readable description of this request.
diff --git a/bindings/python/tests/tests_edge_event.py b/bindings/python/tests/tests_edge_event.py
index d7766ec..bd73802 100644
--- a/bindings/python/tests/tests_edge_event.py
+++ b/bindings/python/tests/tests_edge_event.py
@@ -4,6 +4,7 @@
 import time
 from datetime import timedelta
 from functools import partial
+from select import select
 from threading import Thread
 from typing import Optional
 from unittest import TestCase
@@ -201,6 +202,42 @@ class ReadingMultipleEdgeEvents(TestCase):
             self.global_seqno += 1
 
 
+class PollLineRequestObject(TestCase):
+    def setUp(self) -> None:
+        self.sim = gpiosim.Chip(num_lines=8)
+        self.request = gpiod.request_lines(
+            self.sim.dev_path, {2: gpiod.LineSettings(edge_detection=Edge.BOTH)}
+        )
+        self.thread: Optional[Thread] = None
+
+    def tearDown(self) -> None:
+        if self.thread:
+            self.thread.join()
+            del self.thread
+        self.request.release()
+        del self.request
+        del self.sim
+
+    def trigger_rising_edge(self, offset: int) -> None:
+        time.sleep(0.05)
+        self.sim.set_pull(offset, Pull.UP)
+
+    def test_select_request_object(self):
+        self.thread = Thread(target=partial(self.trigger_rising_edge, 2))
+        self.thread.start()
+
+        rd, wr, ex = select([self.request], [], [], 1)
+        self.assertFalse(wr)
+        self.assertFalse(ex)
+        self.assertEqual(rd[0], self.request)
+
+        events = rd[0].read_edge_events()
+        self.assertEqual(len(events), 1)
+
+        event = events[0]
+        self.assertEqual(event.line_offset, 2)
+
+
 class EdgeEventStringRepresentation(TestCase):
     def test_edge_event_str(self) -> None:
         sim = gpiosim.Chip()
diff --git a/bindings/python/tests/tests_info_event.py b/bindings/python/tests/tests_info_event.py
index e726a54..b3688f1 100644
--- a/bindings/python/tests/tests_info_event.py
+++ b/bindings/python/tests/tests_info_event.py
@@ -7,6 +7,7 @@ import threading
 import time
 from dataclasses import FrozenInstanceError
 from functools import partial
+from select import select
 from typing import Optional
 from unittest import TestCase
 
@@ -131,6 +132,23 @@ class WatchingInfoEventWorks(TestCase):
         self.assertGreater(ts_rel, ts_rec)
         self.assertGreater(ts_rec, ts_req)
 
+    def test_select_chip_object(self):
+        info = self.chip.watch_line_info(7)
+
+        self.thread = threading.Thread(
+            target=partial(request_reconfigure_release_line, self.sim.dev_path, 7)
+        )
+        self.thread.start()
+
+        rd, wr, ex = select([self.chip], [], [], 1)
+        self.assertFalse(wr)
+        self.assertFalse(ex)
+        self.assertEqual(rd[0], self.chip)
+
+        event = rd[0].read_info_event()
+        self.assertEqual(event.event_type, _EventType.LINE_REQUESTED)
+        self.assertEqual(event.line_info.offset, 7)
+
 
 class UnwatchingLineInfo(TestCase):
     def setUp(self) -> None:

---
base-commit: 6d9133a259e64da5e03c7e7784f0f27de7b3e59f
change-id: 20241210-python-fileno-1ed9c7bf413a

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

RE: [PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Vincent Fazio]

> -----Original Message-----
> 
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Certain polling APIs in the standard library - most notably: the
> select() function and the poll class - allow to poll any object that implements
> the fileno() method returning the underlying file descriptor number.
> 
> Implement fileno() for Chip and LineRequest which allows users to do:
> 
>   rd, _, _ = select([chip/request], [], [], 1)
> 
> where rd will contain the actual object passed to select which makes for
> easier reading of events afterwards.
> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> +class PollLineRequestObject(TestCase):
> +    def setUp(self) -> None:
> +        self.sim = gpiosim.Chip(num_lines=8)
> +        self.request = gpiod.request_lines(
> +            self.sim.dev_path, {2:
> gpiod.LineSettings(edge_detection=Edge.BOTH)}
> +        )
> +        self.thread: Optional[Thread] = None
> +
> +    def tearDown(self) -> None:
> +        if self.thread:
> +            self.thread.join()
> +            del self.thread
> +        self.request.release()
> +        del self.request
> +        del self.sim
> +
> +    def trigger_rising_edge(self, offset: int) -> None:
> +        time.sleep(0.05)
> +        self.sim.set_pull(offset, Pull.UP)
> +
> +    def test_select_request_object(self):

Nit:

def test_select_request_object(self) -> None:

> diff --git a/bindings/python/tests/tests_info_event.py
> b/bindings/python/tests/tests_info_event.py
> index e726a54..b3688f1 100644
> --- a/bindings/python/tests/tests_info_event.py
> +++ b/bindings/python/tests/tests_info_event.py
> @@ -7,6 +7,7 @@ import threading
>  import time
>  from dataclasses import FrozenInstanceError  from functools import partial
> +from select import select
>  from typing import Optional
>  from unittest import TestCase
> 
> @@ -131,6 +132,23 @@ class WatchingInfoEventWorks(TestCase):
>          self.assertGreater(ts_rel, ts_rec)
>          self.assertGreater(ts_rec, ts_req)
> 
> +    def test_select_chip_object(self):

Nit:

def test_select_chip_object(self) -> None:

These fail `mypy --strict` otherwise. These are optional checks so I'll leave it up to you to decide if you want to implement them.

Otherwise-

Reviewed-by: Vincent Fazio <vfazio@xes-inc.com>

Re: [PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Bartosz Golaszewski]

On Tue, Dec 10, 2024 at 3:17 PM Vincent Fazio <vfazio@xes-inc.com> wrote:
>
> >
> > @@ -131,6 +132,23 @@ class WatchingInfoEventWorks(TestCase):
> >          self.assertGreater(ts_rel, ts_rec)
> >          self.assertGreater(ts_rec, ts_req)
> >
> > +    def test_select_chip_object(self):
>
> Nit:
>
> def test_select_chip_object(self) -> None:
>
> These fail `mypy --strict` otherwise. These are optional checks so I'll leave it up to you to decide if you want to implement them.
>

Ah! This is why I didn't see it, I missed the --strict switch. Thanks.

On an unrelated note: mypy --strict is giving me this:

bindings/python/gpiod/line.py:19: error: Non-overlapping equality
check (left operand type: "Value", right operand type: "int")
[comparison-overlap]

for:

18     def __bool__(self) -> bool:
19         return self == self.ACTIVE

How do I fix it?

Bart

> Otherwise-
>
> Reviewed-by: Vincent Fazio <vfazio@xes-inc.com>
>

Re: [PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Vincent Fazio]

> -----Original Message-----
> Ah! This is why I didn't see it, I missed the --strict switch. Thanks.
> 
> On an unrelated note: mypy --strict is giving me this:
> 
> bindings/python/gpiod/line.py:19: error: Non-overlapping equality check (left
> operand type: "Value", right operand type: "int") [comparison-overlap]
> 
> for:
> 
> 18     def __bool__(self) -> bool:
> 19         return self == self.ACTIVE
> 
> How do I fix it?

This is odd, because I specifically ignore this:
https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/python/pyproject.toml#n57

How do you have this set up?

I run mypy checks from a virtual environment within bindings/python/ and this is not failing for me right now:

```
(venv) vfazio@vfazio4 ~/development/libgpiod/bindings/python $ mypy --strict
tests/tests_info_event.py:135: error: Function is missing a return type annotation  [no-untyped-def]
tests/tests_info_event.py:135: note: Use "-> None" if function does not return a value
tests/tests_edge_event.py:225: error: Function is missing a return type annotation  [no-untyped-def]
tests/tests_edge_event.py:225: note: Use "-> None" if function does not return a value
Found 2 errors in 2 files (checked 30 source files)
```

I can look into other ways to try to suppress this if necessary, like by adding an explicit ignore in the file itself.

Re: [PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Kent Gibson]

On Tue, Dec 10, 2024 at 03:21:29PM +0100, Bartosz Golaszewski wrote:
> On Tue, Dec 10, 2024 at 3:17 PM Vincent Fazio <vfazio@xes-inc.com> wrote:
> >
> > >
> > > @@ -131,6 +132,23 @@ class WatchingInfoEventWorks(TestCase):
> > >          self.assertGreater(ts_rel, ts_rec)
> > >          self.assertGreater(ts_rec, ts_req)
> > >
> > > +    def test_select_chip_object(self):
> >
> > Nit:
> >
> > def test_select_chip_object(self) -> None:
> >
> > These fail `mypy --strict` otherwise. These are optional checks so I'll leave it up to you to decide if you want to implement them.
> >
>
> Ah! This is why I didn't see it, I missed the --strict switch. Thanks.
>
> On an unrelated note: mypy --strict is giving me this:
>
> bindings/python/gpiod/line.py:19: error: Non-overlapping equality
> check (left operand type: "Value", right operand type: "int")
> [comparison-overlap]
>
> for:
>
> 18     def __bool__(self) -> bool:
> 19         return self == self.ACTIVE
>
> How do I fix it?
>

How about this:

    def __bool__(self) -> bool:
        return self.value == _ext.VALUE_ACTIVE

?

Granted that is a workaround and your original code should be fine, but
it does get rid of the warning...

Cheers,
Kent.

RE: [PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Vincent Fazio]

> -----Original Message-----
> From: Vincent Fazio
> Sent: Tuesday, December 10, 2024 8:27 AM
> To: Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Kent Gibson <warthog618@gmail.com>; Linus Walleij
> <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; Bartosz Golaszewski
> <bartosz.golaszewski@linaro.org>
> Subject: Re: [PATCH libgpiod] bindings: python: provide fileno() for Chip and
> LineRequest
> 
> 
> 
> > -----Original Message-----
> > Ah! This is why I didn't see it, I missed the --strict switch. Thanks.
> >
> > On an unrelated note: mypy --strict is giving me this:
> >
> > bindings/python/gpiod/line.py:19: error: Non-overlapping equality check
> (left
> > operand type: "Value", right operand type: "int") [comparison-overlap]
> >
> > for:
> >
> > 18     def __bool__(self) -> bool:
> > 19         return self == self.ACTIVE
> >
> > How do I fix it?
> 
> This is odd, because I specifically ignore this:
> https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/python
> /pyproject.toml#n57
> 
> How do you have this set up?

If you're doing this outside of the bindings/python directory, try specifying the config file:

```
(xtf-py3.12) vfazio@vfazio4 ~/development/libgpiod $ mypy --config-file=bindings/python/pyproject.toml --strict bindings/python/gpiod
Success: no issues found in 13 source files
```

However, if you run it on `bindings/python` the output will be noisy because it's not limiting itself to the "files" section within the mypy settings.

I wrote the pyproject settings with the unwritten rule that the checks would be rooted within `bindings/python` and not outside of it.

We should probably document our expectations for how these checks are run and then update the config file to respect those expectations.

Re: [PATCH libgpiod] bindings: python: provide fileno() for Chip and LineRequest

[Bartosz Golaszewski]

On Tue, Dec 10, 2024 at 4:04 PM Vincent Fazio <vfazio@xes-inc.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Vincent Fazio
> > Sent: Tuesday, December 10, 2024 8:27 AM
> > To: Bartosz Golaszewski <brgl@bgdev.pl>
> > Cc: Kent Gibson <warthog618@gmail.com>; Linus Walleij
> > <linus.walleij@linaro.org>; linux-gpio@vger.kernel.org; Bartosz Golaszewski
> > <bartosz.golaszewski@linaro.org>
> > Subject: Re: [PATCH libgpiod] bindings: python: provide fileno() for Chip and
> > LineRequest
> >
> >
> >
> > > -----Original Message-----
> > > Ah! This is why I didn't see it, I missed the --strict switch. Thanks.
> > >
> > > On an unrelated note: mypy --strict is giving me this:
> > >
> > > bindings/python/gpiod/line.py:19: error: Non-overlapping equality check
> > (left
> > > operand type: "Value", right operand type: "int") [comparison-overlap]
> > >
> > > for:
> > >
> > > 18     def __bool__(self) -> bool:
> > > 19         return self == self.ACTIVE
> > >
> > > How do I fix it?
> >
> > This is odd, because I specifically ignore this:
> > https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/tree/bindings/python
> > /pyproject.toml#n57
> >
> > How do you have this set up?
>
> If you're doing this outside of the bindings/python directory, try specifying the config file:
>
> ```
> (xtf-py3.12) vfazio@vfazio4 ~/development/libgpiod $ mypy --config-file=bindings/python/pyproject.toml --strict bindings/python/gpiod
> Success: no issues found in 13 source files
> ```

Yes, this works, thanks.

Bart

>
> However, if you run it on `bindings/python` the output will be noisy because it's not limiting itself to the "files" section within the mypy settings.
>
> I wrote the pyproject settings with the unwritten rule that the checks would be rooted within `bindings/python` and not outside of it.
>

Got it.

Bart

> We should probably document our expectations for how these checks are run and then update the config file to respect those expectations.