Skip to contentSkip to conversation

bindings: implement python bindings for libgpiod v2

PATCH Superseded Bartosz Golaszewski 3 participants 30 messages Oct 7, 2022 – Oct 26, 2022 GPIO

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover bindings: implement python bindings for libgpiod v2

This is the third iteration of python bindings for libgpiod but it really has very little in common with the previous version.

This time the code has been split into high-level python and low-level C layers with the latter only doing the bare minimum.

The data model is mostly based on the C++ one with the main difference being utilizing dynamic typing and keyword arguments in place of the builder pattern. That allows us to reduce the number of methods and objects.

Because python doesn't have RAII, unlike C++, we provide a module-level request_lines() helper as gpiod.Chip(path).request_lines(...) one-liner could lead to the chip left dangling even after the last reference is dropped.

Because python forces us to dynamically allocate objects all the time even for fundamental types (which are also immutable) there's no point in trying to replicate the edge-event buffer. Instead LineRequest.read_edge_event() just returns a list of events.

Bartosz Golaszewski (4):
bindings: python: remove old version
bindings: python: add examples
bindings: python: add tests
bindings: python: implement python bindings for libgpiod v2

 bindings/python/.gitignore                   |    8 +
 bindings/python/Makefile.am                  |   26 +-
 bindings/python/examples/Makefile.am         |   16 +-
 bindings/python/examples/gpiodetect.py       |   15 +-
 bindings/python/examples/gpiofind.py         |   14 +-
 bindings/python/examples/gpioget.py          |   34 +-
 bindings/python/examples/gpioinfo.py         |   41 +-
 bindings/python/examples/gpiomon.py          |   52 +-
 bindings/python/examples/gpioset.py          |   46 +-
 bindings/python/gpiod/Makefile.am            |   17 +
 bindings/python/gpiod/__init__.py            |   53 +
 bindings/python/gpiod/chip.py                |  308 ++
 bindings/python/gpiod/chip_info.py           |   21 +
 bindings/python/gpiod/edge_event.py          |   46 +
 bindings/python/gpiod/exception.py           |   20 +
 bindings/python/gpiod/ext/Makefile.am        |   11 +
 bindings/python/gpiod/ext/chip.c             |  335 +++
 bindings/python/gpiod/ext/common.c           |   92 +
 bindings/python/gpiod/ext/internal.h         |   20 +
 bindings/python/gpiod/ext/line-config.c      |  133 +
 bindings/python/gpiod/ext/line-settings.c    |  130 +
 bindings/python/gpiod/ext/module.c           |  193 ++
 bindings/python/gpiod/ext/request.c          |  402 +++
 bindings/python/gpiod/info_event.py          |   33 +
 bindings/python/gpiod/internal.py            |   19 +
 bindings/python/gpiod/line.py                |   56 +
 bindings/python/gpiod/line_info.py           |   73 +
 bindings/python/gpiod/line_request.py        |  258 ++
 bindings/python/gpiod/line_settings.py       |   62 +
 bindings/python/gpiodmodule.c                | 2662 ------------------
 bindings/python/setup.py                     |   47 +
 bindings/python/tests/Makefile.am            |   26 +-
 bindings/python/tests/__init__.py            |   17 +
 bindings/python/tests/__main__.py            |   16 +
 bindings/python/tests/gpiod_py_test.py       |  832 ------
 bindings/python/tests/gpiomockupmodule.c     |  309 --
 bindings/python/tests/gpiosim/Makefile.am    |    7 +
 bindings/python/tests/gpiosim/__init__.py    |    4 +
 bindings/python/tests/gpiosim/chip.py        |   66 +
 bindings/python/tests/gpiosim/ext.c          |  345 +++
 bindings/python/tests/helpers.py             |   16 +
 bindings/python/tests/tests_chip.py          |  231 ++
 bindings/python/tests/tests_chip_info.py     |   52 +
 bindings/python/tests/tests_edge_event.py    |  219 ++
 bindings/python/tests/tests_info_event.py    |  189 ++
 bindings/python/tests/tests_line_info.py     |  101 +
 bindings/python/tests/tests_line_request.py  |  449 +++
 bindings/python/tests/tests_line_settings.py |   79 +
 bindings/python/tests/tests_module.py        |   59 +
 configure.ac                                 |    3 +
 50 files changed, 4338 insertions(+), 3925 deletions(-)
 create mode 100644 bindings/python/.gitignore
 create mode 100644 bindings/python/gpiod/Makefile.am
 create mode 100644 bindings/python/gpiod/__init__.py
 create mode 100644 bindings/python/gpiod/chip.py
 create mode 100644 bindings/python/gpiod/chip_info.py
 create mode 100644 bindings/python/gpiod/edge_event.py
 create mode 100644 bindings/python/gpiod/exception.py
 create mode 100644 bindings/python/gpiod/ext/Makefile.am
 create mode 100644 bindings/python/gpiod/ext/chip.c
 create mode 100644 bindings/python/gpiod/ext/common.c
 create mode 100644 bindings/python/gpiod/ext/internal.h
 create mode 100644 bindings/python/gpiod/ext/line-config.c
 create mode 100644 bindings/python/gpiod/ext/line-settings.c
 create mode 100644 bindings/python/gpiod/ext/module.c
 create mode 100644 bindings/python/gpiod/ext/request.c
 create mode 100644 bindings/python/gpiod/info_event.py
 create mode 100644 bindings/python/gpiod/internal.py
 create mode 100644 bindings/python/gpiod/line.py
 create mode 100644 bindings/python/gpiod/line_info.py
 create mode 100644 bindings/python/gpiod/line_request.py
 create mode 100644 bindings/python/gpiod/line_settings.py
 delete mode 100644 bindings/python/gpiodmodule.c
 create mode 100644 bindings/python/setup.py
 create mode 100644 bindings/python/tests/__init__.py
 create mode 100644 bindings/python/tests/__main__.py
 delete mode 100755 bindings/python/tests/gpiod_py_test.py
 delete mode 100644 bindings/python/tests/gpiomockupmodule.c
 create mode 100644 bindings/python/tests/gpiosim/Makefile.am
 create mode 100644 bindings/python/tests/gpiosim/__init__.py
 create mode 100644 bindings/python/tests/gpiosim/chip.py
 create mode 100644 bindings/python/tests/gpiosim/ext.c
 create mode 100644 bindings/python/tests/helpers.py
 create mode 100644 bindings/python/tests/tests_chip.py
 create mode 100644 bindings/python/tests/tests_chip_info.py
 create mode 100644 bindings/python/tests/tests_edge_event.py
 create mode 100644 bindings/python/tests/tests_info_event.py
 create mode 100644 bindings/python/tests/tests_line_info.py
 create mode 100644 bindings/python/tests/tests_line_request.py
 create mode 100644 bindings/python/tests/tests_line_settings.py
 create mode 100644 bindings/python/tests/tests_module.py
Signature · 1 line
2.34.1
#1 lore raw

PATCH v3 1/4 bindings: python: remove old version

This removes v1 python bindings for easier review of v2.

---
 bindings/python/Makefile.am              |   25 -
 bindings/python/examples/Makefile.am     |   10 -
 bindings/python/examples/gpiodetect.py   |   16 -
 bindings/python/examples/gpiofind.py     |   20 -
 bindings/python/examples/gpioget.py      |   25 -
 bindings/python/examples/gpioinfo.py     |   28 -
 bindings/python/examples/gpiomon.py      |   42 -
 bindings/python/examples/gpioset.py      |   25 -
 bindings/python/gpiodmodule.c            | 2662 ----------------------
 bindings/python/tests/Makefile.am        |   13 -
 bindings/python/tests/gpiod_py_test.py   |  832 -------
 bindings/python/tests/gpiomockupmodule.c |  309 ---
 12 files changed, 4007 deletions(-)
 delete mode 100644 bindings/python/Makefile.am
 delete mode 100644 bindings/python/examples/Makefile.am
 delete mode 100755 bindings/python/examples/gpiodetect.py
 delete mode 100755 bindings/python/examples/gpiofind.py
 delete mode 100755 bindings/python/examples/gpioget.py
 delete mode 100755 bindings/python/examples/gpioinfo.py
 delete mode 100755 bindings/python/examples/gpiomon.py
 delete mode 100755 bindings/python/examples/gpioset.py
 delete mode 100644 bindings/python/gpiodmodule.c
 delete mode 100644 bindings/python/tests/Makefile.am
 delete mode 100755 bindings/python/tests/gpiod_py_test.py
 delete mode 100644 bindings/python/tests/gpiomockupmodule.c
bindings/python/Makefile.am deleted +0 -25
deleted file mode 100644
index 4405d8f..0000000
@@ -1,25 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-pyexec_LTLIBRARIES = gpiod.la
-
-gpiod_la_SOURCES = gpiodmodule.c
-
-gpiod_la_CFLAGS = -I$(top_srcdir)/include/
-gpiod_la_CFLAGS += -Wall -Wextra -g -std=gnu89 $(PYTHON_CPPFLAGS)
-gpiod_la_LDFLAGS = -module -avoid-version
-gpiod_la_LIBADD = $(top_builddir)/lib/libgpiod.la $(PYTHON_LIBS)
-
-SUBDIRS = .
-
-if WITH_TESTS
-
-SUBDIRS += tests
-
-endif
-
-if WITH_EXAMPLES
-
-SUBDIRS += examples
-
-endif
bindings/python/examples/Makefile.am deleted +0 -10
deleted file mode 100644
index 4169469..0000000
@@ -1,10 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-EXTRA_DIST =				\
-		gpiodetect.py		\
-		gpiofind.py		\
-		gpioget.py		\
-		gpioinfo.py		\
-		gpiomon.py		\
-		gpioset.py
bindings/python/examples/gpiodetect.py deleted +0 -16
deleted file mode 100755
index da6ee9a..0000000
@@ -1,16 +0,0 @@
-#!/usr/bin/env python3
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-'''Reimplementation of the gpiodetect tool in Python.'''
-
-import gpiod
-import os
-
-if __name__ == '__main__':
-    for entry in os.scandir('/dev/'):
-        if gpiod.is_gpiochip_device(entry.path):
-            with gpiod.Chip(entry.path) as chip:
-                print('{} [{}] ({} lines)'.format(chip.name(),
-                                                  chip.label(),
-                                                  chip.num_lines()))
bindings/python/examples/gpiofind.py deleted +0 -20
deleted file mode 100755
index a9ec734..0000000
@@ -1,20 +0,0 @@
-#!/usr/bin/env python3
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-'''Reimplementation of the gpiofind tool in Python.'''
-
-import gpiod
-import os
-import sys
-
-if __name__ == '__main__':
-    for entry in os.scandir('/dev/'):
-        if gpiod.is_gpiochip_device(entry.path):
-            with gpiod.Chip(entry.path) as chip:
-                offset = chip.find_line(sys.argv[1], unique=True)
-                if offset is not None:
-                     print('{} {}'.format(line.owner().name(), offset))
-                     sys.exit(0)
-
-    sys.exit(1)
bindings/python/examples/gpioget.py deleted +0 -25
deleted file mode 100755
index 26a2ced..0000000
@@ -1,25 +0,0 @@
-#!/usr/bin/env python3
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-'''Simplified reimplementation of the gpioget tool in Python.'''
-
-import gpiod
-import sys
-
-if __name__ == '__main__':
-    if len(sys.argv) < 3:
-        raise TypeError('usage: gpioget.py <gpiochip> <offset1> <offset2> ...')
-
-    with gpiod.Chip(sys.argv[1]) as chip:
-        offsets = []
-        for off in sys.argv[2:]:
-            offsets.append(int(off))
-
-        lines = chip.get_lines(offsets)
-        lines.request(consumer=sys.argv[0], type=gpiod.LINE_REQ_DIR_IN)
-        vals = lines.get_values()
-
-        for val in vals:
-            print(val, end=' ')
-        print()
bindings/python/examples/gpioinfo.py deleted +0 -28
deleted file mode 100755
index 84188f1..0000000
@@ -1,28 +0,0 @@
-#!/usr/bin/env python3
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-'''Simplified reimplementation of the gpioinfo tool in Python.'''
-
-import gpiod
-import os
-
-if __name__ == '__main__':
-    for entry in os.scandir('/dev/'):
-        if gpiod.is_gpiochip_device(entry.path):
-            with gpiod.Chip(entry.path) as chip:
-                print('{} - {} lines:'.format(chip.name(), chip.num_lines()))
-
-                for line in gpiod.LineIter(chip):
-                    offset = line.offset()
-                    name = line.name()
-                    consumer = line.consumer()
-                    direction = line.direction()
-                    active_low = line.is_active_low()
-
-                    print('\tline {:>3}: {:>18} {:>12} {:>8} {:>10}'.format(
-                          offset,
-                          'unnamed' if name is None else name,
-                          'unused' if consumer is None else consumer,
-                          'input' if direction == gpiod.Line.DIRECTION_INPUT else 'output',
-                          'active-low' if active_low else 'active-high'))
bindings/python/examples/gpiomon.py deleted +0 -42

42 lines not on this page · raw message

bindings/python/examples/gpioset.py deleted +0 -25
deleted file mode 100755
index 63e08dc..0000000
@@ -1,25 +0,0 @@
-#!/usr/bin/env python3
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-'''Simplified reimplementation of the gpioset tool in Python.'''
-
-import gpiod
-import sys
-
-if __name__ == '__main__':
-    if len(sys.argv) < 3:
-        raise TypeError('usage: gpioset.py <gpiochip> <offset1>=<value1> ...')
-
-    with gpiod.Chip(sys.argv[1]) as chip:
-        offsets = []
-        values = []
-        for arg in sys.argv[2:]:
-            arg = arg.split('=')
-            offsets.append(int(arg[0]))
-            values.append(int(arg[1]))
-
-        lines = chip.get_lines(offsets)
-        lines.request(consumer=sys.argv[0], type=gpiod.LINE_REQ_DIR_OUT)
-        lines.set_values(values)
-        input()
bindings/python/gpiodmodule.c deleted +0 -2662

2,662 lines not on this page · raw message

bindings/python/tests/Makefile.am deleted +0 -13
deleted file mode 100644
index 972b669..0000000
@@ -1,13 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-or-later
-# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <[email protected]>
-
-dist_bin_SCRIPTS = gpiod_py_test.py
-
-pyexec_LTLIBRARIES = gpiomockup.la
-
-gpiomockup_la_SOURCES = gpiomockupmodule.c
-gpiomockup_la_CFLAGS = -I$(top_srcdir)/tests/mockup/
-gpiomockup_la_CFLAGS += -Wall -Wextra -g -std=gnu89 $(PYTHON_CPPFLAGS)
-gpiomockup_la_LDFLAGS = -module -avoid-version
-gpiomockup_la_LIBADD = $(top_builddir)/tests/mockup/libgpiomockup.la
-gpiomockup_la_LIBADD += $(PYTHON_LIBS)
bindings/python/tests/gpiod_py_test.py deleted +0 -832

134 lines not on this page · raw message

bindings/python/tests/gpiomockupmodule.c deleted +0 -309

309 lines not on this page · raw message

The rest of this diff is not shown. View the raw message.

Signature · 1 line
2.34.1

Part of the diff is not shown here. View the raw message.

#2 lore raw

PATCH v3 2/4 bindings: python: add examples

In reply to Bartosz Golaszewski

This adds the regular set of example programs implemented using libgpiod python bindings.

---
 bindings/python/examples/Makefile.am   | 10 +++++++
 bindings/python/examples/gpiodetect.py | 17 ++++++++++++
 bindings/python/examples/gpiofind.py   | 20 ++++++++++++++
 bindings/python/examples/gpioget.py    | 31 +++++++++++++++++++++
 bindings/python/examples/gpioinfo.py   | 35 ++++++++++++++++++++++++
 bindings/python/examples/gpiomon.py    | 28 +++++++++++++++++++
 bindings/python/examples/gpioset.py    | 37 ++++++++++++++++++++++++++
 7 files changed, 178 insertions(+)
 create mode 100644 bindings/python/examples/Makefile.am
 create mode 100755 bindings/python/examples/gpiodetect.py
 create mode 100755 bindings/python/examples/gpiofind.py
 create mode 100755 bindings/python/examples/gpioget.py
 create mode 100755 bindings/python/examples/gpioinfo.py
 create mode 100755 bindings/python/examples/gpiomon.py
 create mode 100755 bindings/python/examples/gpioset.py
bindings/python/examples/Makefile.am new file +10 -0
new file mode 100644
index 0000000..f42b80e
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+EXTRA_DIST = \
+	gpiodetect.py \
+	gpiofind.py \
+	gpioget.py \
+	gpioinfo.py \
+	gpiomon.py \
+	gpioset.py
bindings/python/examples/gpiodetect.py new file +17 -0
new file mode 100755
index 0000000..c32014f
@@ -0,0 +1,17 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Reimplementation of the gpiodetect tool in Python."""
+
+import gpiod
+import os
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                info = chip.get_info()
+                print(
+                    "{} [{}] ({} lines)".format(info.name, info.label, info.num_lines)
+                )
bindings/python/examples/gpiofind.py new file +20 -0
new file mode 100755
index 0000000..2f30445
@@ -0,0 +1,20 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Reimplementation of the gpiofind tool in Python."""
+
+import gpiod
+import os
+import sys
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                offset = chip.map_line(sys.argv[1])
+                if offset is not None:
+                    print("{} {}".format(chip.get_info().name, offset))
+                    sys.exit(0)
+
+    sys.exit(1)
bindings/python/examples/gpioget.py new file +31 -0
new file mode 100755
index 0000000..d441535
@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioget tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Direction
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError("usage: gpioget.py <gpiochip> <offset1> <offset2> ...")
+
+    path = sys.argv[1]
+    lines = []
+    for line in sys.argv[2:]:
+        lines.append(int(line) if line.isdigit() else line)
+
+    request = gpiod.request_lines(
+        path,
+        consumer="gpioget.py",
+        config={tuple(lines): gpiod.LineSettings(direction=Direction.INPUT)},
+    )
+
+    vals = request.get_values()
+
+    for val in vals:
+        print("{} ".format(val.value), end="")
+    print()
bindings/python/examples/gpioinfo.py new file +35 -0
new file mode 100755
index 0000000..e8c7d46
@@ -0,0 +1,35 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioinfo tool in Python."""
+
+import gpiod
+import os
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                cinfo = chip.get_info()
+                print("{} - {} lines:".format(cinfo.name, cinfo.num_lines))
+
+                for offset in range(0, cinfo.num_lines):
+                    linfo = chip.get_line_info(offset)
+                    offset = linfo.offset
+                    name = linfo.name
+                    consumer = linfo.consumer
+                    direction = linfo.direction
+                    active_low = linfo.active_low
+
+                    print(
+                        "\tline {:>3}: {:>18} {:>12} {:>8} {:>10}".format(
+                            offset,
+                            "unnamed" if name is None else name,
+                            "unused" if consumer is None else consumer,
+                            "input"
+                            if direction == gpiod.line.Direction.INPUT
+                            else "output",
+                            "active-low" if active_low else "active-high",
+                        )
+                    )
bindings/python/examples/gpiomon.py new file +28 -0
new file mode 100755
index 0000000..e0db16f
@@ -0,0 +1,28 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpiomon tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Edge
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError("usage: gpiomon.py <gpiochip> <offset1> <offset2> ...")
+
+    path = sys.argv[1]
+    lines = []
+    for line in sys.argv[2:]:
+        lines.append(int(line) if line.isdigit() else line)
+
+    with gpiod.request_lines(
+        path,
+        consumer="gpiomon.py",
+        config={tuple(lines): gpiod.LineSettings(edge_detection=Edge.BOTH)},
+    ) as request:
+        while True:
+            for event in request.read_edge_event():
+                print(event)
bindings/python/examples/gpioset.py new file +37 -0
new file mode 100755
index 0000000..f0b0681
@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioset tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Direction, Value
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError(
+            "usage: gpioset.py <gpiochip> <offset1>=<value1> <offset2>=<value2> ..."
+        )
+
+    path = sys.argv[1]
+    values = dict()
+    lines = []
+    for arg in sys.argv[2:]:
+        arg = arg.split("=")
+        key = int(arg[0]) if arg[0].isdigit() else arg[0]
+        val = int(arg[1])
+
+        lines.append(key)
+        values[key] = Value(val)
+
+    request = gpiod.request_lines(
+        path,
+        consumer="gpioset.py",
+        config={tuple(lines): gpiod.LineSettings(direction=Direction.OUTPUT)},
+    )
+
+    vals = request.set_values(values)
+
+    input()
Signature · 1 line
2.34.1
#3 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 45 lines · Bartosz Golaszewski

This adds the regular set of example programs implemented using libgpiod python bindings.

---
 bindings/python/examples/Makefile.am   | 10 +++++++
 bindings/python/examples/gpiodetect.py | 17 ++++++++++++
 bindings/python/examples/gpiofind.py   | 20 ++++++++++++++
 bindings/python/examples/gpioget.py    | 31 +++++++++++++++++++++
 bindings/python/examples/gpioinfo.py   | 35 ++++++++++++++++++++++++
 bindings/python/examples/gpiomon.py    | 28 +++++++++++++++++++
 bindings/python/examples/gpioset.py    | 37 ++++++++++++++++++++++++++
 7 files changed, 178 insertions(+)
 create mode 100644 bindings/python/examples/Makefile.am
 create mode 100755 bindings/python/examples/gpiodetect.py
 create mode 100755 bindings/python/examples/gpiofind.py
 create mode 100755 bindings/python/examples/gpioget.py
 create mode 100755 bindings/python/examples/gpioinfo.py
 create mode 100755 bindings/python/examples/gpiomon.py
 create mode 100755 bindings/python/examples/gpioset.py
bindings/python/examples/Makefile.am new file +10 -0
new file mode 100644
index 0000000..f42b80e
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+EXTRA_DIST = \
+	gpiodetect.py \
+	gpiofind.py \
+	gpioget.py \
+	gpioinfo.py \
+	gpiomon.py \
+	gpioset.py
bindings/python/examples/gpiodetect.py new file +2 -0
new file mode 100755
index 0000000..c32014f
@@ -0,0 +1,17 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later

bindings/python/examples/gpiodetect.py line 12

Quoted 10 lines
bindings/python/examples/gpiodetect.py new file +10 -0
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Reimplementation of the gpiodetect tool in Python."""
+
+import gpiod
+import os
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):

Add a helper generator function that returns the available chip paths? And in order might be nice too.

Quoted 16 lines
bindings/python/examples/gpiodetect.py new file +5 -0
+            with gpiod.Chip(entry.path) as chip:
+                info = chip.get_info()
+                print(
+                    "{} [{}] ({} lines)".format(info.name, info.label, info.num_lines)
+                )
bindings/python/examples/gpiofind.py new file +5 -0
new file mode 100755
index 0000000..2f30445
@@ -0,0 +1,20 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Reimplementation of the gpiofind tool in Python."""

bindings/python/examples/gpiofind.py line 15

Quoted 10 lines
bindings/python/examples/gpiofind.py new file +10 -0
+
+import gpiod
+import os
+import sys
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                offset = chip.map_line(sys.argv[1])
                            chip.offset_from_id(...
Quoted 21 lines
bindings/python/examples/gpiofind.py new file +5 -0
+                if offset is not None:
+                    print("{} {}".format(chip.get_info().name, offset))
+                    sys.exit(0)
+
+    sys.exit(1)
bindings/python/examples/gpioget.py new file +10 -0
new file mode 100755
index 0000000..d441535
@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioget tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Direction

bindings/python/examples/gpioget.py line 20

Quoted 10 lines
bindings/python/examples/gpioget.py new file +10 -0
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError("usage: gpioget.py <gpiochip> <offset1> <offset2> ...")
+
+    path = sys.argv[1]
+    lines = []
+    for line in sys.argv[2:]:
+        lines.append(int(line) if line.isdigit() else line)
+

Just leave the line ids as string?

else use a list comprehension:

lines = [ int(id) if id.isdigit() else id for id in sys.argv[2:] ]

Similarly elsewhere.

Quoted 29 lines
bindings/python/examples/gpioget.py new file +11 -0
+    request = gpiod.request_lines(
+        path,
+        consumer="gpioget.py",
+        config={tuple(lines): gpiod.LineSettings(direction=Direction.INPUT)},
+    )
+
+    vals = request.get_values()
+
+    for val in vals:
+        print("{} ".format(val.value), end="")
+    print()
bindings/python/examples/gpioinfo.py new file +12 -0
new file mode 100755
index 0000000..e8c7d46
@@ -0,0 +1,35 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioinfo tool in Python."""
+
+import gpiod
+import os
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):

bindings/python/examples/gpioinfo.py line 22

Quoted 10 lines
bindings/python/examples/gpioinfo.py new file +10 -0
+            with gpiod.Chip(entry.path) as chip:
+                cinfo = chip.get_info()
+                print("{} - {} lines:".format(cinfo.name, cinfo.num_lines))
+
+                for offset in range(0, cinfo.num_lines):
+                    linfo = chip.get_line_info(offset)
+                    offset = linfo.offset
+                    name = linfo.name
+                    consumer = linfo.consumer
+                    direction = linfo.direction
                    is_input = linfo.direction == gpiod.line.Direction.INPUT

That is for space saving below.
Drop the others as they are only referenced once (if you follow the suggestion below).

bindings/python/examples/gpioinfo.py line 33

Quoted 11 lines
bindings/python/examples/gpioinfo.py new file +11 -0
+                    active_low = linfo.active_low
+
+                    print(
+                        "\tline {:>3}: {:>18} {:>12} {:>8} {:>10}".format(
+                            offset,
+                            "unnamed" if name is None else name,
+                            "unused" if consumer is None else consumer,
+                            "input"
+                            if direction == gpiod.line.Direction.INPUT
+                            else "output",
+                            "active-low" if active_low else "active-high",

linfo.offset,
linfo.name or "unnamed",
linfo.consumer or "unused",
is_input and "input" or "output", linfo.active_low and "active_low" or "active-high",

Quoted 60 lines
bindings/python/examples/gpioinfo.py new file +2 -0
+                        )
+                    )
bindings/python/examples/gpiomon.py new file +28 -0
new file mode 100755
index 0000000..e0db16f
@@ -0,0 +1,28 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpiomon tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Edge
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError("usage: gpiomon.py <gpiochip> <offset1> <offset2> ...")
+
+    path = sys.argv[1]
+    lines = []
+    for line in sys.argv[2:]:
+        lines.append(int(line) if line.isdigit() else line)
+
+    with gpiod.request_lines(
+        path,
+        consumer="gpiomon.py",
+        config={tuple(lines): gpiod.LineSettings(edge_detection=Edge.BOTH)},
+    ) as request:
+        while True:
+            for event in request.read_edge_event():
+                print(event)
bindings/python/examples/gpioset.py new file +18 -0
new file mode 100755
index 0000000..f0b0681
@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioset tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Direction, Value
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError(
+            "usage: gpioset.py <gpiochip> <offset1>=<value1> <offset2>=<value2> ..."
+        )
+
+    path = sys.argv[1]

bindings/python/examples/gpioset.py line 28

Quoted 10 lines
bindings/python/examples/gpioset.py new file +10 -0
+    values = dict()
+    lines = []
+    for arg in sys.argv[2:]:
+        arg = arg.split("=")
+        key = int(arg[0]) if arg[0].isdigit() else arg[0]
+        val = int(arg[1])
+
+        lines.append(key)
+        values[key] = Value(val)
+

lvs = [ arg.split('=') for arg in sys.argv[2:] ] lines = [ x[0] for x in lvs ]
values = dict[lvs]

bindings/python/examples/gpioset.py line 37

Quoted 10 lines
bindings/python/examples/gpioset.py new file +9 -0
+    request = gpiod.request_lines(
+        path,
+        consumer="gpioset.py",
+        config={tuple(lines): gpiod.LineSettings(direction=Direction.OUTPUT)},
+    )
+
+    vals = request.set_values(values)
+
+    input()

No gpiowatch?

Add some examples for features the tools don't use, like requests with both inputs and outputs, and reconfigure?

Cheers,
Kent.

#4 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 259 lines · Kent Gibson
Quoted 45 lines · Bartosz Golaszewski

This adds the regular set of example programs implemented using libgpiod python bindings.

---
 bindings/python/examples/Makefile.am   | 10 +++++++
 bindings/python/examples/gpiodetect.py | 17 ++++++++++++
 bindings/python/examples/gpiofind.py   | 20 ++++++++++++++
 bindings/python/examples/gpioget.py    | 31 +++++++++++++++++++++
 bindings/python/examples/gpioinfo.py   | 35 ++++++++++++++++++++++++
 bindings/python/examples/gpiomon.py    | 28 +++++++++++++++++++
 bindings/python/examples/gpioset.py    | 37 ++++++++++++++++++++++++++
 7 files changed, 178 insertions(+)
 create mode 100644 bindings/python/examples/Makefile.am
 create mode 100755 bindings/python/examples/gpiodetect.py
 create mode 100755 bindings/python/examples/gpiofind.py
 create mode 100755 bindings/python/examples/gpioget.py
 create mode 100755 bindings/python/examples/gpioinfo.py
 create mode 100755 bindings/python/examples/gpiomon.py
 create mode 100755 bindings/python/examples/gpioset.py
bindings/python/examples/Makefile.am new file +10 -0
new file mode 100644
index 0000000..f42b80e
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+EXTRA_DIST = \
+     gpiodetect.py \
+     gpiofind.py \
+     gpioget.py \
+     gpioinfo.py \
+     gpiomon.py \
+     gpioset.py
bindings/python/examples/gpiodetect.py new file +2 -0
new file mode 100755
index 0000000..c32014f
@@ -0,0 +1,17 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later

bindings/python/examples/gpiodetect.py line 12

Quoted 10 lines
bindings/python/examples/gpiodetect.py new file +10 -0
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Reimplementation of the gpiodetect tool in Python."""
+
+import gpiod
+import os
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):

Add a helper generator function that returns the available chip paths? And in order might be nice too.

Quoted 16 lines
bindings/python/examples/gpiodetect.py new file +5 -0
+            with gpiod.Chip(entry.path) as chip:
+                info = chip.get_info()
+                print(
+                    "{} [{}] ({} lines)".format(info.name, info.label, info.num_lines)
+                )
bindings/python/examples/gpiofind.py new file +5 -0
new file mode 100755
index 0000000..2f30445
@@ -0,0 +1,20 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Reimplementation of the gpiofind tool in Python."""

bindings/python/examples/gpiofind.py line 15

Quoted 10 lines
bindings/python/examples/gpiofind.py new file +10 -0
+
+import gpiod
+import os
+import sys
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                offset = chip.map_line(sys.argv[1])
                            chip.offset_from_id(...
Quoted 21 lines
bindings/python/examples/gpiofind.py new file +5 -0
+                if offset is not None:
+                    print("{} {}".format(chip.get_info().name, offset))
+                    sys.exit(0)
+
+    sys.exit(1)
bindings/python/examples/gpioget.py new file +10 -0
new file mode 100755
index 0000000..d441535
@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioget tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Direction

bindings/python/examples/gpioget.py line 20

Quoted 10 lines
bindings/python/examples/gpioget.py new file +10 -0
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError("usage: gpioget.py <gpiochip> <offset1> <offset2> ...")
+
+    path = sys.argv[1]
+    lines = []
+    for line in sys.argv[2:]:
+        lines.append(int(line) if line.isdigit() else line)
+

Just leave the line ids as string?

else use a list comprehension:

lines = [ int(id) if id.isdigit() else id for id in sys.argv[2:] ]

Similarly elsewhere.

Quoted 29 lines
bindings/python/examples/gpioget.py new file +11 -0
+    request = gpiod.request_lines(
+        path,
+        consumer="gpioget.py",
+        config={tuple(lines): gpiod.LineSettings(direction=Direction.INPUT)},
+    )
+
+    vals = request.get_values()
+
+    for val in vals:
+        print("{} ".format(val.value), end="")
+    print()
bindings/python/examples/gpioinfo.py new file +12 -0
new file mode 100755
index 0000000..e8c7d46
@@ -0,0 +1,35 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioinfo tool in Python."""
+
+import gpiod
+import os
+
+if __name__ == "__main__":
+    for entry in os.scandir("/dev/"):
+        if gpiod.is_gpiochip_device(entry.path):

bindings/python/examples/gpioinfo.py line 22

Quoted 10 lines
bindings/python/examples/gpioinfo.py new file +10 -0
+            with gpiod.Chip(entry.path) as chip:
+                cinfo = chip.get_info()
+                print("{} - {} lines:".format(cinfo.name, cinfo.num_lines))
+
+                for offset in range(0, cinfo.num_lines):
+                    linfo = chip.get_line_info(offset)
+                    offset = linfo.offset
+                    name = linfo.name
+                    consumer = linfo.consumer
+                    direction = linfo.direction
                    is_input = linfo.direction == gpiod.line.Direction.INPUT

That is for space saving below.
Drop the others as they are only referenced once (if you follow the suggestion below).

bindings/python/examples/gpioinfo.py line 33

Quoted 11 lines
bindings/python/examples/gpioinfo.py new file +11 -0
+                    active_low = linfo.active_low
+
+                    print(
+                        "\tline {:>3}: {:>18} {:>12} {:>8} {:>10}".format(
+                            offset,
+                            "unnamed" if name is None else name,
+                            "unused" if consumer is None else consumer,
+                            "input"
+                            if direction == gpiod.line.Direction.INPUT
+                            else "output",
+                            "active-low" if active_low else "active-high",

linfo.offset,
linfo.name or "unnamed",
linfo.consumer or "unused",
is_input and "input" or "output", linfo.active_low and "active_low" or "active-high",

Quoted 66 lines
bindings/python/examples/gpioinfo.py new file +2 -0
+                        )
+                    )
bindings/python/examples/gpiomon.py new file +28 -0
new file mode 100755
index 0000000..e0db16f
@@ -0,0 +1,28 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpiomon tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Edge
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError("usage: gpiomon.py <gpiochip> <offset1> <offset2> ...")
+
+    path = sys.argv[1]
+    lines = []
+    for line in sys.argv[2:]:
+        lines.append(int(line) if line.isdigit() else line)
+
+    with gpiod.request_lines(
+        path,
+        consumer="gpiomon.py",
+        config={tuple(lines): gpiod.LineSettings(edge_detection=Edge.BOTH)},
+    ) as request:
+        while True:
+            for event in request.read_edge_event():
+                print(event)
bindings/python/examples/gpioset.py new file +24 -0
new file mode 100755
index 0000000..f0b0681
@@ -0,0 +1,37 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+"""Simplified reimplementation of the gpioset tool in Python."""
+
+import gpiod
+import sys
+
+from gpiod.line import Direction, Value
+
+if __name__ == "__main__":
+    if len(sys.argv) < 3:
+        raise TypeError(
+            "usage: gpioset.py <gpiochip> <offset1>=<value1> <offset2>=<value2> ..."
+        )
+
+    path = sys.argv[1]
+    values = dict()
+    lines = []
+    for arg in sys.argv[2:]:
+        arg = arg.split("=")
+        key = int(arg[0]) if arg[0].isdigit() else arg[0]
+        val = int(arg[1])
Quoted 8 lines

bindings/python/examples/gpioset.py line 28

Quoted 4 lines
bindings/python/examples/gpioset.py new file +4 -0
+
+        lines.append(key)
+        values[key] = Value(val)
+

lvs = [ arg.split('=') for arg in sys.argv[2:] ] lines = [ x[0] for x in lvs ]
values = dict[lvs]

It must be dict(lvs) but even then values are still strings, not their enum integer representations, so we still need to convert them to ints before passing them to set_values(). I'd leave it as it is now.

Quoted 14 lines

bindings/python/examples/gpioset.py

Quoted 11 lines
bindings/python/examples/gpioset.py new file +9 -1
+    request = gpiod.request_lines(
+        path,
+        consumer="gpioset.py",
+        config={tuple(lines): gpiod.LineSettings(direction=Direction.OUTPUT)},
+    )
+
+    vals = request.set_values(values)
+
+    input()
--

No gpiowatch?

There's no gpiowatch yet in the main set of tools, I'll add them once they land.

Quoted 3 lines

Add some examples for features the tools don't use, like requests with both inputs and outputs, and reconfigure?

These are already covered in tests. I may add them in the future but don't want to add a lot of code that's not routinely run as part of tests.

Bart

Quoted 2 lines

Cheers,
Kent.

#5 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 6 lines · Bartosz Golaszewski
Quoted 5 lines · Kent Gibson
Quoted 3 lines · Bartosz Golaszewski

This adds the regular set of example programs implemented using libgpiod python bindings.

Quoted 14 lines
Quoted 6 lines
Quoted 6 lines
bindings/python/examples/gpioset.py new file +5 -0
+    path = sys.argv[1]
+    values = dict()
+    lines = []
+    for arg in sys.argv[2:]:
+        arg = arg.split("=")
Quoted 8 lines

bindings/python/examples/gpioset.py line 28

Quoted 6 lines
bindings/python/examples/gpioset.py new file +6 -0
+        key = int(arg[0]) if arg[0].isdigit() else arg[0]
+        val = int(arg[1])
+
+        lines.append(key)
+        values[key] = Value(val)
+
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.
You could do it in one with a more appropriate parser function.

Cheers,
Kent.

#6 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 20 lines · Kent Gibson
Quoted 5 lines · Bartosz Golaszewski
Quoted 4 lines · Kent Gibson
Quoted 3 lines · Bartosz Golaszewski

This adds the regular set of example programs implemented using libgpiod python bindings.

Quoted 14 lines
Quoted 6 lines
Quoted 6 lines
bindings/python/examples/gpioset.py new file +5 -0
+    path = sys.argv[1]
+    values = dict()
+    lines = []
+    for arg in sys.argv[2:]:
+        arg = arg.split("=")
Quoted 8 lines

bindings/python/examples/gpioset.py line 28

Quoted 6 lines
bindings/python/examples/gpioset.py new file +6 -0
+        key = int(arg[0]) if arg[0].isdigit() else arg[0]
+        val = int(arg[1])
+
+        lines.append(key)
+        values[key] = Value(val)
+
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
Quoted 6 lines
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.

In Python we have map(), which I think is the best for that kind of job.

Quoted 1 line

You could do it in one with a more appropriate parser function.

It seems we need some Python guru to revisit the code, because to me it looks a bit C:ish :-)

Maybe I can ask colleague of mine, if he has time for a such... No guarantees, though.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#7 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 21 lines · Andy Shevchenko
Quoted 20 lines · Kent Gibson
Quoted 19 lines · Bartosz Golaszewski
Quoted 18 lines · Kent Gibson
Quoted 5 lines · Bartosz Golaszewski

This adds the regular set of example programs implemented using libgpiod python bindings.

bindings/python/examples/gpioset.py new file +1 -0
+    path = sys.argv[1]

bindings/python/examples/gpioset.py line 28

Quoted 10 lines
bindings/python/examples/gpioset.py new file +10 -0
+    values = dict()
+    lines = []
+    for arg in sys.argv[2:]:
+        arg = arg.split("=")
+        key = int(arg[0]) if arg[0].isdigit() else arg[0]
+        val = int(arg[1])
+
+        lines.append(key)
+        values[key] = Value(val)
+
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
Quoted 9 lines
Quoted 6 lines
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.

In Python we have map(), which I think is the best for that kind of job.

My understanding is map/filter is old school and list comprehensions have replaced map, as generators have replaced filter.

i.e.
list(map(function, iterable))
becomes
[function(x) for x in iterable]

Either way, what we are missing here is a parser function that gives us exactly the (offset,value) output we want from the command line string.

Oh, and we need both the lines list and the values dict, both of which are easily created from the interim lvs.

Quoted 5 lines
Quoted 1 line

You could do it in one with a more appropriate parser function.

It seems we need some Python guru to revisit the code, because to me it looks a bit C:ish :-)

The for loop or the list comprehension?
Last I checked only one of those is available in C.
And yeah, the for loop version reads as C, so not at all Pythonic, which is why I suggested the list comprehension.

Quoted 3 lines

Maybe I can ask colleague of mine, if he has time for a such... No guarantees, though.

Maybe I'm wrong and they've flipped back to map/filter. Stanger things have happened.

Cheers,
Kent.

#8 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 5 lines · Kent Gibson
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Kent Gibson
Quoted 2 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

On Fri, Oct 07, 2022 at 04:55:19PM +0200, Bartosz Golaszewski wrote:

...

Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.

In Python we have map(), which I think is the best for that kind of job.

Quoted 7 lines

My understanding is map/filter is old school and list comprehensions have replaced map, as generators have replaced filter.

i.e.
list(map(function, iterable))
becomes
[function(x) for x in iterable]

Definitely it does not cover all the cases map() is taking care of. So it can't be old school :-)

* Yes, in this particular case it may be map() or list comprehension. But I think with map() the two lines can become one.

Quoted 8 lines

Either way, what we are missing here is a parser function that gives us exactly the (offset,value) output we want from the command line string.

Oh, and we need both the lines list and the values dict, both of which are easily created from the interim lvs.

Quoted 2 lines
Quoted 1 line

You could do it in one with a more appropriate parser function.

Quoted 7 lines
Quoted 2 lines

It seems we need some Python guru to revisit the code, because to me it looks a bit C:ish :-)

The for loop or the list comprehension?
Last I checked only one of those is available in C.
And yeah, the for loop version reads as C, so not at all Pythonic, which is why I suggested the list comprehension.

Yes, but I believe it does not utilize the powerfulness of the current Python. Anyway, I'm not a Py guru, take my remarks with a grain of salt.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#9 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 46 lines · Andy Shevchenko
Quoted 5 lines · Kent Gibson
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Kent Gibson
Quoted 2 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

On Fri, Oct 07, 2022 at 04:55:19PM +0200, Bartosz Golaszewski wrote:

...

Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.

In Python we have map(), which I think is the best for that kind of job.

Quoted 7 lines

My understanding is map/filter is old school and list comprehensions have replaced map, as generators have replaced filter.

i.e.
list(map(function, iterable))
becomes
[function(x) for x in iterable]

Definitely it does not cover all the cases map() is taking care of. So it can't be old school :-)

* Yes, in this particular case it may be map() or list comprehension. But I think with map() the two lines can become one.

Quoted 11 lines

Either way, what we are missing here is a parser function that gives us exactly the (offset,value) output we want from the command line string.

Oh, and we need both the lines list and the values dict, both of which are easily created from the interim lvs.

Quoted 4 lines
Quoted 1 line

You could do it in one with a more appropriate parser function.

It seems we need some Python guru to revisit the code, because to me it looks a bit C:ish :-)

Quoted 8 lines
Quoted 4 lines

The for loop or the list comprehension?
Last I checked only one of those is available in C.
And yeah, the for loop version reads as C, so not at all Pythonic, which is why I suggested the list comprehension.

Yes, but I believe it does not utilize the powerfulness of the current Python. Anyway, I'm not a Py guru, take my remarks with a grain of salt.

How about this?

    lvs = list(
        map(
            lambda val: [val[0], Value(int(val[1]))],
            [arg.split("=") for arg in sys.argv[2:]],
        )
    )
    lines = [x[0] for x in lvs]
    values = dict(lvs)

It's so much less readable but at least it's pythonic, look at those lambdas and comprehension lists and even a map! :)

Anyway - unlike the programming interface - these are just implementation details that can be always improved later.

Bart

#10 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 57 lines · Bartosz Golaszewski
Quoted 46 lines · Andy Shevchenko
Quoted 5 lines · Kent Gibson
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Kent Gibson
Quoted 2 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

On Fri, Oct 07, 2022 at 04:55:19PM +0200, Bartosz Golaszewski wrote:

...

Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.

In Python we have map(), which I think is the best for that kind of job.

Quoted 7 lines

My understanding is map/filter is old school and list comprehensions have replaced map, as generators have replaced filter.

i.e.
list(map(function, iterable))
becomes
[function(x) for x in iterable]

Definitely it does not cover all the cases map() is taking care of. So it can't be old school :-)

* Yes, in this particular case it may be map() or list comprehension. But I think with map() the two lines can become one.

Quoted 11 lines

Either way, what we are missing here is a parser function that gives us exactly the (offset,value) output we want from the command line string.

Oh, and we need both the lines list and the values dict, both of which are easily created from the interim lvs.

Quoted 4 lines
Quoted 1 line

You could do it in one with a more appropriate parser function.

It seems we need some Python guru to revisit the code, because to me it looks a bit C:ish :-)

Quoted 8 lines
Quoted 4 lines

The for loop or the list comprehension?
Last I checked only one of those is available in C.
And yeah, the for loop version reads as C, so not at all Pythonic, which is why I suggested the list comprehension.

Yes, but I believe it does not utilize the powerfulness of the current Python. Anyway, I'm not a Py guru, take my remarks with a grain of salt.

Quoted 8 lines

How about this?

    lvs = list(
        map(
            lambda val: [val[0], Value(int(val[1]))],
            [arg.split("=") for arg in sys.argv[2:]],
        )
    )

which is the same as

    lvs = [ (x,Value(int(y))) for (x,y) in [ arg.split("=") for arg in sys.argv[2:]] ]

which is the same as my two liner, just nested - though it may only iterate through the list once if the inner list comprehension is treated as a generator. Not sure.

Either way, not too fussed - it is only example code. As long as it isn't a for loop ;-).

Cheers,
Kent.

Quoted 10 lines

lines = [x[0] for x in lvs]
values = dict(lvs)

It's so much less readable but at least it's pythonic, look at those lambdas and comprehension lists and even a map! :)

Anyway - unlike the programming interface - these are just implementation details that can be always improved later.

Bart

#11 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 69 lines · Kent Gibson
Quoted 57 lines · Bartosz Golaszewski
Quoted 46 lines · Andy Shevchenko
Quoted 5 lines · Kent Gibson
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Kent Gibson
Quoted 2 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

On Fri, Oct 07, 2022 at 04:55:19PM +0200, Bartosz Golaszewski wrote:

...

Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
Quoted 1 line
Quoted 1 line
        lvs = [ arg.split('=') for arg in sys.argv[2:] ]
            lvs = [ (x,int(y)) for (x,y) in lvs ]
Quoted 3 lines
Quoted 2 lines

lines = [ x[0] for x in lvs ]
values = dict(lvs)

An extra pass to fix the int values.

In Python we have map(), which I think is the best for that kind of job.

Quoted 7 lines

My understanding is map/filter is old school and list comprehensions have replaced map, as generators have replaced filter.

i.e.
list(map(function, iterable))
becomes
[function(x) for x in iterable]

Definitely it does not cover all the cases map() is taking care of. So it can't be old school :-)

* Yes, in this particular case it may be map() or list comprehension. But I think with map() the two lines can become one.

Quoted 11 lines

Either way, what we are missing here is a parser function that gives us exactly the (offset,value) output we want from the command line string.

Oh, and we need both the lines list and the values dict, both of which are easily created from the interim lvs.

Quoted 4 lines
Quoted 1 line

You could do it in one with a more appropriate parser function.

It seems we need some Python guru to revisit the code, because to me it looks a bit C:ish :-)

Quoted 8 lines
Quoted 4 lines

The for loop or the list comprehension?
Last I checked only one of those is available in C.
And yeah, the for loop version reads as C, so not at all Pythonic, which is why I suggested the list comprehension.

Yes, but I believe it does not utilize the powerfulness of the current Python. Anyway, I'm not a Py guru, take my remarks with a grain of salt.

Quoted 8 lines

How about this?

    lvs = list(
        map(
            lambda val: [val[0], Value(int(val[1]))],
            [arg.split("=") for arg in sys.argv[2:]],
        )
    )

which is the same as

Quoted 9 lines
    lvs = [ (x,Value(int(y))) for (x,y) in [ arg.split("=") for arg in sys.argv[2:]] ]

which is the same as my two liner, just nested - though it may only iterate through the list once if the inner list comprehension is treated as a generator. Not sure.

Either way, not too fussed - it is only example code. As long as it isn't a for loop ;-).

Oh, btw, the parser fn version would be something like:

def parse_value(arg):

     (x,y) = arg.split("=")
     return (x, Value(int(y)))

lvs = [ parse_value(arg) for arg in sys.argv[2:]
...

Is that clearer?

Cheers,
Kent.

#12 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 1 line · Kent Gibson

On Tue, Oct 18, 2022 at 12:09:10AM +0800, Kent Gibson wrote:

...

Quoted 5 lines

Oh, btw, the parser fn version would be something like:

def parse_value(arg):

     (x,y) = arg.split("=")
     return (x, Value(int(y)))

Not a lisp, no need for too many parentheses. Also, we could use splices:

	eqidx = arg.index('=')
	return arg[:eqidx], Value(int(arg[eqidx + 1:]))

or with split()

	l, v = arg.split('=')
	return l, Value(int(v))
Quoted 1 line

lvs = [ parse_value(arg) for arg in sys.argv[2:]

Dunno why you put spaces inside outer [], but okay.

Quoted 1 line

Is that clearer?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#13 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Kent Gibson

On Tue, Oct 18, 2022 at 12:09:10AM +0800, Kent Gibson wrote:

...

Quoted 11 lines
Quoted 5 lines

Oh, btw, the parser fn version would be something like:

def parse_value(arg):

     (x,y) = arg.split("=")
     return (x, Value(int(y)))

Not a lisp, no need for too many parentheses. Also, we could use splices:

	eqidx = arg.index('=')
	return arg[:eqidx], Value(int(arg[eqidx + 1:]))
Quoted 8 lines

or with split()

	l, v = arg.split('=')
	return l, Value(int(v))
Quoted 1 line

lvs = [ parse_value(arg) for arg in sys.argv[2:]

Dunno why you put spaces inside outer [], but okay.

and this actually can be directly put to the dict constructor:

values = OrderedDict(parse_value(arg) for arg in sys.argv[2:])

This looks short enough and readable.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#14 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 25 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
Quoted 1 line · Kent Gibson

On Tue, Oct 18, 2022 at 12:09:10AM +0800, Kent Gibson wrote:

...

Quoted 18 lines
Quoted 5 lines

Oh, btw, the parser fn version would be something like:

def parse_value(arg):

     (x,y) = arg.split("=")
     return (x, Value(int(y)))

Not a lisp, no need for too many parentheses. Also, we could use splices:

      eqidx = arg.index('=')
      return arg[:eqidx], Value(int(arg[eqidx + 1:]))

or with split()

l, v = arg.split('=')
return l, Value(int(v))

Quoted 1 line

lvs = [ parse_value(arg) for arg in sys.argv[2:]

Quoted 8 lines
Quoted 1 line

Dunno why you put spaces inside outer [], but okay.

and this actually can be directly put to the dict constructor:

values = OrderedDict(parse_value(arg) for arg in sys.argv[2:])

This looks short enough and readable.

Let's stop bikeshedding, I'll move on to stuff that really matters now. :)

Bartosz

#15 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

In reply to Bartosz Golaszewski
Quoted 2 lines · Bartosz Golaszewski

On Mon, Oct 17, 2022 at 4:19 PM Andy Shevchenko
<[email protected]> wrote:

...

Quoted 8 lines

How about this?

    lvs = list(
        map(
            lambda val: [val[0], Value(int(val[1]))],
            [arg.split("=") for arg in sys.argv[2:]],
        )
    )

Yeah, this looks ugly... So initial variant with two lines looks to me like this:

lvs = [arg.split("=") for arg in sys.argv[2:]] # btw, needs handling 2 exceptions values = dict((x, Value(int(y))) for (x,y) in lvs) # needs to handle an exception # Perhaps you need ordered dict?
lines = values.keys()

Quoted 5 lines

lines = [x[0] for x in lvs]
values = dict(lvs)

It's so much less readable but at least it's pythonic, look at those lambdas and comprehension lists and even a map! :)

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#16 lore raw

Re: [libgpiod v2][PATCH v3 2/4] bindings: python: add examples

Quoted 15 lines · Andy Shevchenko
Quoted 2 lines · Bartosz Golaszewski

On Mon, Oct 17, 2022 at 4:19 PM Andy Shevchenko
<[email protected]> wrote:

...

Quoted 8 lines

How about this?

    lvs = list(
        map(
            lambda val: [val[0], Value(int(val[1]))],
            [arg.split("=") for arg in sys.argv[2:]],
        )
    )
Quoted 8 lines

Yeah, this looks ugly... So initial variant with two lines looks to me like this:

lvs = [arg.split("=") for arg in sys.argv[2:]] # btw, needs handling 2 exceptions values = dict((x, Value(int(y))) for (x,y) in lvs) # needs to handle an exception # Perhaps you need ordered dict?
lines = values.keys()

Indeed, an OrderedDict keys would provide the lines in argv order, so values.keys() could be used in place of lines.

And if you use a parser function then it can deal with the parsing exceptions.

values = OrderedDict([ parse(arg) for args in sys.argv[2:] ])

Cheers,
Kent.

Quoted 12 lines
Quoted 5 lines

lines = [x[0] for x in lvs]
values = dict(lvs)

It's so much less readable but at least it's pythonic, look at those lambdas and comprehension lists and even a map! :)

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#17 lore raw

PATCH v3 3/4 bindings: python: add tests

In reply to Bartosz Golaszewski

This adds a test-suite for python bindings based on the gpio-sim kernel module.

---
 bindings/python/tests/Makefile.am            |  17 +
 bindings/python/tests/__init__.py            |  17 +
 bindings/python/tests/__main__.py            |  16 +
 bindings/python/tests/gpiosim/Makefile.am    |   7 +
 bindings/python/tests/gpiosim/__init__.py    |   4 +
 bindings/python/tests/gpiosim/chip.py        |  66 +++
 bindings/python/tests/gpiosim/ext.c          | 345 ++++++++++++++
 bindings/python/tests/helpers.py             |  16 +
 bindings/python/tests/tests_chip.py          | 231 ++++++++++
 bindings/python/tests/tests_chip_info.py     |  52 +++
 bindings/python/tests/tests_edge_event.py    | 219 +++++++++
 bindings/python/tests/tests_info_event.py    | 189 ++++++++
 bindings/python/tests/tests_line_info.py     | 101 +++++
 bindings/python/tests/tests_line_request.py  | 449 +++++++++++++++++++
 bindings/python/tests/tests_line_settings.py |  79 ++++
 bindings/python/tests/tests_module.py        |  59 +++
 16 files changed, 1867 insertions(+)
 create mode 100644 bindings/python/tests/Makefile.am
 create mode 100644 bindings/python/tests/__init__.py
 create mode 100644 bindings/python/tests/__main__.py
 create mode 100644 bindings/python/tests/gpiosim/Makefile.am
 create mode 100644 bindings/python/tests/gpiosim/__init__.py
 create mode 100644 bindings/python/tests/gpiosim/chip.py
 create mode 100644 bindings/python/tests/gpiosim/ext.c
 create mode 100644 bindings/python/tests/helpers.py
 create mode 100644 bindings/python/tests/tests_chip.py
 create mode 100644 bindings/python/tests/tests_chip_info.py
 create mode 100644 bindings/python/tests/tests_edge_event.py
 create mode 100644 bindings/python/tests/tests_info_event.py
 create mode 100644 bindings/python/tests/tests_line_info.py
 create mode 100644 bindings/python/tests/tests_line_request.py
 create mode 100644 bindings/python/tests/tests_line_settings.py
 create mode 100644 bindings/python/tests/tests_module.py
bindings/python/tests/Makefile.am new file +17 -0
new file mode 100644
index 0000000..7dcdebb
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+SUBDIRS = gpiosim
+
+EXTRA_DIST = \
+	helpers.py \
+	__init__.py \
+	__main__.py \
+	tests_chip_info.py \
+	tests_chip.py \
+	tests_edge_event.py \
+	tests_info_event.py \
+	tests_line_info.py \
+	tests_line_request.py \
+	tests_line_settings.py \
+	tests_module.py
bindings/python/tests/__init__.py new file +17 -0
new file mode 100644
index 0000000..2bf14e6
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+import os
+import unittest
+
+from packaging import version
+
+required_kernel_version = "5.19.0"
+current_version = os.uname().release.split("-")[0]
+
+if version.parse(current_version) < version.parse(required_kernel_version):
+    raise NotImplementedError(
+        "linux kernel version must be at least {} - got {}".format(
+            required_kernel_version, current_version
+        )
+    )
bindings/python/tests/__main__.py new file +16 -0
new file mode 100644
index 0000000..b5d7f0a
@@ -0,0 +1,16 @@
+#!/usr/bin/python3
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+import unittest
+
+from .tests_chip import *
+from .tests_chip_info import *
+from .tests_edge_event import *
+from .tests_info_event import *
+from .tests_line_info import *
+from .tests_line_settings import *
+from .tests_module import *
+from .tests_line_request import *
+
+unittest.main()
bindings/python/tests/gpiosim/Makefile.am new file +7 -0
new file mode 100644
index 0000000..7004f3a
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+EXTRA_DIST = \
+	chip.py \
+	ext.c \
+	__init__.py
bindings/python/tests/gpiosim/__init__.py new file +4 -0
new file mode 100644
index 0000000..f65e413
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from .chip import Chip
bindings/python/tests/gpiosim/chip.py new file +66 -0

66 lines not on this page · raw message

bindings/python/tests/gpiosim/ext.c new file +345 -0

345 lines not on this page · raw message

bindings/python/tests/helpers.py new file +16 -0
new file mode 100644
index 0000000..f9a15e8
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+import os
+
+
+class LinkGuard:
+    def __init__(self, src, dst):
+        self.src = src
+        self.dst = dst
+
+    def __enter__(self):
+        os.symlink(self.src, self.dst)
+
+    def __exit__(self, type, val, tb):
+        os.unlink(self.dst)
bindings/python/tests/tests_chip.py new file +231 -0

231 lines not on this page · raw message

bindings/python/tests/tests_chip_info.py new file +52 -0

52 lines not on this page · raw message

bindings/python/tests/tests_edge_event.py new file +219 -0

219 lines not on this page · raw message

bindings/python/tests/tests_info_event.py new file +189 -0

189 lines not on this page · raw message

bindings/python/tests/tests_line_info.py new file +101 -0

101 lines not on this page · raw message

bindings/python/tests/tests_line_request.py new file +449 -0

449 lines not on this page · raw message

bindings/python/tests/tests_line_settings.py new file +79 -0

79 lines not on this page · raw message

bindings/python/tests/tests_module.py new file +59 -0

59 lines not on this page · raw message

Signature · 1 line
2.34.1
#18 lore raw

Re: [libgpiod v2][PATCH v3 3/4] bindings: python: add tests

Quoted 4 lines · Bartosz Golaszewski

This adds a test-suite for python bindings based on the gpio-sim kernel module.

<snip>

bindings/python/tests/gpiosim/ext.c line 342

Quoted 8 lines
bindings/python/tests/gpiosim/ext.c new file +8 -0
+	for (modconst = module_constants; modconst->name; modconst++) {
+		ret = PyModule_AddIntConstant(module,
+					      modconst->name, modconst->val);
+		if (ret) {
+			Py_DECREF(module);
+			return NULL;
+		}
+ 	}

^
space before tab

<snip>

bindings/python/tests/tests_edge_event.py line 205

Quoted 8 lines
bindings/python/tests/tests_edge_event.py new file +8 -0
+
+    # TODO buffer capacity
+    # def test_read_over_buffer_capacity(self):
+    #     buf = gpiod.EdgeEventBuffer(2)
+    #     self.assertTrue(self.request.wait_edge_event(datetime.timedelta(seconds=1)))
+    #     self.assertEqual(self.request.read_edge_event(buf), 2)
+    #     self.assertEqual(len(buf), 2)
+

Why the TODO? That API has been changed since, no?

So no problems with the tests, though admittedly I've only skimmed them relative to the other patches as I'm doing them last and have reached the limits of my attention span. I may have another look later, but I'd rather get the other reviews out now than wait until I'm up to that.

Cheers,
Kent.

#19 lore raw

PATCH v3 4/4 bindings: python: implement python bindings for libgpiod v2

In reply to Bartosz Golaszewski

This adds python bindings for libgpiod v2. As opposed to v1, they are mostly written in python with just low-level elements written in C and interfacing with libgpiod.so.

We've also added setup.py which will allow to use pip for managing the bindings and split them into a separate meta-openembedded recipe.

---
 bindings/python/.gitignore                |   8 +
 bindings/python/Makefile.am               |  35 ++
 bindings/python/gpiod/Makefile.am         |  17 +
 bindings/python/gpiod/__init__.py         |  53 +++
 bindings/python/gpiod/chip.py             | 308 +++++++++++++++++
 bindings/python/gpiod/chip_info.py        |  21 ++
 bindings/python/gpiod/edge_event.py       |  46 +++
 bindings/python/gpiod/exception.py        |  20 ++
 bindings/python/gpiod/ext/Makefile.am     |  11 +
 bindings/python/gpiod/ext/chip.c          | 335 ++++++++++++++++++
 bindings/python/gpiod/ext/common.c        |  92 +++++
 bindings/python/gpiod/ext/internal.h      |  20 ++
 bindings/python/gpiod/ext/line-config.c   | 133 +++++++
 bindings/python/gpiod/ext/line-settings.c | 130 +++++++
 bindings/python/gpiod/ext/module.c        | 193 +++++++++++
 bindings/python/gpiod/ext/request.c       | 402 ++++++++++++++++++++++
 bindings/python/gpiod/info_event.py       |  33 ++
 bindings/python/gpiod/internal.py         |  19 +
 bindings/python/gpiod/line.py             |  56 +++
 bindings/python/gpiod/line_info.py        |  73 ++++
 bindings/python/gpiod/line_request.py     | 258 ++++++++++++++
 bindings/python/gpiod/line_settings.py    |  62 ++++
 bindings/python/setup.py                  |  47 +++
 configure.ac                              |   3 +
 24 files changed, 2375 insertions(+)
 create mode 100644 bindings/python/.gitignore
 create mode 100644 bindings/python/Makefile.am
 create mode 100644 bindings/python/gpiod/Makefile.am
 create mode 100644 bindings/python/gpiod/__init__.py
 create mode 100644 bindings/python/gpiod/chip.py
 create mode 100644 bindings/python/gpiod/chip_info.py
 create mode 100644 bindings/python/gpiod/edge_event.py
 create mode 100644 bindings/python/gpiod/exception.py
 create mode 100644 bindings/python/gpiod/ext/Makefile.am
 create mode 100644 bindings/python/gpiod/ext/chip.c
 create mode 100644 bindings/python/gpiod/ext/common.c
 create mode 100644 bindings/python/gpiod/ext/internal.h
 create mode 100644 bindings/python/gpiod/ext/line-config.c
 create mode 100644 bindings/python/gpiod/ext/line-settings.c
 create mode 100644 bindings/python/gpiod/ext/module.c
 create mode 100644 bindings/python/gpiod/ext/request.c
 create mode 100644 bindings/python/gpiod/info_event.py
 create mode 100644 bindings/python/gpiod/internal.py
 create mode 100644 bindings/python/gpiod/line.py
 create mode 100644 bindings/python/gpiod/line_info.py
 create mode 100644 bindings/python/gpiod/line_request.py
 create mode 100644 bindings/python/gpiod/line_settings.py
 create mode 100644 bindings/python/setup.py
bindings/python/.gitignore new file +8 -0
new file mode 100644
index 0000000..b603068
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2017-2022 Bartosz Golaszewski <[email protected]>
+
+build/
+__pycache__/
+dist/
+gpiod.egg-info/
+*.so
bindings/python/Makefile.am new file +35 -0
new file mode 100644
index 0000000..3212a8f
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+EXTRA_DIST = setup.py
+
+if WITH_TESTS
+
+BUILD_TESTS = 1
+
+endif
+
+all-local:
+	GPIOD_VERSION_STRING=$(VERSION_STR) \
+	GPIOD_WITH_TESTS=$(BUILD_TESTS) \
+	$(PYTHON) setup.py build_ext --inplace \
+		--include-dirs=$(top_srcdir)/include/:$(top_srcdir)/tests/gpiosim/ \
+		--library-dirs=$(top_builddir)/lib/.libs/:$(top_srcdir)/tests/gpiosim/.libs/
+
+install-exec-local:
+	GPIOD_WITH_TESTS= \
+	$(PYTHON) setup.py install --prefix=$(prefix)
+
+SUBDIRS = gpiod
+
+if WITH_TESTS
+
+SUBDIRS += tests
+
+endif
+
+if WITH_EXAMPLES
+
+SUBDIRS += examples
+
+endif
bindings/python/gpiod/Makefile.am new file +17 -0
new file mode 100644
index 0000000..b0f4126
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+SUBDIRS = ext
+
+EXTRA_DIST = \
+	chip_info.py \
+	chip.py \
+	edge_event.py \
+	exception.py \
+	info_event.py \
+	__init__.py \
+	internal.py \
+	line_info.py \
+	line.py \
+	line_request.py \
+	line_settings.py 
bindings/python/gpiod/__init__.py new file +53 -0

53 lines not on this page · raw message

bindings/python/gpiod/chip.py new file +308 -0

308 lines not on this page · raw message

bindings/python/gpiod/chip_info.py new file +21 -0
new file mode 100644
index 0000000..a506b55
@@ -0,0 +1,21 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+
+from dataclasses import dataclass
+
+
+@dataclass(frozen=True, repr=False)
+class ChipInfo:
+    """
+    Snapshot of a chip's status.
+    """
+
+    name: str
+    label: str
+    num_lines: int
+
+    def __str__(self):
+        return '<ChipInfo name="{}" label="{}" num_lines={}>'.format(
+            self.name, self.label, self.num_lines
+        )
bindings/python/gpiod/edge_event.py new file +46 -0

46 lines not on this page · raw message

bindings/python/gpiod/exception.py new file +20 -0
new file mode 100644
index 0000000..07ffaa6
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+
+class ChipClosedError(Exception):
+    """
+    Error raised when an already closed chip is used.
+    """
+
+    def __init__(self):
+        super().__init__("I/O operation on closed chip")
+
+
+class RequestReleasedError(Exception):
+    """
+    Error raised when a released request is used.
+    """
+
+    def __init__(self):
+        super().__init__("GPIO lines have been released")
bindings/python/gpiod/ext/Makefile.am new file +11 -0
new file mode 100644
index 0000000..9c81b17
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+EXTRA_DIST = \
+	chip.c \
+	common.c \
+	internal.h \
+	line-config.c \
+	line-settings.c \
+	module.c \
+	request.c
bindings/python/gpiod/ext/chip.c new file +335 -0

335 lines not on this page · raw message

bindings/python/gpiod/ext/common.c new file +92 -0

92 lines not on this page · raw message

bindings/python/gpiod/ext/internal.h new file +20 -0
new file mode 100644
index 0000000..ed80034
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]> */
+
+#ifndef __LIBGPIOD_PYTHON_MODULE_H__
+#define __LIBGPIOD_PYTHON_MODULE_H__
+
+#include <gpiod.h>
+#include <Python.h>
+
+PyObject *_Py_gpiod_SetErrFromErrno(const char *filename);
+#define Py_gpiod_SetErrFromErrno() _Py_gpiod_SetErrFromErrno(__FILE__)
+
+PyObject *Py_gpiod_GetGlobalType(const char *type_name);
+unsigned int Py_gpiod_PyLongAsUnsignedInt(PyObject *pylong);
+void Py_gpiod_dealloc(PyObject *self);
+PyObject *Py_gpiod_MakeRequestObject(struct gpiod_line_request *request);
+struct gpiod_line_config *Py_gpiod_LineConfigGetData(PyObject *obj);
+struct gpiod_line_settings *Py_gpiod_LineSettingsGetData(PyObject *obj);
+
+#endif /* __LIBGPIOD_PYTHON_MODULE_H__ */
bindings/python/gpiod/ext/line-config.c new file +133 -0

133 lines not on this page · raw message

bindings/python/gpiod/ext/line-settings.c new file +130 -0

130 lines not on this page · raw message

bindings/python/gpiod/ext/module.c new file +193 -0

193 lines not on this page · raw message

bindings/python/gpiod/ext/request.c new file +402 -0

402 lines not on this page · raw message

bindings/python/gpiod/info_event.py new file +33 -0
new file mode 100644
index 0000000..78b1459
@@ -0,0 +1,33 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from . import _ext
+from .line_info import LineInfo
+from dataclasses import dataclass
+from enum import Enum
+
+
+@dataclass(frozen=True, init=False, repr=False)
+class InfoEvent:
+    """
+    Immutable object containing data about a single line info event.
+    """
+
+    class Type(Enum):
+        LINE_REQUESTED = _ext.INFO_EVENT_TYPE_LINE_REQUESTED
+        LINE_RELEASED = _ext.INFO_EVENT_TYPE_LINE_RELEASED
+        LINE_CONFIG_CHANGED = _ext.INFO_EVENT_TYPE_LINE_CONFIG_CHANGED
+
+    event_type: Type
+    timestamp_ns: int
+    line_info: LineInfo
+
+    def __init__(self, event_type: int, timestamp_ns: int, line_info: LineInfo):
+        object.__setattr__(self, "event_type", InfoEvent.Type(event_type))
+        object.__setattr__(self, "timestamp_ns", timestamp_ns)
+        object.__setattr__(self, "line_info", line_info)
+
+    def __str__(self):
+        return "<InfoEvent type={} timestamp_ns={} line_info={}>".format(
+            self.event_type, self.timestamp_ns, self.line_info
+        )
bindings/python/gpiod/internal.py new file +19 -0
new file mode 100644
index 0000000..37e8b62
@@ -0,0 +1,19 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from datetime import timedelta
+from select import select
+from typing import Optional, Union
+
+
+def poll_fd(fd: int, timeout: Optional[Union[timedelta, float]] = None) -> bool:
+    if timeout is None:
+        timeout = 0.0
+
+    if isinstance(timeout, timedelta):
+        sec = timeout.total_seconds()
+    else:
+        sec = timeout
+
+    readable, _, _ = select([fd], [], [], sec)
+    return True if fd in readable else False
bindings/python/gpiod/line.py new file +56 -0

56 lines not on this page · raw message

bindings/python/gpiod/line_info.py new file +73 -0

73 lines not on this page · raw message

bindings/python/gpiod/line_request.py new file +258 -0

258 lines not on this page · raw message

bindings/python/gpiod/line_settings.py new file +62 -0

62 lines not on this page · raw message

bindings/python/setup.py new file +47 -0

47 lines not on this page · raw message

configure.ac +3 -0
index 6ac1d8e..048b2ac 100644
@@ -244,8 +244,11 @@ AC_CONFIG_FILES([Makefile
 		 bindings/cxx/examples/Makefile
 		 bindings/cxx/tests/Makefile
 		 bindings/python/Makefile
+		 bindings/python/gpiod/Makefile
+		 bindings/python/gpiod/ext/Makefile
 		 bindings/python/examples/Makefile
 		 bindings/python/tests/Makefile
+		 bindings/python/tests/gpiosim/Makefile
 		 man/Makefile])
 
 AC_OUTPUT
Signature · 1 line
2.34.1
#20 lore raw

Re: [libgpiod v2][PATCH v3 4/4] bindings: python: implement python bindings for libgpiod v2

Quoted 6 lines · Bartosz Golaszewski

This adds python bindings for libgpiod v2. As opposed to v1, they are mostly written in python with just low-level elements written in C and interfacing with libgpiod.so.

We've also added setup.py which will allow to use pip for managing the bindings and split them into a separate meta-openembedded recipe.

...

bindings/python/gpiod/Makefile.am line 17

Quoted 1 line
bindings/python/gpiod/Makefile.am new file +1 -0
+	line_settings.py 

Trailing white space.

...

bindings/python/gpiod/__init__.py line 22

Quoted 3 lines
bindings/python/gpiod/__init__.py new file +3 -0
+__version__ = _ext.__version__
+
+

Single blank line is enough, no?
Same seems applicable to other files.

...

bindings/python/gpiod/__init__.py line 40

Quoted 4 lines
bindings/python/gpiod/__init__.py new file +4 -0
+def request_lines(path: str, *args, **kwargs) -> LineRequest:
+    """
+    Open a GPIO chip pointed to by 'path', request lines according to the
+    configuration arguments, close the chip and return the request object.

This description seems wrong. First we collect the request object, then close the chip, right?

Quoted 12 lines
bindings/python/gpiod/__init__.py new file +12 -0
+    Args:
+      path
+        Path to the GPIO character device file.
+      *args
+      **kwargs
+        See Chip.request_lines() for configuration arguments.
+
+    Returns:
+      Returns a new LineRequest object.
+    """
+    with Chip(path) as chip:
+        return chip.request_lines(*args, **kwargs)

...

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+class Chip:
+    """
+    Represents a GPIO chip.
+
+    Chip object manages all resources associated with the GPIO chip it represents.
+
+    The gpiochip device file is opened during the object's construction. The Chip
+    object's constructor takes the path to the GPIO chip device file
+    as the only argument.
+

bindings/python/gpiod/chip.py line 37

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+    Callers must close the chip by calling the close() method when it's no longer
+    used.
+
+    Example:
+
+        chip = gpiod.Chip(\"/dev/gpiochip0\")
+        do_something(chip)
+        chip.close()
+
+    The gpiod.Chip class also supports controlled execution ('with' statement).

Oh, this does not sound pythonic, does it? I would expect that this will be closed automatically on object destruction.

Or user may call it explicitly by

del chip

Of course .close() method may be good to have.

Quoted 21 lines
bindings/python/gpiod/chip.py new file +21 -0
+    Example:
+
+        with gpiod.Chip(path="/dev/gpiochip0") as chip:
+            do_something(chip)
+    """
+
+    def __init__(self, path: str):
+        """
+        Open a GPIO device.
+
+        Args:
+          path:
+            Path to the GPIO character device file.
+        """
+        self._chip = _ext.Chip(path)
+
+    def __bool__(self) -> bool:
+        """
+        Boolean conversion for GPIO chips.
+
+        Returns:

bindings/python/gpiod/chip.py line 68

Quoted 10 lines
bindings/python/gpiod/chip.py new file +9 -0
+          True if the chip is open and False if it's closed.
+        """
+        return True if self._chip else False
+
+    def __enter__(self):
+        """
+        Controlled execution enter callback.
+        """
 
+        self._check_closed()

I don't understand this dance in the most of the methods. Why do we need to check this?

Quoted 41 lines
bindings/python/gpiod/chip.py new file +41 -0
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback) -> None:
+        """
+        Controlled execution exit callback.
+        """
+        self.close()
+
+    def _check_closed(self) -> None:
+        if not self._chip:
+            raise ChipClosedError()
+
+    def close(self) -> None:
+        """
+        Close the associated GPIO chip descriptor. The chip object must no
+        longer be used after this method is called.
+        """
+        self._check_closed()
+        self._chip.close()
+        self._chip = None
+
+    def get_info(self) -> ChipInfo:
+        """
+        Get the information about the chip.
+
+        Returns:
+          New gpiod.ChipInfo object.
+        """
+        self._check_closed()
+        return self._chip.get_info()
+
+    def map_line(self, id: Union[str, int]) -> int:
+        """
+        Map a line's identifier to its offset within the chip.
+
+        Args:
+          id:
+            Name of the GPIO line, its offset as a string or its offset as an
+            integer.
+
+        Returns:

bindings/python/gpiod/chip.py line 119

Quoted 10 lines
bindings/python/gpiod/chip.py new file +9 -0
+          If id is an integer - it's returned as is (unless it's out of range
+          for this chip). If it's a string, the method tries to interpret it as
+          the name of the line first and tries too perform a name lookup within
+          the chip. If it fails, it tries to convert the string to an integer
+          and check if it represents a valid offset within the chip and if
+          so - returns it.
+        """
+        self._check_closed()
 
+        if not isinstance(id, int):

Why not use positive conditional here and everywhere in the similar cases?

	if isinstance():
		...
	else
		...

bindings/python/gpiod/chip.py line 127

Quoted 9 lines
bindings/python/gpiod/chip.py new file +8 -0
+            try:
+                return self._chip.map_line(id)
+            except OSError as ex:
+                if ex.errno == ENOENT:
 
+                    try:
+                        offset = int(id)
+                    except ValueError:
+                        raise ex

How this can be non-exceptional? I don't see how id can change its type from non-int (which is checked above by isinstance() call) to int?

bindings/python/gpiod/chip.py line 129

Quoted 2 lines
bindings/python/gpiod/chip.py new file +2 -0
+                else:
+                    raise ex

Last time I have interaction with Python and developers who write the code in Python the custom exception class was good thing to have. Is this changed during the times?

Quoted 43 lines
bindings/python/gpiod/chip.py new file +43 -0
+        else:
+            offset = id
+
+        if offset >= self.get_info().num_lines:
+            raise ValueError("line offset of out range")
+
+        return offset
+
+    def _get_line_info(self, line: Union[int, str], watch: bool) -> LineInfo:
+        self._check_closed()
+        return self._chip.get_line_info(self.map_line(line), watch)
+
+    def get_line_info(self, line: Union[int, str]) -> LineInfo:
+        """
+        Get the snapshot of information about the line at given offset.
+
+        Args:
+          line:
+            Offset or name of the GPIO line to get information for.
+
+        Returns:
+          New LineInfo object.
+        """
+        return self._get_line_info(line, watch=False)
+
+    def watch_line_info(self, line: Union[int, str]) -> LineInfo:
+        """
+        Get the snapshot of information about the line at given offset and
+        start watching it for future changes.
+
+        Args:
+          line:
+            Offset or name of the GPIO line to get information for.
+
+        Returns:
+          New gpiod.LineInfo object.
+        """
+        return self._get_line_info(line, watch=True)
+
+    def unwatch_line_info(self, line: Union[int, str]) -> None:
+        """
+        Stop watching a line for status changes.
+

bindings/python/gpiod/chip.py line 182

Quoted 10 lines
bindings/python/gpiod/chip.py new file +9 -0
+        Args:
+          line:
+            Offset or name of the line to stop watching.
+        """
+        self._check_closed()
+        return self._chip.unwatch_line_info(self.map_line(line))
 
+    def wait_info_event(
+        self, timeout: Optional[Union[timedelta, float]] = None
+    ) -> bool:

pylint (btw, do you use it?) has a limit of 100 for a long time. The above can be places on one line.

Quoted 53 lines
bindings/python/gpiod/chip.py new file +53 -0
+        """
+        Wait for line status change events on any of the watched lines on the
+        chip.
+
+        Args:
+          timeout:
+            Wait time limit represented as either a datetime.timedelta object
+            or the number of seconds stored in a float.
+
+        Returns:
+          True if an info event is ready to be read from the chip, False if the
+          wait timed out without any events.
+        """
+        self._check_closed()
+
+        return poll_fd(self.fd, timeout)
+
+    def read_info_event(self) -> InfoEvent:
+        """
+        Read a single line status change event from the chip.
+
+        Returns:
+          New gpiod.InfoEvent object.
+
+        Note:
+          This function may block if there are no available events in the queue.
+        """
+        self._check_closed()
+        return self._chip.read_info_event()
+
+    def request_lines(
+        self,
+        config: dict[tuple[Union[int, str]], Optional[LineSettings]],
+        consumer: Optional[str] = None,
+        event_buffer_size: Optional[int] = None,
+    ) -> LineRequest:
+        """
+        Request a set of lines for exclusive usage.
+
+        Args:
+          config:
+            Dictionary mapping offsets or names (or tuples thereof) to
+            LineSettings. If None is passed as the value of the mapping,
+            default settings are used.
+          consumer:
+            Consumer string to use for this request.
+          event_buffer_size:
+            Size of the kernel edge event buffer to configure for this request.
+
+        Returns:
+          New LineRequest object.
+        """
+        self._check_closed()

bindings/python/gpiod/chip.py line 245

Quoted 10 lines
bindings/python/gpiod/chip.py new file +9 -0
+
+        line_cfg = _ext.LineConfig()
+
+        for lines, settings in config.items():
+            offsets = list()
+            name_map = dict()
+            offset_map = dict()
 
+            if isinstance(lines, int) or isinstance(lines, str):
+                lines = (lines,)

Instead of putting str and int objects into one bucket, handling exceptions, etc, I would rather see two methods (one per type).

Quoted 62 lines
bindings/python/gpiod/chip.py new file +62 -0
+            for line in lines:
+                offset = self.map_line(line)
+                offsets.append(offset)
+                if isinstance(line, str):
+                    name_map[line] = offset
+                    offset_map[offset] = line
+
+            if settings is None:
+                settings = LineSettings()
+
+            line_cfg.add_line_settings(
+                offsets, _line_settings_to_ext_line_settings(settings)
+            )
+
+        req_internal = self._chip.request_lines(line_cfg, consumer, event_buffer_size)
+        request = LineRequest(req_internal)
+
+        request._offsets = req_internal.offsets
+        request._name_map = name_map
+        request._offset_map = offset_map
+
+        request._lines = list()
+        for off in request.offsets:
+            request._lines.append(offset_map[off] if off in offset_map else off)
+
+        return request
+
+    def __repr__(self) -> str:
+        """
+        Return a string that can be used to re-create this chip object.
+        """
+        if not self._chip:
+            return "<Chip CLOSED>"
+
+        return 'Chip("{}")'.format(self.path)
+
+    def __str__(self) -> str:
+        """
+        Return a user-friendly, human-readable description of this chip.
+        """
+        if not self._chip:
+            return "<Chip CLOSED>"
+
+        return '<Chip path="{}" fd={} info={}>'.format(
+            self.path, self.fd, self.get_info()
+        )
+
+    @property
+    def path(self) -> str:
+        """
+        Filesystem path used to open this chip.
+        """
+        self._check_closed()
+        return self._chip.path
+
+    @property
+    def fd(self) -> int:
+        """
+        File descriptor associated with this chip.
+        """
+        self._check_closed()
+        return self._chip.fd
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#21 lore raw

Re: [libgpiod v2][PATCH v3 4/4] bindings: python: implement python bindings for libgpiod v2

Quoted 15 lines · Andy Shevchenko
Quoted 6 lines · Bartosz Golaszewski

This adds python bindings for libgpiod v2. As opposed to v1, they are mostly written in python with just low-level elements written in C and interfacing with libgpiod.so.

We've also added setup.py which will allow to use pip for managing the bindings and split them into a separate meta-openembedded recipe.

...

Quoted 1 line

+ line_settings.py

Trailing white space.

Quoted 9 lines

...

bindings/python/gpiod/__init__.py line 22

Quoted 3 lines
bindings/python/gpiod/__init__.py new file +3 -0
+__version__ = _ext.__version__
+
+

Single blank line is enough, no?
Same seems applicable to other files.

I use black (https://pypi.org/project/black/) for formatting and it does this between classes and in some other cases. I like this formatting and it seems it's in line with various PEPs.

Quoted 10 lines

...

bindings/python/gpiod/__init__.py line 40

Quoted 4 lines
bindings/python/gpiod/__init__.py new file +4 -0
+def request_lines(path: str, *args, **kwargs) -> LineRequest:
+    """
+    Open a GPIO chip pointed to by 'path', request lines according to the
+    configuration arguments, close the chip and return the request object.

This description seems wrong. First we collect the request object, then close the chip, right?

Well, yeah, we return it after we close the chip. Not sure what you mean.

Quoted 39 lines
Quoted 12 lines
bindings/python/gpiod/__init__.py new file +12 -0
+    Args:
+      path
+        Path to the GPIO character device file.
+      *args
+      **kwargs
+        See Chip.request_lines() for configuration arguments.
+
+    Returns:
+      Returns a new LineRequest object.
+    """
+    with Chip(path) as chip:
+        return chip.request_lines(*args, **kwargs)

...

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+class Chip:
+    """
+    Represents a GPIO chip.
+
+    Chip object manages all resources associated with the GPIO chip it represents.
+
+    The gpiochip device file is opened during the object's construction. The Chip
+    object's constructor takes the path to the GPIO chip device file
+    as the only argument.
+

bindings/python/gpiod/chip.py line 37

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+    Callers must close the chip by calling the close() method when it's no longer
+    used.
+
+    Example:
+
+        chip = gpiod.Chip(\"/dev/gpiochip0\")
+        do_something(chip)
+        chip.close()
+
+    The gpiod.Chip class also supports controlled execution ('with' statement).

Oh, this does not sound pythonic, does it? I would expect that this will be closed automatically on object destruction.

Quoted 6 lines

Or user may call it explicitly by

del chip

Of course .close() method may be good to have.

Python is not like C++, it doesn't have RAII. There are no guarantees from the language itself that an object will be destroyed the moment the last reference is dropped. It's more lika java in that aspect. It's true that cpython implementation works like this for most cases but we must not rely on any given implementation. Controlled execution is very much the pythonic way.

Quoted 30 lines
Quoted 29 lines
bindings/python/gpiod/chip.py new file +29 -0
+    Example:
+
+        with gpiod.Chip(path="/dev/gpiochip0") as chip:
+            do_something(chip)
+    """
+
+    def __init__(self, path: str):
+        """
+        Open a GPIO device.
+
+        Args:
+          path:
+            Path to the GPIO character device file.
+        """
+        self._chip = _ext.Chip(path)
+
+    def __bool__(self) -> bool:
+        """
+        Boolean conversion for GPIO chips.
+
+        Returns:
+          True if the chip is open and False if it's closed.
+        """
+        return True if self._chip else False
+
+    def __enter__(self):
+        """
+        Controlled execution enter callback.
+        """
Quoted 5 lines
Quoted 1 line

+ self._check_closed()

I don't understand this dance in the most of the methods. Why do we need to check this?

Because we don't want to allow the user to use a closed chip? The underlying C part doesn't check it so if we don't do it here, it will segfault.

Quoted 64 lines
Quoted 41 lines
bindings/python/gpiod/chip.py new file +41 -0
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback) -> None:
+        """
+        Controlled execution exit callback.
+        """
+        self.close()
+
+    def _check_closed(self) -> None:
+        if not self._chip:
+            raise ChipClosedError()
+
+    def close(self) -> None:
+        """
+        Close the associated GPIO chip descriptor. The chip object must no
+        longer be used after this method is called.
+        """
+        self._check_closed()
+        self._chip.close()
+        self._chip = None
+
+    def get_info(self) -> ChipInfo:
+        """
+        Get the information about the chip.
+
+        Returns:
+          New gpiod.ChipInfo object.
+        """
+        self._check_closed()
+        return self._chip.get_info()
+
+    def map_line(self, id: Union[str, int]) -> int:
+        """
+        Map a line's identifier to its offset within the chip.
+
+        Args:
+          id:
+            Name of the GPIO line, its offset as a string or its offset as an
+            integer.
+
+        Returns:

bindings/python/gpiod/chip.py line 119

Quoted 10 lines
bindings/python/gpiod/chip.py new file +9 -0
+          If id is an integer - it's returned as is (unless it's out of range
+          for this chip). If it's a string, the method tries to interpret it as
+          the name of the line first and tries too perform a name lookup within
+          the chip. If it fails, it tries to convert the string to an integer
+          and check if it represents a valid offset within the chip and if
+          so - returns it.
+        """
+        self._check_closed()
 
+        if not isinstance(id, int):

Why not use positive conditional here and everywhere in the similar cases?

        if isinstance():
                ...
        else
                ...
Quoted 4 lines
bindings/python/gpiod/chip.py new file +4 -0
+            try:
+                return self._chip.map_line(id)
+            except OSError as ex:
+                if ex.errno == ENOENT:
Quoted 8 lines

bindings/python/gpiod/chip.py line 127

Quoted 4 lines
bindings/python/gpiod/chip.py new file +4 -0
+                    try:
+                        offset = int(id)
+                    except ValueError:
+                        raise ex

How this can be non-exceptional? I don't see how id can change its type from non-int (which is checked above by isinstance() call) to int?

It's explained in the pydoc for this method. :)

If the line ID is not an int, try to map the name to an offset, if it fails, try to convert it to offset from whatever type (officially only string but it's python) was supplied.

Quoted 7 lines

bindings/python/gpiod/chip.py line 129

Quoted 2 lines
bindings/python/gpiod/chip.py new file +2 -0
+                else:
+                    raise ex

Last time I have interaction with Python and developers who write the code in Python the custom exception class was good thing to have. Is this changed during the times?

We do have custom exceptions, see gpiod/exception.py. It's just that in this case we want to propagate whatever exception was raised by libgpiod - OSError translates directly from errno.

Quoted 50 lines
Quoted 49 lines
bindings/python/gpiod/chip.py new file +49 -0
+        else:
+            offset = id
+
+        if offset >= self.get_info().num_lines:
+            raise ValueError("line offset of out range")
+
+        return offset
+
+    def _get_line_info(self, line: Union[int, str], watch: bool) -> LineInfo:
+        self._check_closed()
+        return self._chip.get_line_info(self.map_line(line), watch)
+
+    def get_line_info(self, line: Union[int, str]) -> LineInfo:
+        """
+        Get the snapshot of information about the line at given offset.
+
+        Args:
+          line:
+            Offset or name of the GPIO line to get information for.
+
+        Returns:
+          New LineInfo object.
+        """
+        return self._get_line_info(line, watch=False)
+
+    def watch_line_info(self, line: Union[int, str]) -> LineInfo:
+        """
+        Get the snapshot of information about the line at given offset and
+        start watching it for future changes.
+
+        Args:
+          line:
+            Offset or name of the GPIO line to get information for.
+
+        Returns:
+          New gpiod.LineInfo object.
+        """
+        return self._get_line_info(line, watch=True)
+
+    def unwatch_line_info(self, line: Union[int, str]) -> None:
+        """
+        Stop watching a line for status changes.
+
+        Args:
+          line:
+            Offset or name of the line to stop watching.
+        """
+        self._check_closed()
+        return self._chip.unwatch_line_info(self.map_line(line))
Quoted 7 lines

bindings/python/gpiod/chip.py line 182

Quoted 3 lines
bindings/python/gpiod/chip.py new file +3 -0
+    def wait_info_event(
+        self, timeout: Optional[Union[timedelta, float]] = None
+    ) -> bool:

pylint (btw, do you use it?) has a limit of 100 for a long time. The above can be places on one line.

I use black and it enforces a 88 character limit.

Quoted 61 lines
Quoted 60 lines
bindings/python/gpiod/chip.py new file +60 -0
+        """
+        Wait for line status change events on any of the watched lines on the
+        chip.
+
+        Args:
+          timeout:
+            Wait time limit represented as either a datetime.timedelta object
+            or the number of seconds stored in a float.
+
+        Returns:
+          True if an info event is ready to be read from the chip, False if the
+          wait timed out without any events.
+        """
+        self._check_closed()
+
+        return poll_fd(self.fd, timeout)
+
+    def read_info_event(self) -> InfoEvent:
+        """
+        Read a single line status change event from the chip.
+
+        Returns:
+          New gpiod.InfoEvent object.
+
+        Note:
+          This function may block if there are no available events in the queue.
+        """
+        self._check_closed()
+        return self._chip.read_info_event()
+
+    def request_lines(
+        self,
+        config: dict[tuple[Union[int, str]], Optional[LineSettings]],
+        consumer: Optional[str] = None,
+        event_buffer_size: Optional[int] = None,
+    ) -> LineRequest:
+        """
+        Request a set of lines for exclusive usage.
+
+        Args:
+          config:
+            Dictionary mapping offsets or names (or tuples thereof) to
+            LineSettings. If None is passed as the value of the mapping,
+            default settings are used.
+          consumer:
+            Consumer string to use for this request.
+          event_buffer_size:
+            Size of the kernel edge event buffer to configure for this request.
+
+        Returns:
+          New LineRequest object.
+        """
+        self._check_closed()
+
+        line_cfg = _ext.LineConfig()
+
+        for lines, settings in config.items():
+            offsets = list()
+            name_map = dict()
+            offset_map = dict()
Quoted 6 lines

bindings/python/gpiod/chip.py line 245

Quoted 2 lines
bindings/python/gpiod/chip.py new file +2 -0
+            if isinstance(lines, int) or isinstance(lines, str):
+                lines = (lines,)

Instead of putting str and int objects into one bucket, handling exceptions, etc, I would rather see two methods (one per type).

But I very much want to leverage python's dynamic typing and allow mixing the two.

Bartosz

Quoted 68 lines
Quoted 52 lines
bindings/python/gpiod/chip.py new file +52 -0
+            for line in lines:
+                offset = self.map_line(line)
+                offsets.append(offset)
+                if isinstance(line, str):
+                    name_map[line] = offset
+                    offset_map[offset] = line
+
+            if settings is None:
+                settings = LineSettings()
+
+            line_cfg.add_line_settings(
+                offsets, _line_settings_to_ext_line_settings(settings)
+            )
+
+        req_internal = self._chip.request_lines(line_cfg, consumer, event_buffer_size)
+        request = LineRequest(req_internal)
+
+        request._offsets = req_internal.offsets
+        request._name_map = name_map
+        request._offset_map = offset_map
+
+        request._lines = list()
+        for off in request.offsets:
+            request._lines.append(offset_map[off] if off in offset_map else off)
+
+        return request
+
+    def __repr__(self) -> str:
+        """
+        Return a string that can be used to re-create this chip object.
+        """
+        if not self._chip:
+            return "<Chip CLOSED>"
+
+        return 'Chip("{}")'.format(self.path)
+
+    def __str__(self) -> str:
+        """
+        Return a user-friendly, human-readable description of this chip.
+        """
+        if not self._chip:
+            return "<Chip CLOSED>"
+
+        return '<Chip path="{}" fd={} info={}>'.format(
+            self.path, self.fd, self.get_info()
+        )
+
+    @property
+    def path(self) -> str:
+        """
+        Filesystem path used to open this chip.
+        """

bindings/python/gpiod/chip.py line 308

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+        self._check_closed()
+        return self._chip.path
+
+    @property
+    def fd(self) -> int:
+        """
+        File descriptor associated with this chip.
+        """
+        self._check_closed()
+        return self._chip.fd

--
With Best Regards,
Andy Shevchenko

#22 lore raw

Re: [libgpiod v2][PATCH v3 4/4] bindings: python: implement python bindings for libgpiod v2

In reply to Bartosz Golaszewski
Quoted 8 lines · Bartosz Golaszewski

This adds python bindings for libgpiod v2. As opposed to v1, they are mostly written in python with just low-level elements written in C and interfacing with libgpiod.so.

We've also added setup.py which will allow to use pip for managing the bindings and split them into a separate meta-openembedded recipe.

<snipping out files with no probs>

Quoted 9 lines

+++ b/bindings/python/gpiod/Makefile.am

bindings/python/gpiod/Makefile.am new file +7 -0
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+SUBDIRS = ext
+
+EXTRA_DIST = \
+	chip_info.py \

bindings/python/gpiod/Makefile.am line 17

Quoted 10 lines
bindings/python/gpiod/Makefile.am new file +10 -0
+	chip.py \
+	edge_event.py \
+	exception.py \
+	info_event.py \
+	__init__.py \
+	internal.py \
+	line_info.py \
+	line.py \
+	line_request.py \
+	line_settings.py 
                    ^

trailing whitespace

Quoted 90 lines

+++ b/bindings/python/gpiod/chip.py

bindings/python/gpiod/chip.py new file +88 -0
@@ -0,0 +1,308 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from . import _ext
+from .chip_info import ChipInfo
+from .exception import ChipClosedError
+from .info_event import InfoEvent
+from .internal import poll_fd
+from .line_info import LineInfo
+from .line_settings import LineSettings, _line_settings_to_ext_line_settings
+from .line_request import LineRequest
+from datetime import timedelta
+from errno import ENOENT
+from select import select
+from typing import Union, Optional
+
+
+class Chip:
+    """
+    Represents a GPIO chip.
+
+    Chip object manages all resources associated with the GPIO chip it represents.
+
+    The gpiochip device file is opened during the object's construction. The Chip
+    object's constructor takes the path to the GPIO chip device file
+    as the only argument.
+
+    Callers must close the chip by calling the close() method when it's no longer
+    used.
+
+    Example:
+
+        chip = gpiod.Chip(\"/dev/gpiochip0\")
+        do_something(chip)
+        chip.close()
+
+    The gpiod.Chip class also supports controlled execution ('with' statement).
+
+    Example:
+
+        with gpiod.Chip(path="/dev/gpiochip0") as chip:
+            do_something(chip)
+    """
+
+    def __init__(self, path: str):
+        """
+        Open a GPIO device.
+
+        Args:
+          path:
+            Path to the GPIO character device file.
+        """
+        self._chip = _ext.Chip(path)
+
+    def __bool__(self) -> bool:
+        """
+        Boolean conversion for GPIO chips.
+
+        Returns:
+          True if the chip is open and False if it's closed.
+        """
+        return True if self._chip else False
+
+    def __enter__(self):
+        """
+        Controlled execution enter callback.
+        """
+        self._check_closed()
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback) -> None:
+        """
+        Controlled execution exit callback.
+        """
+        self.close()
+
+    def _check_closed(self) -> None:
+        if not self._chip:
+            raise ChipClosedError()
+
+    def close(self) -> None:
+        """
+        Close the associated GPIO chip descriptor. The chip object must no
+        longer be used after this method is called.
+        """
+        self._check_closed()
+        self._chip.close()
+        self._chip = None

bindings/python/gpiod/chip.py line 98

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+
+    def get_info(self) -> ChipInfo:
+        """
+        Get the information about the chip.
+
+        Returns:
+          New gpiod.ChipInfo object.
+        """
+        self._check_closed()
+        return self._chip.get_info()

Could the info be cached, rather than going to the kernel every time? It's not as though it changes, and this gets used a lot by map_line() below for range checking.

Quoted 7 lines
bindings/python/gpiod/chip.py new file +7 -0
+
+    def map_line(self, id: Union[str, int]) -> int:
+        """
+        Map a line's identifier to its offset within the chip.
+
+        Args:
+          id:

bindings/python/gpiod/chip.py line 115

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+            Name of the GPIO line, its offset as a string or its offset as an
+            integer.
+
+        Returns:
+          If id is an integer - it's returned as is (unless it's out of range
+          for this chip). If it's a string, the method tries to interpret it as
+          the name of the line first and tries too perform a name lookup within
+          the chip. If it fails, it tries to convert the string to an integer
+          and check if it represents a valid offset within the chip and if
+          so - returns it.

map_line() is a bit vague, like find_line(). How about offset_from_id()?

Should ids fallback to being interpreted as ints if they can't be found as strings? Why not leave that call to the user?
If they aren't sure they can try it as a string, and if that fails try it as an int.
(I realise this is the reverse argument of my comment in the examples patch btw - playing devil's advocate here - and there.)

Is the range check on ints necessary? The kernel will do that when you make the call - what is the benefit of doing it here?
In which case why accept int at all? Which would make the function offset_from_name() - just like the C.

Quoted 14 lines
bindings/python/gpiod/chip.py new file +14 -0
+        """
+        self._check_closed()
+
+        if not isinstance(id, int):
+            try:
+                return self._chip.map_line(id)
+            except OSError as ex:
+                if ex.errno == ENOENT:
+                    try:
+                        offset = int(id)
+                    except ValueError:
+                        raise ex
+                else:
+                    raise ex

bindings/python/gpiod/chip.py line 139

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+        else:
+            offset = id
+
+        if offset >= self.get_info().num_lines:
+            raise ValueError("line offset of out range")
+
+        return offset
+
+    def _get_line_info(self, line: Union[int, str], watch: bool) -> LineInfo:
+        self._check_closed()

Wrt the _check_closed(), what is to prevent another thread closing the chip here, and causing the next line to segfault?

And self.map_line(line) can release the GIL so all bets are off even if you assume cooperative multitasking - not that you can assume that.

Quoted 77 lines
bindings/python/gpiod/chip.py new file +77 -0
+        return self._chip.get_line_info(self.map_line(line), watch)
+
+    def get_line_info(self, line: Union[int, str]) -> LineInfo:
+        """
+        Get the snapshot of information about the line at given offset.
+
+        Args:
+          line:
+            Offset or name of the GPIO line to get information for.
+
+        Returns:
+          New LineInfo object.
+        """
+        return self._get_line_info(line, watch=False)
+
+    def watch_line_info(self, line: Union[int, str]) -> LineInfo:
+        """
+        Get the snapshot of information about the line at given offset and
+        start watching it for future changes.
+
+        Args:
+          line:
+            Offset or name of the GPIO line to get information for.
+
+        Returns:
+          New gpiod.LineInfo object.
+        """
+        return self._get_line_info(line, watch=True)
+
+    def unwatch_line_info(self, line: Union[int, str]) -> None:
+        """
+        Stop watching a line for status changes.
+
+        Args:
+          line:
+            Offset or name of the line to stop watching.
+        """
+        self._check_closed()
+        return self._chip.unwatch_line_info(self.map_line(line))
+
+    def wait_info_event(
+        self, timeout: Optional[Union[timedelta, float]] = None
+    ) -> bool:
+        """
+        Wait for line status change events on any of the watched lines on the
+        chip.
+
+        Args:
+          timeout:
+            Wait time limit represented as either a datetime.timedelta object
+            or the number of seconds stored in a float.
+
+        Returns:
+          True if an info event is ready to be read from the chip, False if the
+          wait timed out without any events.
+        """
+        self._check_closed()
+
+        return poll_fd(self.fd, timeout)
+
+    def read_info_event(self) -> InfoEvent:
+        """
+        Read a single line status change event from the chip.
+
+        Returns:
+          New gpiod.InfoEvent object.
+
+        Note:
+          This function may block if there are no available events in the queue.
+        """
+        self._check_closed()
+        return self._chip.read_info_event()
+
+    def request_lines(
+        self,
+        config: dict[tuple[Union[int, str]], Optional[LineSettings]],
+        consumer: Optional[str] = None,

bindings/python/gpiod/chip.py line 226

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+        event_buffer_size: Optional[int] = None,
+    ) -> LineRequest:
+        """
+        Request a set of lines for exclusive usage.
+
+        Args:
+          config:
+            Dictionary mapping offsets or names (or tuples thereof) to
+            LineSettings. If None is passed as the value of the mapping,
+            default settings are used.

What are the semantics for lines being repeated in the dict keys (which is possible as the keys are tuples, and also because the line can be identified by offset or name)?

Quoted 16 lines
bindings/python/gpiod/chip.py new file +16 -0
+          consumer:
+            Consumer string to use for this request.
+          event_buffer_size:
+            Size of the kernel edge event buffer to configure for this request.
+
+        Returns:
+          New LineRequest object.
+        """
+        self._check_closed()
+
+        line_cfg = _ext.LineConfig()
+
+        for lines, settings in config.items():
+            offsets = list()
+            name_map = dict()
+            offset_map = dict()

bindings/python/gpiod/chip.py line 252

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+
+            if isinstance(lines, int) or isinstance(lines, str):
+                lines = (lines,)
+
+            for line in lines:
+                offset = self.map_line(line)
+                offsets.append(offset)
+                if isinstance(line, str):
+                    name_map[line] = offset
+                    offset_map[offset] = line

Use list comprehensions instead of the for loop?

bindings/python/gpiod/chip.py line 256

Quoted 4 lines
bindings/python/gpiod/chip.py new file +4 -0
+
+            if settings is None:
+                settings = LineSettings()
+

settings = settings or LineSettings()

Would use that directly below if it would fit in the line.

Rename _line_settings_to_ext_line_settings to _line_settings_to_ext, as the second line_settings is redundant? Then it might fit.

Quoted 4 lines
bindings/python/gpiod/chip.py new file +4 -0
+            line_cfg.add_line_settings(
+                offsets, _line_settings_to_ext_line_settings(settings)
+            )
+

bindings/python/gpiod/chip.py line 270

Quoted 10 lines
bindings/python/gpiod/chip.py new file +10 -0
+        req_internal = self._chip.request_lines(line_cfg, consumer, event_buffer_size)
+        request = LineRequest(req_internal)
+
+        request._offsets = req_internal.offsets
+        request._name_map = name_map
+        request._offset_map = offset_map
+
+        request._lines = list()
+        for off in request.offsets:
+            request._lines.append(offset_map[off] if off in offset_map else off)

Again, prefer to use list comprehensions to translate lists like this.

Quoted 57 lines
bindings/python/gpiod/chip.py new file +38 -0
+
+        return request
+
+    def __repr__(self) -> str:
+        """
+        Return a string that can be used to re-create this chip object.
+        """
+        if not self._chip:
+            return "<Chip CLOSED>"
+
+        return 'Chip("{}")'.format(self.path)
+
+    def __str__(self) -> str:
+        """
+        Return a user-friendly, human-readable description of this chip.
+        """
+        if not self._chip:
+            return "<Chip CLOSED>"
+
+        return '<Chip path="{}" fd={} info={}>'.format(
+            self.path, self.fd, self.get_info()
+        )
+
+    @property
+    def path(self) -> str:
+        """
+        Filesystem path used to open this chip.
+        """
+        self._check_closed()
+        return self._chip.path
+
+    @property
+    def fd(self) -> int:
+        """
+        File descriptor associated with this chip.
+        """
+        self._check_closed()
+        return self._chip.fd
bindings/python/gpiod/ext/chip.c new file +13 -0
new file mode 100644
index 0000000..47d5455
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+#include "internal.h"
+
+typedef struct {
+	PyObject_HEAD;
+	struct gpiod_chip *chip;
+} chip_object;
+
+static int
+chip_init(chip_object *self, PyObject *args, PyObject *Py_UNUSED(ignored))
+{

bindings/python/gpiod/ext/chip.c line 23

Quoted 10 lines
bindings/python/gpiod/ext/chip.c new file +10 -0
+	char *path;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "s", &path);
+	if (!ret)
+		return -1;
+
+	Py_BEGIN_ALLOW_THREADS;
+	self->chip = gpiod_chip_open(path);
+	Py_END_ALLOW_THREADS;

Is the assignment to self->chip while not holding the GIL (between the BEGIN and END) safe?
To be sure, assign to a local then assign that to self->chip after the END.

Similarly other BEGIN/END blocks should only access locals. And even then you probably need locks to prevent threads trampling each other, but the locks need the GIL, IIUC.

So I suspect your chip._check_closed()s need to be more like
with chip._lock()

Similarly the other object validity checks.

Quoted 66 lines
bindings/python/gpiod/ext/chip.c new file +66 -0
+	if (!self->chip) {
+		Py_gpiod_SetErrFromErrno();
+		return -1;
+	}
+
+	return 0;
+}
+
+static void chip_finalize(chip_object *self)
+{
+	if (self->chip)
+		PyObject_CallMethod((PyObject *)self, "close", "");
+}
+
+static PyObject *chip_path(chip_object *self, void *Py_UNUSED(ignored))
+{
+	return PyUnicode_FromString(gpiod_chip_get_path(self->chip));
+}
+
+static PyObject *chip_fd(chip_object *self, void *Py_UNUSED(ignored))
+{
+	return PyLong_FromLong(gpiod_chip_get_fd(self->chip));
+}
+
+static PyGetSetDef chip_getset[] = {
+	{
+		.name = "path",
+		.get = (getter)chip_path,
+	},
+	{
+		.name = "fd",
+		.get = (getter)chip_fd,
+	},
+	{ }
+};
+
+static PyObject *chip_close(chip_object *self, PyObject *Py_UNUSED(ignored))
+{
+	Py_BEGIN_ALLOW_THREADS;
+	gpiod_chip_close(self->chip);
+	Py_END_ALLOW_THREADS;
+	self->chip = NULL;
+
+	Py_RETURN_NONE;
+}
+
+static PyObject *chip_get_info(chip_object *self, PyObject *Py_UNUSED(ignored))
+{
+	struct gpiod_chip_info *info;
+	PyObject *type, *ret;
+
+	type = Py_gpiod_GetGlobalType("ChipInfo");
+	if (!type)
+		return NULL;
+
+	info = gpiod_chip_get_info(self->chip);
+	if (!info)
+		return PyErr_SetFromErrno(PyExc_OSError);
+
+	 ret = PyObject_CallFunction(type, "ssI",
+				     gpiod_chip_info_get_name(info),
+				     gpiod_chip_info_get_label(info),
+				     gpiod_chip_info_get_num_lines(info));
+	 gpiod_chip_info_free(info);
+	 return ret;
+}

bindings/python/gpiod/ext/chip.c line 99

Quoted 10 lines
bindings/python/gpiod/ext/chip.c new file +10 -0
+
+static PyObject *make_line_info(struct gpiod_line_info *info)
+{
+	PyObject *type;
+
+	type = Py_gpiod_GetGlobalType("LineInfo");
+	if (!type)
+		return NULL;
+
+	return PyObject_CallFunction(type, "IsOsiOiiiiOi",
                                                   ^
                                       "IsOsiOiiiiOk"

debounce_period_us is an unsigned long, not int.

Quoted 253 lines
bindings/python/gpiod/ext/chip.c new file +236 -0
+				gpiod_line_info_get_offset(info),
+				gpiod_line_info_get_name(info),
+				gpiod_line_info_is_used(info) ?
+							Py_True : Py_False,
+				gpiod_line_info_get_consumer(info),
+				gpiod_line_info_get_direction(info),
+				gpiod_line_info_is_active_low(info) ?
+							Py_True : Py_False,
+				gpiod_line_info_get_bias(info),
+				gpiod_line_info_get_drive(info),
+				gpiod_line_info_get_edge_detection(info),
+				gpiod_line_info_get_event_clock(info),
+				gpiod_line_info_is_debounced(info) ?
+							Py_True : Py_False,
+				gpiod_line_info_get_debounce_period_us(info));
+}
+
+static PyObject *chip_get_line_info(chip_object *self, PyObject *args)
+{
+	struct gpiod_line_info *info;
+	unsigned int offset;
+	PyObject *info_obj;
+	int ret, watch;
+
+	ret = PyArg_ParseTuple(args, "Ip", &offset, &watch);
+	if (!ret)
+		return NULL;
+
+	Py_BEGIN_ALLOW_THREADS;
+	if (watch)
+		info = gpiod_chip_watch_line_info(self->chip, offset);
+	else
+		info = gpiod_chip_get_line_info(self->chip, offset);
+	Py_END_ALLOW_THREADS;
+	if (!info)
+		return Py_gpiod_SetErrFromErrno();
+
+	info_obj = make_line_info(info);
+	gpiod_line_info_free(info);
+	return info_obj;
+}
+
+static PyObject *
+chip_unwatch_line_info(chip_object *self, PyObject *args)
+{
+	unsigned int offset;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "I", &offset);
+	if (!ret)
+		return NULL;
+
+	Py_BEGIN_ALLOW_THREADS;
+	ret = gpiod_chip_unwatch_line_info(self->chip, offset);
+	Py_END_ALLOW_THREADS;
+	if (ret)
+		return Py_gpiod_SetErrFromErrno();
+
+	Py_RETURN_NONE;
+}
+
+static PyObject *
+chip_read_info_event(chip_object *self, PyObject *Py_UNUSED(ignored))
+{
+	PyObject *type, *info_obj, *event_obj;
+	struct gpiod_info_event *event;
+	struct gpiod_line_info *info;
+
+	type = Py_gpiod_GetGlobalType("InfoEvent");
+	if (!type)
+		return NULL;
+
+	Py_BEGIN_ALLOW_THREADS;
+	event = gpiod_chip_read_info_event(self->chip);
+	Py_END_ALLOW_THREADS;
+	if (!event)
+		return Py_gpiod_SetErrFromErrno();
+
+	info = gpiod_info_event_get_line_info(event);
+
+	info_obj = make_line_info(info);
+	if (!info_obj) {
+		gpiod_info_event_free(event);
+		return NULL;
+	}
+
+	event_obj = PyObject_CallFunction(type, "iKO",
+				gpiod_info_event_get_event_type(event),
+				gpiod_info_event_get_timestamp_ns(event),
+				info_obj);
+	Py_DECREF(info_obj);
+	gpiod_info_event_free(event);
+	return event_obj;
+}
+
+static PyObject *chip_map_line(chip_object *self, PyObject *args)
+{
+	int ret, offset;
+	char *name;
+
+	ret = PyArg_ParseTuple(args, "s", &name);
+	if (!ret)
+		return NULL;
+
+	Py_BEGIN_ALLOW_THREADS;
+	offset = gpiod_chip_get_line_offset_from_name(self->chip, name);
+	Py_END_ALLOW_THREADS;
+	if (offset < 0)
+		return Py_gpiod_SetErrFromErrno();
+
+	return PyLong_FromLong(offset);
+}
+
+static struct gpiod_request_config *
+make_request_config(PyObject *consumer_obj, PyObject *event_buffer_size_obj)
+{
+	struct gpiod_request_config *req_cfg;
+	size_t event_buffer_size;
+	const char *consumer;
+
+	req_cfg = gpiod_request_config_new();
+	if (!req_cfg) {
+		Py_gpiod_SetErrFromErrno();
+		return NULL;
+	}
+
+	if (consumer_obj != Py_None) {
+		consumer = PyUnicode_AsUTF8(consumer_obj);
+		if (!consumer) {
+			gpiod_request_config_free(req_cfg);
+			return NULL;
+		}
+
+		gpiod_request_config_set_consumer(req_cfg, consumer);
+	}
+
+	if (event_buffer_size_obj != Py_None) {
+		event_buffer_size = PyLong_AsSize_t(event_buffer_size_obj);
+		if (PyErr_Occurred()) {
+			gpiod_request_config_free(req_cfg);
+			return NULL;
+		}
+
+		gpiod_request_config_set_event_buffer_size(req_cfg,
+							   event_buffer_size);
+	}
+
+	return req_cfg;
+}
+
+static PyObject *chip_request_lines(chip_object *self, PyObject *args)
+{
+	PyObject *line_config, *consumer, *event_buffer_size, *req_obj;
+	struct gpiod_request_config *req_cfg;
+	struct gpiod_line_config *line_cfg;
+	struct gpiod_line_request *request;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "OOO",
+			       &line_config, &consumer, &event_buffer_size);
+	if (!ret)
+		return NULL;
+
+	line_cfg = Py_gpiod_LineConfigGetData(line_config);
+	if (!line_cfg)
+		return NULL;
+
+	req_cfg = make_request_config(consumer, event_buffer_size);
+	if (!req_cfg)
+		return NULL;
+
+	Py_BEGIN_ALLOW_THREADS;
+	request = gpiod_chip_request_lines(self->chip, req_cfg, line_cfg);
+	Py_END_ALLOW_THREADS;
+	gpiod_request_config_free(req_cfg);
+	if (!request)
+		return Py_gpiod_SetErrFromErrno();
+
+	req_obj = Py_gpiod_MakeRequestObject(request);
+	if (!req_obj)
+		gpiod_line_request_release(request);
+
+	return req_obj;
+}
+
+static PyMethodDef chip_methods[] = {
+	{
+		.ml_name = "close",
+		.ml_meth = (PyCFunction)chip_close,
+		.ml_flags = METH_NOARGS,
+	},
+	{
+		.ml_name = "get_info",
+		.ml_meth = (PyCFunction)chip_get_info,
+		.ml_flags = METH_NOARGS,
+	},
+	{
+		.ml_name = "get_line_info",
+		.ml_meth = (PyCFunction)chip_get_line_info,
+		.ml_flags = METH_VARARGS,
+	},
+	{
+		.ml_name = "unwatch_line_info",
+		.ml_meth = (PyCFunction)chip_unwatch_line_info,
+		.ml_flags = METH_VARARGS,
+	},
+	{
+		.ml_name = "read_info_event",
+		.ml_meth = (PyCFunction)chip_read_info_event,
+		.ml_flags = METH_NOARGS,
+	},
+	{
+		.ml_name = "map_line",
+		.ml_meth = (PyCFunction)chip_map_line,
+		.ml_flags = METH_VARARGS,
+	},
+	{
+		.ml_name = "request_lines",
+		.ml_meth = (PyCFunction)chip_request_lines,
+		.ml_flags = METH_VARARGS,
+	},
+	{ }
+};
+
+PyTypeObject chip_type = {
+	PyVarObject_HEAD_INIT(NULL, 0)
+	.tp_name = "gpiod._ext.Chip",
+	.tp_basicsize = sizeof(chip_object),
+	.tp_flags = Py_TPFLAGS_DEFAULT,
+	.tp_new = PyType_GenericNew,
+	.tp_init = (initproc)chip_init,
+	.tp_finalize = (destructor)chip_finalize,
+	.tp_dealloc = (destructor)Py_gpiod_dealloc,
+	.tp_getset = chip_getset,
+	.tp_methods = chip_methods,
+};
bindings/python/gpiod/ext/line-settings.c new file +11 -0
new file mode 100644
index 0000000..bd2a66a
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+#include "internal.h"
+
+typedef struct {
+	PyObject_HEAD;
+	struct gpiod_line_settings *settings;
+} line_settings_object;
+
+static int set_int_prop(struct gpiod_line_settings *settings, int val,

bindings/python/gpiod/ext/line-settings.c line 21

Quoted 10 lines
bindings/python/gpiod/ext/line-settings.c new file +10 -0
+			int (*func)(struct gpiod_line_settings *, int))
+{
+	int ret;
+
+	ret = func(settings, val);
+	if (ret) {
+		Py_gpiod_SetErrFromErrno();
+		return -1;
+	}
+

Since the ret value is never used, skip the ret and just test the func return directly?

Similarly elsewhere.

Quoted 13 lines
bindings/python/gpiod/ext/line-settings.c new file +13 -0
+	return 0;
+}
+
+static int
+line_settings_init(line_settings_object *self, PyObject *args, PyObject *kwargs)
+{
+	static char *kwlist[] = {
+		"direction",
+		"edge_detection",
+		"bias",
+		"drive",
+		"active_low",
+		"debounce_period",

bindings/python/gpiod/ext/line-settings.c line 44

Quoted 10 lines
bindings/python/gpiod/ext/line-settings.c new file +10 -0
+		"event_clock",
+		"output_value",
+		NULL
+	};
+
+	int direction, edge, bias, drive, active_low, event_clock, output_value,
+	    ret;
+	unsigned long debounce_period;
+
+	ret = PyArg_ParseTupleAndKeywords(args, kwargs, "IIIIpdII", kwlist,
                                                          ^
                                                     IIIIpkII

d is a double, k is an unsigned long

Quoted 148 lines
bindings/python/gpiod/ext/line-settings.c new file +86 -0
+			&direction, &edge, &bias, &drive, &active_low,
+			&debounce_period, &event_clock, &output_value);
+	if (!ret)
+		return -1;
+
+	self->settings = gpiod_line_settings_new();
+	if (!self->settings) {
+		Py_gpiod_SetErrFromErrno();
+		return -1;
+	}
+
+	ret = set_int_prop(self->settings, direction,
+			   gpiod_line_settings_set_direction);
+	if (ret)
+		return -1;
+
+	ret = set_int_prop(self->settings, edge,
+			   gpiod_line_settings_set_edge_detection);
+	if (ret)
+		return -1;
+
+	ret = set_int_prop(self->settings, bias,
+			   gpiod_line_settings_set_bias);
+	if (ret)
+		return -1;
+
+	ret = set_int_prop(self->settings, drive,
+			   gpiod_line_settings_set_drive);
+	if (ret)
+		return -1;
+
+	gpiod_line_settings_set_active_low(self->settings, active_low);
+	gpiod_line_settings_set_debounce_period_us(self->settings,
+						   debounce_period);
+
+	ret = set_int_prop(self->settings, edge,
+			   gpiod_line_settings_set_edge_detection);
+	if (ret)
+		return -1;
+
+	ret = set_int_prop(self->settings, output_value,
+			   gpiod_line_settings_set_output_value);
+	if (ret)
+		return -1;
+
+	return 0;
+}
+
+static void line_settings_finalize(line_settings_object *self)
+{
+	if (self->settings)
+		gpiod_line_settings_free(self->settings);
+}
+
+PyTypeObject line_settings_type = {
+	PyVarObject_HEAD_INIT(NULL, 0)
+	.tp_name = "gpiod._ext.LineSettings",
+	.tp_basicsize = sizeof(line_settings_object),
+	.tp_flags = Py_TPFLAGS_DEFAULT,
+	.tp_new = PyType_GenericNew,
+	.tp_init = (initproc)line_settings_init,
+	.tp_finalize = (destructor)line_settings_finalize,
+	.tp_dealloc = (destructor)Py_gpiod_dealloc,
+};
+
+struct gpiod_line_settings *Py_gpiod_LineSettingsGetData(PyObject *obj)
+{
+	line_settings_object *settings;
+	PyObject *type;
+
+	type = PyObject_Type(obj);
+	if (!type)
+		return NULL;
+
+	if ((PyTypeObject *)type != &line_settings_type) {
+		PyErr_SetString(PyExc_TypeError,
+				"not a gpiod._ext.LineSettings object");
+		Py_DECREF(type);
+		return NULL;
+	}
+	Py_DECREF(type);
+
+	settings = (line_settings_object *)obj;
+
+	return settings->settings;
+}
bindings/python/gpiod/ext/request.c new file +56 -0
new file mode 100644
index 0000000..36b5b48
@@ -0,0 +1,402 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+// SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+#include "internal.h"
+
+typedef struct {
+	PyObject_HEAD;
+	struct gpiod_line_request *request;
+	unsigned int *offsets;
+	int *values;
+	size_t num_lines;
+} request_object;
+
+static int request_init(PyObject *Py_UNUSED(ignored0),
+			PyObject *Py_UNUSED(ignored1),
+			PyObject *Py_UNUSED(ignored2))
+{
+	PyErr_SetString(PyExc_NotImplementedError,
+			"_ext.LineRequest cannot be instantiated");
+
+	return -1;
+}
+
+static void request_finalize(request_object *self)
+{
+	if (self->request)
+		PyObject_CallMethod((PyObject *)self, "release", "");
+
+	if (self->offsets)
+		PyMem_Free(self->offsets);
+
+	if (self->values)
+		PyMem_Free(self->values);
+}
+
+static PyObject *
+request_num_lines(request_object *self, void *Py_UNUSED(ignored))
+{
+	return PyLong_FromUnsignedLong(
+			gpiod_line_request_get_num_lines(self->request));
+}
+
+static PyObject *request_offsets(request_object *self, void *Py_UNUSED(ignored))
+{
+	PyObject *lines, *line;
+	unsigned int *offsets;
+	size_t num_lines, i;
+	int ret;
+
+	num_lines = gpiod_line_request_get_num_lines(self->request);
+
+	offsets = PyMem_Calloc(num_lines, sizeof(unsigned int));
+	if (!offsets)
+		return PyErr_NoMemory();
+
+	gpiod_line_request_get_offsets(self->request, offsets);

bindings/python/gpiod/ext/request.c line 66

Quoted 10 lines
bindings/python/gpiod/ext/request.c new file +10 -0
+
+	lines = PyList_New(num_lines);
+	if (!lines) {
+		PyMem_Free(offsets);
+		return NULL;
+	}
+
+	for (i = 0; i < num_lines; i++) {
+		line = PyLong_FromUnsignedLong(offsets[i]);
+		if (!lines) {
                 ^

Should be line.

Quoted 96 lines
bindings/python/gpiod/ext/request.c new file +96 -0
+			Py_DECREF(lines);
+			PyMem_Free(offsets);
+			return NULL;
+		}
+
+		ret = PyList_SetItem(lines, i, line);
+		if (ret) {
+			Py_DECREF(line);
+			Py_DECREF(lines);
+			PyMem_Free(offsets);
+			return NULL;
+		}
+	}
+
+	PyMem_Free(offsets);
+	return lines;
+}
+
+static PyObject *request_fd(request_object *self, void *Py_UNUSED(ignored))
+{
+	return PyLong_FromLong(gpiod_line_request_get_fd(self->request));
+}
+
+static PyGetSetDef request_getset[] = {
+	{
+		.name = "num_lines",
+		.get = (getter)request_num_lines,
+	},
+	{
+		.name = "offsets",
+		.get = (getter)request_offsets,
+	},
+	{
+		.name = "fd",
+		.get = (getter)request_fd,
+	},
+	{ }
+};
+
+static PyObject *
+request_release(request_object *self, PyObject *Py_UNUSED(ignored))
+{
+	Py_BEGIN_ALLOW_THREADS;
+	gpiod_line_request_release(self->request);
+	Py_END_ALLOW_THREADS;
+	self->request = NULL;
+
+	Py_RETURN_NONE;
+}
+
+static void clear_buffers(request_object *self)
+{
+	memset(self->offsets, 0, self->num_lines * sizeof(unsigned int));
+	memset(self->values, 0, self->num_lines * sizeof(int));
+}
+
+static PyObject *request_get_values(request_object *self, PyObject *args)
+{
+	PyObject *offsets, *values, *val, *type, *iter, *next;
+	Py_ssize_t num_offsets;
+	unsigned int pos;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "OO", &offsets, &values);
+	if (!ret)
+		return NULL;
+
+	num_offsets = PyObject_Size(offsets);
+	if (num_offsets < 0)
+		return NULL;
+
+	type = Py_gpiod_GetGlobalType("Value");
+	if (!type)
+		return NULL;
+
+	iter = PyObject_GetIter(offsets);
+	if (!iter)
+		return NULL;
+
+	clear_buffers(self);
+
+	for (pos = 0;; pos++) {
+		next = PyIter_Next(iter);
+		if (!next) {
+			Py_DECREF(iter);
+			break;
+		}
+
+		self->offsets[pos] = Py_gpiod_PyLongAsUnsignedInt(next);
+		Py_DECREF(next);
+		if (PyErr_Occurred()) {
+			Py_DECREF(iter);
+			return NULL;
+		}
+	}
+

bindings/python/gpiod/ext/request.c line 172

Quoted 10 lines
bindings/python/gpiod/ext/request.c new file +10 -0
+	Py_BEGIN_ALLOW_THREADS;
+	ret = gpiod_line_request_get_values_subset(self->request,
+						   self->num_lines,
+						   self->offsets,
+						   self->values);
+	Py_END_ALLOW_THREADS;
+	if (ret)
+		return Py_gpiod_SetErrFromErrno();
+
+	for (pos = 0; pos < num_offsets; pos++) {

gpiod/ext/request.c:172:20: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘Py_ssize_t’ {aka ‘int’} [-Wsign-compare]
172 | for (pos = 0; pos < num_offsets; pos++) {
| ^

Quoted 94 lines
bindings/python/gpiod/ext/request.c new file +94 -0
+		val = PyObject_CallFunction(type, "i", self->values[pos]);
+		if (!val)
+			return NULL;
+
+		ret = PyList_SetItem(values, pos, val);
+		if (ret) {
+			Py_DECREF(val);
+			return NULL;
+		}
+	}
+
+	Py_RETURN_NONE;
+}
+
+static PyObject *request_set_values(request_object *self, PyObject *args)
+{
+	PyObject *values, *key, *val, *val_stripped;
+	Py_ssize_t pos = 0;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "O", &values);
+	if (!ret)
+		return NULL;
+
+	clear_buffers(self);
+
+	while (PyDict_Next(values, &pos, &key, &val)) {
+		self->offsets[pos] = Py_gpiod_PyLongAsUnsignedInt(key);
+		if (PyErr_Occurred())
+			return NULL;
+
+		val_stripped = PyObject_GetAttrString(val, "value");
+		if (!val_stripped)
+			return NULL;
+
+		self->values[pos] = PyLong_AsLong(val_stripped);
+		Py_DECREF(val_stripped);
+		if (PyErr_Occurred())
+			return NULL;
+	}
+
+	Py_BEGIN_ALLOW_THREADS;
+	ret = gpiod_line_request_set_values_subset(self->request,
+						   self->num_lines,
+						   self->offsets,
+						   self->values);
+	Py_END_ALLOW_THREADS;
+	if (ret)
+		return Py_gpiod_SetErrFromErrno();
+
+	Py_RETURN_NONE;
+}
+
+static PyObject *request_reconfigure_lines(request_object *self, PyObject *args)
+{
+	struct gpiod_line_config *line_cfg;
+	PyObject *line_cfg_obj;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "O", &line_cfg_obj);
+	if (!ret)
+		return NULL;
+
+	line_cfg = Py_gpiod_LineConfigGetData(line_cfg_obj);
+	if (!line_cfg)
+		return NULL;
+
+	Py_BEGIN_ALLOW_THREADS;
+	ret = gpiod_line_request_reconfigure_lines(self->request, line_cfg);
+	Py_END_ALLOW_THREADS;
+	if (ret)
+		return Py_gpiod_SetErrFromErrno();
+
+	Py_RETURN_NONE;
+}
+
+static PyObject *request_read_edge_event(request_object *self, PyObject *args)
+{
+	PyObject *max_events_obj, *event_obj, *events, *type;
+	struct gpiod_edge_event_buffer *buffer;
+	size_t max_events, num_events, i;
+	struct gpiod_edge_event *event;
+	int ret;
+
+	ret = PyArg_ParseTuple(args, "O", &max_events_obj);
+	if (!ret)
+		return NULL;
+
+	if (max_events_obj != Py_None) {
+		max_events = PyLong_AsSize_t(max_events_obj);
+		if (PyErr_Occurred())
+			return NULL;
+	} else {
+		max_events = 64;

bindings/python/gpiod/ext/request.c line 276

Quoted 10 lines
bindings/python/gpiod/ext/request.c new file +10 -0
+	}
+
+	type = Py_gpiod_GetGlobalType("EdgeEvent");
+	if (!type)
+		return NULL;
+
+	buffer = gpiod_edge_event_buffer_new(max_events);
+	if (!buffer)
+		return Py_gpiod_SetErrFromErrno();
+

A new buffer every time?
Maybe cache it in the request_object for next time?

Quoted 148 lines
bindings/python/gpiod/ext/request.c new file +126 -0
+	Py_BEGIN_ALLOW_THREADS;
+	ret = gpiod_line_request_read_edge_event(self->request,
+						 buffer, max_events);
+	Py_END_ALLOW_THREADS;
+	if (ret < 0) {
+		gpiod_edge_event_buffer_free(buffer);
+		return NULL;
+	}
+
+	num_events = ret;
+
+	events = PyList_New(num_events);
+	if (!events) {
+		gpiod_edge_event_buffer_free(buffer);
+		return NULL;
+	}
+
+	for (i = 0; i < num_events; i++) {
+		event = gpiod_edge_event_buffer_get_event(buffer, i);
+		if (!event) {
+			gpiod_edge_event_buffer_free(buffer);
+			Py_DECREF(events);
+			return NULL;
+		}
+
+		event_obj = PyObject_CallFunction(type, "iKiii",
+				gpiod_edge_event_get_event_type(event),
+				gpiod_edge_event_get_timestamp_ns(event),
+				gpiod_edge_event_get_line_offset(event),
+				gpiod_edge_event_get_global_seqno(event),
+				gpiod_edge_event_get_line_seqno(event));
+		if (!event_obj) {
+			gpiod_edge_event_buffer_free(buffer);
+			Py_DECREF(events);
+			return NULL;
+		}
+
+		ret = PyList_SetItem(events, i, event_obj);
+		if (ret) {
+			gpiod_edge_event_buffer_free(buffer);
+			Py_DECREF(event_obj);
+			Py_DECREF(events);
+			return NULL;
+		}
+	}
+
+	gpiod_edge_event_buffer_free(buffer);
+
+	return events;
+}
+
+static PyMethodDef request_methods[] = {
+	{
+		.ml_name = "release",
+		.ml_meth = (PyCFunction)request_release,
+		.ml_flags = METH_NOARGS,
+	},
+	{
+		.ml_name = "get_values",
+		.ml_meth = (PyCFunction)request_get_values,
+		.ml_flags = METH_VARARGS,
+	},
+	{
+		.ml_name = "set_values",
+		.ml_meth = (PyCFunction)request_set_values,
+		.ml_flags = METH_VARARGS,
+	},
+	{
+		.ml_name = "reconfigure_lines",
+		.ml_meth = (PyCFunction)request_reconfigure_lines,
+		.ml_flags = METH_VARARGS,
+	},
+	{
+		.ml_name = "read_edge_event",
+		.ml_meth = (PyCFunction)request_read_edge_event,
+		.ml_flags = METH_VARARGS,
+	},
+	{ }
+};
+
+PyTypeObject request_type = {
+	PyVarObject_HEAD_INIT(NULL, 0)
+	.tp_name = "gpiod._ext.Request",
+	.tp_basicsize = sizeof(request_object),
+	.tp_flags = Py_TPFLAGS_DEFAULT,
+	.tp_new = PyType_GenericNew,
+	.tp_init = (initproc)request_init,
+	.tp_finalize = (destructor)request_finalize,
+	.tp_dealloc = (destructor)Py_gpiod_dealloc,
+	.tp_getset = request_getset,
+	.tp_methods = request_methods,
+};
+
+PyObject *Py_gpiod_MakeRequestObject(struct gpiod_line_request *request)
+{
+	request_object *req_obj;
+	unsigned int *offsets;
+	size_t num_lines;
+	int *values;
+
+	num_lines = gpiod_line_request_get_num_lines(request);
+
+	req_obj = PyObject_New(request_object, &request_type);
+	if (!req_obj)
+		return NULL;
+
+	offsets = PyMem_Calloc(num_lines, sizeof(unsigned int));
+	if (!offsets) {
+		Py_DECREF(req_obj);
+		return NULL;
+	}
+
+	values = PyMem_Calloc(num_lines, sizeof(int));
+	if (!values) {
+		PyMem_Free(offsets);
+		Py_DECREF(req_obj);
+		return NULL;
+	}
+
+	req_obj->request = request;
+	req_obj->offsets = offsets;
+	req_obj->values = values;
+	req_obj->num_lines = num_lines;
+
+	return (PyObject *)req_obj;
+}
bindings/python/gpiod/line_info.py new file +16 -0
new file mode 100644
index 0000000..9a6c9bf
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from . import _ext
+from dataclasses import dataclass
+from datetime import timedelta
+from gpiod.line import Direction, Bias, Drive, Edge, Clock
+
+
+@dataclass(frozen=True, init=False, repr=False)
+class LineInfo:
+    """
+    Snapshot of a line's status.
+    """
+
+    offset: int

bindings/python/gpiod/line_info.py line 26

Quoted 10 lines
bindings/python/gpiod/line_info.py new file +10 -0
+    name: str
+    used: bool
+    consumer: str
+    direction: Direction
+    active_low: bool
+    bias: Bias
+    drive: Drive
+    edge_detection: Edge
+    event_clock: Clock
+    debounced: bool

I've probably asked this before, but does debounced differ from not debounce_period?

Quoted 79 lines
bindings/python/gpiod/line_info.py new file +47 -0
+    debounce_period: timedelta
+
+    def __init__(
+        self,
+        offset: int,
+        name: str,
+        used: bool,
+        consumer: str,
+        direction: int,
+        active_low: bool,
+        bias: int,
+        drive: int,
+        edge_detection: int,
+        event_clock: int,
+        debounced: bool,
+        debounce_period_us: int,
+    ):
+        object.__setattr__(self, "offset", offset)
+        object.__setattr__(self, "name", name)
+        object.__setattr__(self, "used", used)
+        object.__setattr__(self, "consumer", consumer)
+        object.__setattr__(self, "direction", Direction(direction))
+        object.__setattr__(self, "active_low", active_low)
+        object.__setattr__(self, "bias", Bias(bias))
+        object.__setattr__(self, "drive", Drive(drive))
+        object.__setattr__(self, "edge_detection", Edge(edge_detection))
+        object.__setattr__(self, "event_clock", Clock(event_clock))
+        object.__setattr__(self, "debounced", debounced)
+        object.__setattr__(
+            self, "debounce_period", timedelta(microseconds=debounce_period_us)
+        )
+
+    def __str__(self):
+        return '<LineInfo offset={} name="{}" used={} consumer="{}" direction={} active_low={} bias={} drive={} edge_detection={} event_clock={} debounced={} debounce_period={}>'.format(
+            self.offset,
+            self.name,
+            self.used,
+            self.consumer,
+            self.direction,
+            self.active_low,
+            self.bias,
+            self.drive,
+            self.edge_detection,
+            self.event_clock,
+            self.debounced,
+            self.debounce_period,
+        )
bindings/python/gpiod/line_request.py new file +26 -0
new file mode 100644
index 0000000..a3ee392
@@ -0,0 +1,258 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from . import _ext
+from .edge_event import EdgeEvent
+from .exception import RequestReleasedError
+from .internal import poll_fd
+from .line import Value
+from .line_settings import LineSettings, _line_settings_to_ext_line_settings
+from collections.abc import Iterable
+from datetime import timedelta
+from typing import Optional, Union
+
+
+class LineRequest:
+    """
+    Stores the context of a set of requested GPIO lines.
+    """
+
+    def __init__(self, req: _ext.Request):
+        """
+        DON'T USE
+
+        LineRequest objects can only be instantiated by a Chip parent. This is
+        not part of stable API.
+        """

bindings/python/gpiod/line_request.py line 36

Quoted 10 lines
bindings/python/gpiod/line_request.py new file +10 -0
+        self._req = req
+
+    def __bool__(self) -> bool:
+        """
+        Boolean conversion for GPIO line requests.
+
+        Returns:
+          True if the request is live and False if it's been released.
+        """
+        return True if self._req else False

return self._req

Quoted 48 lines
bindings/python/gpiod/line_request.py new file +48 -0
+
+    def __enter__(self):
+        """
+        Controlled execution enter callback.
+        """
+        self._check_released()
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        """
+        Controlled execution exit callback.
+        """
+        self.release()
+
+    def _check_released(self) -> None:
+        if not self._req:
+            raise RequestReleasedError()
+
+    def release(self) -> None:
+        """
+        Release this request and free all associated resources. The object must
+        not be used after a call to this method.
+        """
+        self._check_released()
+        self._req.release()
+        self._req = None
+
+    def get_value(self, line: Union[int, str]) -> Value:
+        """
+        Get a single GPIO line value.
+
+        Args:
+          line:
+            Offset or name of the line to get value for.
+
+        Returns:
+          Logical value of the line.
+        """
+        return self.get_values([line])[0]
+
+    def get_values(
+        self, lines: Optional[Iterable[Union[int, str]]] = None
+    ) -> list[Value]:
+        """
+        Get values of a set of GPIO lines.
+
+        Args:
+          lines:

bindings/python/gpiod/line_request.py line 94

Quoted 10 lines
bindings/python/gpiod/line_request.py new file +10 -0
+            List of names or offsets of GPIO lines to get values for. Can be
+            None in which case all requested lines will be read.
+
+        Returns:
+          List of logical line values.
+        """
+        self._check_released()
+
+        if lines is None:
+            lines = self._lines

What if lines is empty? Is that equivalent to None, or an error? If the former...

lines = lines or self._lines

either here or where used below (if that changed to a list comprehension)

bindings/python/gpiod/line_request.py line 106

Quoted 12 lines
bindings/python/gpiod/line_request.py new file +12 -0
+
+        offsets = [None] * len(lines)
+
+        for i, line in enumerate(lines):
+            if isinstance(line, str):
+                if line not in self._name_map:
+                    raise ValueError("unknown line name: {}".format(line))
+
+                offsets[i] = self._name_map[line]
+            else:
+                offsets[i] = line
+

I would do this with a list comprehension and a helper function to do the id to offset mapping. i.e.

offsets = [ offset_from_id(id) for id in lines ]

Quoted 59 lines
bindings/python/gpiod/line_request.py new file +59 -0
+        buf = [None] * len(lines)
+
+        self._req.get_values(offsets, buf)
+        return buf
+
+    def set_value(self, line: Union[int, str], value: Value) -> None:
+        """
+        Set the value of a single GPIO line.
+
+        Args:
+          line:
+            Offset or name of the line to set.
+          value:
+            New value.
+        """
+        self.set_values({line: value})
+
+    def set_values(self, values: dict[Union[int, str], Value]) -> None:
+        """
+        Set the values of a subset of GPIO lines.
+
+        Args:
+          values:
+            Dictionary mapping line offsets or names to desired values.
+        """
+        self._check_released()
+
+        mapped = dict()
+        for i, line in enumerate(values):
+            if isinstance(line, str):
+                if line not in self._name_map:
+                    raise ValueError("unknown line name: {}".format(line))
+
+                mapped[self._name_map[line]] = values[line]
+            else:
+                mapped[line] = values[line]
+
+        self._req.set_values(mapped)
+
+    def reconfigure_lines(
+        self, config: dict[tuple[Union[int, str]], LineSettings]
+    ) -> None:
+        """
+        Reconfigure requested lines.
+
+        Args:
+          config
+            Dictionary mapping offsets or names (or tuples thereof) to
+            LineSettings. If None is passed as the value of the mapping,
+            default settings are used.
+        """
+        self._check_released()
+
+        line_cfg = _ext.LineConfig()
+
+        for lines, settings in config.items():
+            if isinstance(lines, int) or isinstance(lines, str):
+                lines = [lines]
+

bindings/python/gpiod/line_request.py line 175

Quoted 10 lines
bindings/python/gpiod/line_request.py new file +10 -0
+            offsets = [None] * len(lines)
+
+            for i, line in enumerate(lines):
+                if isinstance(line, str):
+                    if line not in self._name_map:
+                        raise ValueError("unknown line name: {}".format(line))
+
+                    offsets[i] = self._name_map[line]
+                else:
+                    offsets[i] = line

And again...
offsets = [ offset_from_id(id) for id in lines ]

Quoted 141 lines
bindings/python/gpiod/line_request.py new file +83 -0
+
+            line_cfg.add_line_settings(
+                offsets, _line_settings_to_ext_line_settings(settings)
+            )
+
+        self._req.reconfigure_lines(line_cfg)
+
+    def wait_edge_event(
+        self, timeout: Optional[Union[timedelta, float]] = None
+    ) -> bool:
+        """
+        Wait for edge events on any of the requested lines.
+
+        Args:
+          timeout:
+            Wait time limit expressed as either a datetime.timedelta object
+            or the number of seconds stored in a float.
+
+        Returns:
+          True if events are ready to be read. False on timeout.
+        """
+        self._check_released()
+
+        return poll_fd(self.fd, timeout)
+
+    def read_edge_event(self, max_events: Optional[int] = None) -> list[EdgeEvent]:
+        """
+        Read a number of edge events from a line request.
+
+        Args:
+          max_events:
+            Maximum number of events to read.
+
+        Returns:
+          List of read EdgeEvent objects.
+        """
+        self._check_released()
+
+        return self._req.read_edge_event(max_events)
+
+    def __str__(self):
+        """
+        Return a user-friendly, human-readable description of this request.
+        """
+        if not self._req:
+            return "<LineRequest RELEASED>"
+
+        return "<LineRequest num_lines={} offsets={} fd={}>".format(
+            self.num_lines, self.offsets, self.fd
+        )
+
+    @property
+    def num_lines(self) -> int:
+        """
+        Number of requested lines.
+        """
+        self._check_released()
+        return len(self._offsets)
+
+    @property
+    def offsets(self) -> list[int]:
+        """
+        List of requested offsets. Lines requested by name are mapped to their
+        offsets.
+        """
+        self._check_released()
+        return self._offsets
+
+    @property
+    def lines(self) -> list[Union[int, str]]:
+        """
+        List of requested lines. Lines requested by name are listed as such.
+        """
+        self._check_released()
+        return self._lines
+
+    @property
+    def fd(self) -> int:
+        """
+        File descriptor associated with this request.
+        """
+        self._check_released()
+        return self._req.fd
bindings/python/gpiod/line_settings.py new file +52 -0
new file mode 100644
index 0000000..1315b0c
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from . import _ext
+from dataclasses import dataclass
+from datetime import timedelta
+from gpiod.line import Direction, Bias, Drive, Edge, Clock, Value
+
+
+@dataclass(repr=False)
+class LineSettings:
+    """
+    Stores a set of line properties.
+    """
+
+    direction: Direction = Direction.AS_IS
+    edge_detection: Edge = Edge.NONE
+    bias: Bias = Bias.AS_IS
+    drive: Drive = Drive.PUSH_PULL
+    active_low: bool = False
+    debounce_period: timedelta = timedelta()
+    event_clock: Clock = Clock.MONOTONIC
+    output_value: Value = Value.INACTIVE
+
+    # __repr__ generated by @dataclass uses repr for enum members resulting in
+    # an unusable representation as those are of the form: <NAME: $value>
+    def __repr__(self):
+        return "LineSettings(direction={}, edge_detection={} bias={} drive={} active_low={} debounce_period={} event_clock={} output_value={})".format(
+            str(self.direction),
+            str(self.edge_detection),
+            str(self.bias),
+            str(self.drive),
+            self.active_low,
+            repr(self.debounce_period),
+            str(self.event_clock),
+            str(self.output_value),
+        )
+
+    def __str__(self):
+        return "<LineSettings direction={} edge_detection={} bias={} drive={} active_low={} debounce_period={} event_clock={} output_value={}>".format(
+            self.direction,
+            self.edge_detection,
+            self.bias,
+            self.drive,
+            self.active_low,
+            self.debounce_period,
+            self.event_clock,
+            self.output_value,
+        )
+
+
+def _line_settings_to_ext_line_settings(settings: LineSettings) -> _ext.LineSettings:

bindings/python/gpiod/line_settings.py line 62

Quoted 10 lines
bindings/python/gpiod/line_settings.py new file +10 -0
+    return _ext.LineSettings(
+        direction=settings.direction.value,
+        edge_detection=settings.edge_detection.value,
+        bias=settings.bias.value,
+        drive=settings.drive.value,
+        active_low=settings.active_low,
+        debounce_period=int(settings.debounce_period.total_seconds() * 1000000),
+        event_clock=settings.event_clock.value,
+        output_value=settings.output_value.value,
+    )

As mentioned earlier, rename to _line_settings_to_ext??

bindings/python/setup.py

Quoted 4 lines
bindings/python/setup.py new file +0 -1
new file mode 100644
index 0000000..ec8f99d
--- /dev/null

Ok, the API is much nicer than the previous versions, so that is all good, given some clarification on the config semantics.

Other than the couple of PyArg_ParseTupleAndKeywords issues, the other few minor bugs, and my obvious preference for using list comprehensions to build lists, the big sticking point for me is thread safety.
I'm not convinced your current approach is thread safe, so convince me, either by proving me wrong or providing another solution. Hopefully I'm wrong.

Cheers,
Kent.

#23 lore raw

Re: [libgpiod v2][PATCH v3 4/4] bindings: python: implement python bindings for libgpiod v2

Quoted 12 lines · Kent Gibson
Quoted 8 lines · Bartosz Golaszewski

This adds python bindings for libgpiod v2. As opposed to v1, they are mostly written in python with just low-level elements written in C and interfacing with libgpiod.so.

We've also added setup.py which will allow to use pip for managing the bindings and split them into a separate meta-openembedded recipe.

<snipping out files with no probs>

Quoted 8 lines

Other than the couple of PyArg_ParseTupleAndKeywords issues, the other few minor bugs, and my obvious preference for using list comprehensions to build lists, the big sticking point for me is thread safety.
I'm not convinced your current approach is thread safe, so convince me, either by proving me wrong or providing another solution. Hopefully I'm wrong.

But it isn't expected to be MT-safe, is it?

Cheers,
Kent.

#24 lore raw

Re: [libgpiod v2][PATCH v3 4/4] bindings: python: implement python bindings for libgpiod v2

In reply to Kent Gibson
Quoted 11 lines · Kent Gibson
Quoted 8 lines · Bartosz Golaszewski

This adds python bindings for libgpiod v2. As opposed to v1, they are mostly written in python with just low-level elements written in C and interfacing with libgpiod.so.

We've also added setup.py which will allow to use pip for managing the bindings and split them into a separate meta-openembedded recipe.

<snip - all those cut are done>

Quoted 7 lines

Should ids fallback to being interpreted as ints if they can't be found as strings? Why not leave that call to the user?
If they aren't sure they can try it as a string, and if that fails try it as an int.
(I realise this is the reverse argument of my comment in the examples patch btw - playing devil's advocate here - and there.)

I've thought about it and I think that the current behavior makes sense in Python where things should "just work" out of the box. If anything, we can add a flag no_implicit_conv or something.

Quoted 2 lines

Is the range check on ints necessary? The kernel will do that when you make the call - what is the benefit of doing it here?

That way we won't make it seem like we correctly mapped the name?

Quoted 3 lines

In which case why accept int at all? Which would make the function offset_from_name() - just like the C.

This was your idea in the first place. :)

The reasoning was - it's python, so let's use its dynamic typing and make the interface pretty lax.

Quoted 19 lines
Quoted 19 lines
bindings/python/gpiod/chip.py new file +19 -0
+        """
+        self._check_closed()
+
+        if not isinstance(id, int):
+            try:
+                return self._chip.map_line(id)
+            except OSError as ex:
+                if ex.errno == ENOENT:
+                    try:
+                        offset = int(id)
+                    except ValueError:
+                        raise ex
+                else:
+                    raise ex
+        else:
+            offset = id
+
+        if offset >= self.get_info().num_lines:
+            raise ValueError("line offset of out range")
Quoted 9 lines

bindings/python/gpiod/chip.py line 139

Quoted 5 lines
bindings/python/gpiod/chip.py new file +5 -0
+
+        return offset
+
+    def _get_line_info(self, line: Union[int, str], watch: bool) -> LineInfo:
+        self._check_closed()

Wrt the _check_closed(), what is to prevent another thread closing the chip here, and causing the next line to segfault?

Nothing, but we don't guarantee thread-safety like you mentioned in your response below.

Quoted 3 lines

And self.map_line(line) can release the GIL so all bets are off even if you assume cooperative multitasking - not that you can assume that.

Yes but that's up to the user to assure correct concurrent access to libgpiod objects. Are there any benefits of trying to assure thread-safety in the library code?

<snip>

Quoted 11 lines
Quoted 11 lines
bindings/python/gpiod/chip.py new file +11 -0
+
+    def request_lines(
+        self,
+        config: dict[tuple[Union[int, str]], Optional[LineSettings]],
+        consumer: Optional[str] = None,
+        event_buffer_size: Optional[int] = None,
+    ) -> LineRequest:
+        """
+        Request a set of lines for exclusive usage.
+
+        Args:
Quoted 9 lines

bindings/python/gpiod/chip.py line 226

Quoted 4 lines
bindings/python/gpiod/chip.py new file +4 -0
+          config:
+            Dictionary mapping offsets or names (or tuples thereof) to
+            LineSettings. If None is passed as the value of the mapping,
+            default settings are used.

What are the semantics for lines being repeated in the dict keys (which is possible as the keys are tuples, and also because the line can be identified by offset or name)?

Good point, I will clear that up in the docs and add test cases.

Quoted 20 lines
Quoted 20 lines
bindings/python/gpiod/chip.py new file +20 -0
+          consumer:
+            Consumer string to use for this request.
+          event_buffer_size:
+            Size of the kernel edge event buffer to configure for this request.
+
+        Returns:
+          New LineRequest object.
+        """
+        self._check_closed()
+
+        line_cfg = _ext.LineConfig()
+
+        for lines, settings in config.items():
+            offsets = list()
+            name_map = dict()
+            offset_map = dict()
+
+            if isinstance(lines, int) or isinstance(lines, str):
+                lines = (lines,)
+
Quoted 9 lines

bindings/python/gpiod/chip.py line 252

Quoted 6 lines
bindings/python/gpiod/chip.py new file +6 -0
+            for line in lines:
+                offset = self.map_line(line)
+                offsets.append(offset)
+                if isinstance(line, str):
+                    name_map[line] = offset
+                    offset_map[offset] = line

Use list comprehensions instead of the for loop?

Not sure it's worth it - we would essentially iterate thrice to fill in every container here?

Quoted 11 lines

bindings/python/gpiod/chip.py line 256

Quoted 4 lines
bindings/python/gpiod/chip.py new file +4 -0
+
+            if settings is None:
+                settings = LineSettings()
+

settings = settings or LineSettings()

Would use that directly below if it would fit in the line.

Rename _line_settings_to_ext_line_settings to _line_settings_to_ext, as the second line_settings is redundant? Then it might fit.

Done

Quoted 8 lines
Quoted 8 lines
bindings/python/gpiod/chip.py new file +8 -0
+            line_cfg.add_line_settings(
+                offsets, _line_settings_to_ext_line_settings(settings)
+            )
+
+        req_internal = self._chip.request_lines(line_cfg, consumer, event_buffer_size)
+        request = LineRequest(req_internal)
+
+        request._offsets = req_internal.offsets
Quoted 9 lines

bindings/python/gpiod/chip.py line 270

Quoted 6 lines
bindings/python/gpiod/chip.py new file +6 -0
+        request._name_map = name_map
+        request._offset_map = offset_map
+
+        request._lines = list()
+        for off in request.offsets:
+            request._lines.append(offset_map[off] if off in offset_map else off)

Again, prefer to use list comprehensions to translate lists like this.

Done

<snip>

Quoted 10 lines
Quoted 10 lines
bindings/python/gpiod/ext/chip.c new file +10 -0
+
+static int
+chip_init(chip_object *self, PyObject *args, PyObject *Py_UNUSED(ignored))
+{
+     char *path;
+     int ret;
+
+     ret = PyArg_ParseTuple(args, "s", &path);
+     if (!ret)
+             return -1;
Quoted 9 lines

bindings/python/gpiod/ext/chip.c line 23

Quoted 4 lines
bindings/python/gpiod/ext/chip.c new file +4 -0
+
+     Py_BEGIN_ALLOW_THREADS;
+     self->chip = gpiod_chip_open(path);
+     Py_END_ALLOW_THREADS;

Is the assignment to self->chip while not holding the GIL (between the BEGIN and END) safe?
To be sure, assign to a local then assign that to self->chip after the END.

Good point.

Quoted 7 lines

Similarly other BEGIN/END blocks should only access locals. And even then you probably need locks to prevent threads trampling each other, but the locks need the GIL, IIUC.

So I suspect your chip._check_closed()s need to be more like
with chip._lock()

I agree that the C code should not crash - so it makes sense to not assign self->chip with GIL released. But for thread-safety in general
- I don't think the module should care about it. Just like C and C++ libs leave it to the user. The only thing we should care about is not keeping any global state that could cause problems in multithreaded apps.

Quoted 81 lines

Similarly the other object validity checks.

Quoted 66 lines
bindings/python/gpiod/ext/chip.c new file +66 -0
+     if (!self->chip) {
+             Py_gpiod_SetErrFromErrno();
+             return -1;
+     }
+
+     return 0;
+}
+
+static void chip_finalize(chip_object *self)
+{
+     if (self->chip)
+             PyObject_CallMethod((PyObject *)self, "close", "");
+}
+
+static PyObject *chip_path(chip_object *self, void *Py_UNUSED(ignored))
+{
+     return PyUnicode_FromString(gpiod_chip_get_path(self->chip));
+}
+
+static PyObject *chip_fd(chip_object *self, void *Py_UNUSED(ignored))
+{
+     return PyLong_FromLong(gpiod_chip_get_fd(self->chip));
+}
+
+static PyGetSetDef chip_getset[] = {
+     {
+             .name = "path",
+             .get = (getter)chip_path,
+     },
+     {
+             .name = "fd",
+             .get = (getter)chip_fd,
+     },
+     { }
+};
+
+static PyObject *chip_close(chip_object *self, PyObject *Py_UNUSED(ignored))
+{
+     Py_BEGIN_ALLOW_THREADS;
+     gpiod_chip_close(self->chip);
+     Py_END_ALLOW_THREADS;
+     self->chip = NULL;
+
+     Py_RETURN_NONE;
+}
+
+static PyObject *chip_get_info(chip_object *self, PyObject *Py_UNUSED(ignored))
+{
+     struct gpiod_chip_info *info;
+     PyObject *type, *ret;
+
+     type = Py_gpiod_GetGlobalType("ChipInfo");
+     if (!type)
+             return NULL;
+
+     info = gpiod_chip_get_info(self->chip);
+     if (!info)
+             return PyErr_SetFromErrno(PyExc_OSError);
+
+      ret = PyObject_CallFunction(type, "ssI",
+                                  gpiod_chip_info_get_name(info),
+                                  gpiod_chip_info_get_label(info),
+                                  gpiod_chip_info_get_num_lines(info));
+      gpiod_chip_info_free(info);
+      return ret;
+}

bindings/python/gpiod/ext/chip.c line 99

Quoted 10 lines
bindings/python/gpiod/ext/chip.c new file +10 -0
+
+static PyObject *make_line_info(struct gpiod_line_info *info)
+{
+     PyObject *type;
+
+     type = Py_gpiod_GetGlobalType("LineInfo");
+     if (!type)
+             return NULL;
+
+     return PyObject_CallFunction(type, "IsOsiOiiiiOi",
                                                   ^
                                       "IsOsiOiiiiOk"
Quoted 2 lines

debounce_period_us is an unsigned long, not int.

Good catch.

<snip>

Quoted 16 lines

bindings/python/gpiod/ext/line-settings.c line 21

Quoted 12 lines
bindings/python/gpiod/ext/line-settings.c new file +12 -0
+
+static int set_int_prop(struct gpiod_line_settings *settings, int val,
+                     int (*func)(struct gpiod_line_settings *, int))
+{
+     int ret;
+
+     ret = func(settings, val);
+     if (ret) {
+             Py_gpiod_SetErrFromErrno();
+             return -1;
+     }
+

Since the ret value is never used, skip the ret and just test the func return directly?

Quoted 2 lines

Similarly elsewhere.

This is just for consistency. It looks better and clearer IMO.

Quoted 26 lines
Quoted 13 lines
bindings/python/gpiod/ext/line-settings.c new file +13 -0
+     return 0;
+}
+
+static int
+line_settings_init(line_settings_object *self, PyObject *args, PyObject *kwargs)
+{
+     static char *kwlist[] = {
+             "direction",
+             "edge_detection",
+             "bias",
+             "drive",
+             "active_low",
+             "debounce_period",

bindings/python/gpiod/ext/line-settings.c line 44

Quoted 10 lines
bindings/python/gpiod/ext/line-settings.c new file +10 -0
+             "event_clock",
+             "output_value",
+             NULL
+     };
+
+     int direction, edge, bias, drive, active_low, event_clock, output_value,
+         ret;
+     unsigned long debounce_period;
+
+     ret = PyArg_ParseTupleAndKeywords(args, kwargs, "IIIIpdII", kwlist,
                                                          ^
                                                     IIIIpkII
Quoted 2 lines

d is a double, k is an unsigned long

Done

<snip!>

Quoted 25 lines
Quoted 14 lines
bindings/python/gpiod/ext/request.c new file +14 -0
+static PyObject *request_offsets(request_object *self, void *Py_UNUSED(ignored))
+{
+     PyObject *lines, *line;
+     unsigned int *offsets;
+     size_t num_lines, i;
+     int ret;
+
+     num_lines = gpiod_line_request_get_num_lines(self->request);
+
+     offsets = PyMem_Calloc(num_lines, sizeof(unsigned int));
+     if (!offsets)
+             return PyErr_NoMemory();
+
+     gpiod_line_request_get_offsets(self->request, offsets);

bindings/python/gpiod/ext/request.c line 66

Quoted 10 lines
bindings/python/gpiod/ext/request.c new file +10 -0
+
+     lines = PyList_New(num_lines);
+     if (!lines) {
+             PyMem_Free(offsets);
+             return NULL;
+     }
+
+     for (i = 0; i < num_lines; i++) {
+             line = PyLong_FromUnsignedLong(offsets[i]);
+             if (!lines) {
                 ^
Quoted 2 lines

Should be line.

Thanks!

<snip>

Quoted 4 lines

A new buffer every time?
Maybe cache it in the request_object for next time?

I was thinking about it and then forgot, thanks!

Quoted 153 lines
Quoted 153 lines
bindings/python/gpiod/ext/request.c new file +126 -0
+     Py_BEGIN_ALLOW_THREADS;
+     ret = gpiod_line_request_read_edge_event(self->request,
+                                              buffer, max_events);
+     Py_END_ALLOW_THREADS;
+     if (ret < 0) {
+             gpiod_edge_event_buffer_free(buffer);
+             return NULL;
+     }
+
+     num_events = ret;
+
+     events = PyList_New(num_events);
+     if (!events) {
+             gpiod_edge_event_buffer_free(buffer);
+             return NULL;
+     }
+
+     for (i = 0; i < num_events; i++) {
+             event = gpiod_edge_event_buffer_get_event(buffer, i);
+             if (!event) {
+                     gpiod_edge_event_buffer_free(buffer);
+                     Py_DECREF(events);
+                     return NULL;
+             }
+
+             event_obj = PyObject_CallFunction(type, "iKiii",
+                             gpiod_edge_event_get_event_type(event),
+                             gpiod_edge_event_get_timestamp_ns(event),
+                             gpiod_edge_event_get_line_offset(event),
+                             gpiod_edge_event_get_global_seqno(event),
+                             gpiod_edge_event_get_line_seqno(event));
+             if (!event_obj) {
+                     gpiod_edge_event_buffer_free(buffer);
+                     Py_DECREF(events);
+                     return NULL;
+             }
+
+             ret = PyList_SetItem(events, i, event_obj);
+             if (ret) {
+                     gpiod_edge_event_buffer_free(buffer);
+                     Py_DECREF(event_obj);
+                     Py_DECREF(events);
+                     return NULL;
+             }
+     }
+
+     gpiod_edge_event_buffer_free(buffer);
+
+     return events;
+}
+
+static PyMethodDef request_methods[] = {
+     {
+             .ml_name = "release",
+             .ml_meth = (PyCFunction)request_release,
+             .ml_flags = METH_NOARGS,
+     },
+     {
+             .ml_name = "get_values",
+             .ml_meth = (PyCFunction)request_get_values,
+             .ml_flags = METH_VARARGS,
+     },
+     {
+             .ml_name = "set_values",
+             .ml_meth = (PyCFunction)request_set_values,
+             .ml_flags = METH_VARARGS,
+     },
+     {
+             .ml_name = "reconfigure_lines",
+             .ml_meth = (PyCFunction)request_reconfigure_lines,
+             .ml_flags = METH_VARARGS,
+     },
+     {
+             .ml_name = "read_edge_event",
+             .ml_meth = (PyCFunction)request_read_edge_event,
+             .ml_flags = METH_VARARGS,
+     },
+     { }
+};
+
+PyTypeObject request_type = {
+     PyVarObject_HEAD_INIT(NULL, 0)
+     .tp_name = "gpiod._ext.Request",
+     .tp_basicsize = sizeof(request_object),
+     .tp_flags = Py_TPFLAGS_DEFAULT,
+     .tp_new = PyType_GenericNew,
+     .tp_init = (initproc)request_init,
+     .tp_finalize = (destructor)request_finalize,
+     .tp_dealloc = (destructor)Py_gpiod_dealloc,
+     .tp_getset = request_getset,
+     .tp_methods = request_methods,
+};
+
+PyObject *Py_gpiod_MakeRequestObject(struct gpiod_line_request *request)
+{
+     request_object *req_obj;
+     unsigned int *offsets;
+     size_t num_lines;
+     int *values;
+
+     num_lines = gpiod_line_request_get_num_lines(request);
+
+     req_obj = PyObject_New(request_object, &request_type);
+     if (!req_obj)
+             return NULL;
+
+     offsets = PyMem_Calloc(num_lines, sizeof(unsigned int));
+     if (!offsets) {
+             Py_DECREF(req_obj);
+             return NULL;
+     }
+
+     values = PyMem_Calloc(num_lines, sizeof(int));
+     if (!values) {
+             PyMem_Free(offsets);
+             Py_DECREF(req_obj);
+             return NULL;
+     }
+
+     req_obj->request = request;
+     req_obj->offsets = offsets;
+     req_obj->values = values;
+     req_obj->num_lines = num_lines;
+
+     return (PyObject *)req_obj;
+}
bindings/python/gpiod/line_info.py new file +21 -0
new file mode 100644
index 0000000..9a6c9bf
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: LGPL-2.1-or-later
+# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <[email protected]>
+
+from . import _ext
+from dataclasses import dataclass
+from datetime import timedelta
+from gpiod.line import Direction, Bias, Drive, Edge, Clock
+
+
+@dataclass(frozen=True, init=False, repr=False)
+class LineInfo:
+    """
+    Snapshot of a line's status.
+    """
+
+    offset: int
+    name: str
+    used: bool
+    consumer: str
+    direction: Direction
+    active_low: bool
Quoted 9 lines

bindings/python/gpiod/line_info.py line 26

Quoted 5 lines
bindings/python/gpiod/line_info.py new file +5 -0
+    bias: Bias
+    drive: Drive
+    edge_detection: Edge
+    event_clock: Clock
+    debounced: bool

I've probably asked this before, but does debounced differ from not debounce_period?

Nope, just consistency with the rest of tree. I'll think whether it's really useful to not have it.

<snip>

Quoted 11 lines
Quoted 11 lines
bindings/python/gpiod/line_info.py new file +11 -0
+    def __init__(self, req: _ext.Request):
+        """
+        DON'T USE
+
+        LineRequest objects can only be instantiated by a Chip parent. This is
+        not part of stable API.
+        """
+        self._req = req
+
+    def __bool__(self) -> bool:
+        """
Quoted 8 lines

bindings/python/gpiod/line_info.py line 43

Quoted 6 lines
bindings/python/gpiod/line_info.py new file +6 -0
+        Boolean conversion for GPIO line requests.
+
+        Returns:
+          True if the request is live and False if it's been released.
+        """
+        return True if self._req else False

return self._req

But this will not convert to bool. The typing hint is just it - a hint. I would be returning the internal request object.

Quoted 67 lines
Quoted 49 lines
bindings/python/gpiod/line_info.py new file +48 -0
+
+    def __enter__(self):
+        """
+        Controlled execution enter callback.
+        """
+        self._check_released()
+        return self
+
+    def __exit__(self, exc_type, exc_value, traceback):
+        """
+        Controlled execution exit callback.
+        """
+        self.release()
+
+    def _check_released(self) -> None:
+        if not self._req:
+            raise RequestReleasedError()
+
+    def release(self) -> None:
+        """
+        Release this request and free all associated resources. The object must
+        not be used after a call to this method.
+        """
+        self._check_released()
+        self._req.release()
+        self._req = None
+
+    def get_value(self, line: Union[int, str]) -> Value:
+        """
+        Get a single GPIO line value.
+
+        Args:
+          line:
+            Offset or name of the line to get value for.
+
+        Returns:
+          Logical value of the line.
+        """
+        return self.get_values([line])[0]
+
+    def get_values(
+        self, lines: Optional[Iterable[Union[int, str]]] = None
+    ) -> list[Value]:
+        """
+        Get values of a set of GPIO lines.
+
+        Args:
+          lines:

bindings/python/gpiod/line_info.py

Quoted 10 lines
bindings/python/gpiod/line_info.py new file +10 -0
+            List of names or offsets of GPIO lines to get values for. Can be
+            None in which case all requested lines will be read.
+
+        Returns:
+          List of logical line values.
+        """
+        self._check_released()
+
+        if lines is None:
+            lines = self._lines

What if lines is empty? Is that equivalent to None, or an error? If the former...

lines = lines or self._lines

either here or where used below (if that changed to a list comprehension)

Quoted 16 lines

bindings/python/gpiod/line_info.py

Quoted 12 lines
bindings/python/gpiod/line_info.py new file +12 -0
+
+        offsets = [None] * len(lines)
+
+        for i, line in enumerate(lines):
+            if isinstance(line, str):
+                if line not in self._name_map:
+                    raise ValueError("unknown line name: {}".format(line))
+
+                offsets[i] = self._name_map[line]
+            else:
+                offsets[i] = line
+

I would do this with a list comprehension and a helper function to do the id to offset mapping. i.e.

Used list and dict comprehensions where possible.

<snip>

Quoted 4 lines

Ok, the API is much nicer than the previous versions, so that is all good, given some clarification on the config semantics.

Quoted 8 lines

Other than the couple of PyArg_ParseTupleAndKeywords issues, the other few minor bugs, and my obvious preference for using list comprehensions to build lists, the big sticking point for me is thread safety.
I'm not convinced your current approach is thread safe, so convince me, either by proving me wrong or providing another solution. Hopefully I'm wrong.

It's not thread-safe. Why would it be? What if you use gpiod and a module_x which also uses gpiod internally? Then you lock some mutex in gpiod and module_x deadlocks with the same mutex? It should be left to the user to handle threads.

Thanks a lot for the thorough review. v4 will be going out shortly!

Bart

Quoted 2 lines

Cheers,
Kent.

#25 lore raw

Re: [libgpiod v2][PATCH v3 4/4] bindings: python: implement python bindings for libgpiod v2

Quoted 11 lines · Bartosz Golaszewski
Quoted 2 lines · Kent Gibson

On Fri, Oct 07, 2022 at 04:55:21PM +0200, Bartosz Golaszewski wrote:

I agree that the C code should not crash - so it makes sense to not assign self->chip with GIL released. But for thread-safety in general
- I don't think the module should care about it. Just like C and C++ libs leave it to the user. The only thing we should care about is not keeping any global state that could cause problems in multithreaded apps.

Yeah, agreed - no locking or thread safety guarantees - we don't want to add the overhead of locking, and the user can provide their own locking wrapper if they need it, probably at request scope.
As long we we don't go messing with Python objects outside the GIL, or sharing state between requests (and we don't) then we should be fine.

Sorry for the confusion there on my part - I think I was flipping back and forth between Python and Rust and carried some of my Rust Sync concerns over to the Python.

Cheers,
Kent.

#26 lore raw
In reply to Bartosz Golaszewski
Quoted 120 lines · Bartosz Golaszewski

This is the third iteration of python bindings for libgpiod but it really has very little in common with the previous version.

This time the code has been split into high-level python and low-level C layers with the latter only doing the bare minimum.

The data model is mostly based on the C++ one with the main difference being utilizing dynamic typing and keyword arguments in place of the builder pattern. That allows us to reduce the number of methods and objects.

Because python doesn't have RAII, unlike C++, we provide a module-level request_lines() helper as gpiod.Chip(path).request_lines(...) one-liner could lead to the chip left dangling even after the last reference is dropped.

Because python forces us to dynamically allocate objects all the time even for fundamental types (which are also immutable) there's no point in trying to replicate the edge-event buffer. Instead LineRequest.read_edge_event() just returns a list of events.

Bartosz Golaszewski (4):
bindings: python: remove old version
bindings: python: add examples
bindings: python: add tests
bindings: python: implement python bindings for libgpiod v2

 bindings/python/.gitignore                   |    8 +
 bindings/python/Makefile.am                  |   26 +-
 bindings/python/examples/Makefile.am         |   16 +-
 bindings/python/examples/gpiodetect.py       |   15 +-
 bindings/python/examples/gpiofind.py         |   14 +-
 bindings/python/examples/gpioget.py          |   34 +-
 bindings/python/examples/gpioinfo.py         |   41 +-
 bindings/python/examples/gpiomon.py          |   52 +-
 bindings/python/examples/gpioset.py          |   46 +-
 bindings/python/gpiod/Makefile.am            |   17 +
 bindings/python/gpiod/__init__.py            |   53 +
 bindings/python/gpiod/chip.py                |  308 ++
 bindings/python/gpiod/chip_info.py           |   21 +
 bindings/python/gpiod/edge_event.py          |   46 +
 bindings/python/gpiod/exception.py           |   20 +
 bindings/python/gpiod/ext/Makefile.am        |   11 +
 bindings/python/gpiod/ext/chip.c             |  335 +++
 bindings/python/gpiod/ext/common.c           |   92 +
 bindings/python/gpiod/ext/internal.h         |   20 +
 bindings/python/gpiod/ext/line-config.c      |  133 +
 bindings/python/gpiod/ext/line-settings.c    |  130 +
 bindings/python/gpiod/ext/module.c           |  193 ++
 bindings/python/gpiod/ext/request.c          |  402 +++
 bindings/python/gpiod/info_event.py          |   33 +
 bindings/python/gpiod/internal.py            |   19 +
 bindings/python/gpiod/line.py                |   56 +
 bindings/python/gpiod/line_info.py           |   73 +
 bindings/python/gpiod/line_request.py        |  258 ++
 bindings/python/gpiod/line_settings.py       |   62 +
 bindings/python/gpiodmodule.c                | 2662 ------------------
 bindings/python/setup.py                     |   47 +
 bindings/python/tests/Makefile.am            |   26 +-
 bindings/python/tests/__init__.py            |   17 +
 bindings/python/tests/__main__.py            |   16 +
 bindings/python/tests/gpiod_py_test.py       |  832 ------
 bindings/python/tests/gpiomockupmodule.c     |  309 --
 bindings/python/tests/gpiosim/Makefile.am    |    7 +
 bindings/python/tests/gpiosim/__init__.py    |    4 +
 bindings/python/tests/gpiosim/chip.py        |   66 +
 bindings/python/tests/gpiosim/ext.c          |  345 +++
 bindings/python/tests/helpers.py             |   16 +
 bindings/python/tests/tests_chip.py          |  231 ++
 bindings/python/tests/tests_chip_info.py     |   52 +
 bindings/python/tests/tests_edge_event.py    |  219 ++
 bindings/python/tests/tests_info_event.py    |  189 ++
 bindings/python/tests/tests_line_info.py     |  101 +
 bindings/python/tests/tests_line_request.py  |  449 +++
 bindings/python/tests/tests_line_settings.py |   79 +
 bindings/python/tests/tests_module.py        |   59 +
 configure.ac                                 |    3 +
 50 files changed, 4338 insertions(+), 3925 deletions(-)
 create mode 100644 bindings/python/.gitignore
 create mode 100644 bindings/python/gpiod/Makefile.am
 create mode 100644 bindings/python/gpiod/__init__.py
 create mode 100644 bindings/python/gpiod/chip.py
 create mode 100644 bindings/python/gpiod/chip_info.py
 create mode 100644 bindings/python/gpiod/edge_event.py
 create mode 100644 bindings/python/gpiod/exception.py
 create mode 100644 bindings/python/gpiod/ext/Makefile.am
 create mode 100644 bindings/python/gpiod/ext/chip.c
 create mode 100644 bindings/python/gpiod/ext/common.c
 create mode 100644 bindings/python/gpiod/ext/internal.h
 create mode 100644 bindings/python/gpiod/ext/line-config.c
 create mode 100644 bindings/python/gpiod/ext/line-settings.c
 create mode 100644 bindings/python/gpiod/ext/module.c
 create mode 100644 bindings/python/gpiod/ext/request.c
 create mode 100644 bindings/python/gpiod/info_event.py
 create mode 100644 bindings/python/gpiod/internal.py
 create mode 100644 bindings/python/gpiod/line.py
 create mode 100644 bindings/python/gpiod/line_info.py
 create mode 100644 bindings/python/gpiod/line_request.py
 create mode 100644 bindings/python/gpiod/line_settings.py
 delete mode 100644 bindings/python/gpiodmodule.c
 create mode 100644 bindings/python/setup.py
 create mode 100644 bindings/python/tests/__init__.py
 create mode 100644 bindings/python/tests/__main__.py
 delete mode 100755 bindings/python/tests/gpiod_py_test.py
 delete mode 100644 bindings/python/tests/gpiomockupmodule.c
 create mode 100644 bindings/python/tests/gpiosim/Makefile.am
 create mode 100644 bindings/python/tests/gpiosim/__init__.py
 create mode 100644 bindings/python/tests/gpiosim/chip.py
 create mode 100644 bindings/python/tests/gpiosim/ext.c
 create mode 100644 bindings/python/tests/helpers.py
 create mode 100644 bindings/python/tests/tests_chip.py
 create mode 100644 bindings/python/tests/tests_chip_info.py
 create mode 100644 bindings/python/tests/tests_edge_event.py
 create mode 100644 bindings/python/tests/tests_info_event.py
 create mode 100644 bindings/python/tests/tests_line_info.py
 create mode 100644 bindings/python/tests/tests_line_request.py
 create mode 100644 bindings/python/tests/tests_line_settings.py
 create mode 100644 bindings/python/tests/tests_module.py
Quoted 3 lines

--
2.34.1

I fixed the one nit from Andy. If there are no objections I'd like to apply this and squash the entire v2 patch series into one big commit and apply it to the master branch. This way we can stop keeping this temporary branch and continue the development (tools, rust, possible further tweaks to the API) on master.

Bart

#27 lore raw
Quoted 23 lines · Bartosz Golaszewski
Quoted 22 lines · Bartosz Golaszewski

This is the third iteration of python bindings for libgpiod but it really has very little in common with the previous version.

This time the code has been split into high-level python and low-level C layers with the latter only doing the bare minimum.

The data model is mostly based on the C++ one with the main difference being utilizing dynamic typing and keyword arguments in place of the builder pattern. That allows us to reduce the number of methods and objects.

Because python doesn't have RAII, unlike C++, we provide a module-level request_lines() helper as gpiod.Chip(path).request_lines(...) one-liner could lead to the chip left dangling even after the last reference is dropped.

Because python forces us to dynamically allocate objects all the time even for fundamental types (which are also immutable) there's no point in trying to replicate the edge-event buffer. Instead LineRequest.read_edge_event() just returns a list of events.

Quoted 9 lines
Quoted 2 lines

Bartosz Golaszewski (4):

I fixed the one nit from Andy. If there are no objections I'd like to apply this and squash the entire v2 patch series into one big commit and apply it to the master branch. This way we can stop keeping this temporary branch and continue the development (tools, rust, possible further tweaks to the API) on master.

I'm in the process of reviewing, so hold off for a bit if you can.

If not, at the very least IIIIpdII -> IIIIpkII in patch 4.

Otherwise you get this on 32 bit platforms:

$ gpioget.py /dev/gpiochip0 17
*** stack smashing detected ***: terminated

Cheers,
Kent.

#28 lore raw
Quoted 35 lines · Kent Gibson
Quoted 23 lines · Bartosz Golaszewski
Quoted 22 lines · Bartosz Golaszewski

This is the third iteration of python bindings for libgpiod but it really has very little in common with the previous version.

This time the code has been split into high-level python and low-level C layers with the latter only doing the bare minimum.

The data model is mostly based on the C++ one with the main difference being utilizing dynamic typing and keyword arguments in place of the builder pattern. That allows us to reduce the number of methods and objects.

Because python doesn't have RAII, unlike C++, we provide a module-level request_lines() helper as gpiod.Chip(path).request_lines(...) one-liner could lead to the chip left dangling even after the last reference is dropped.

Because python forces us to dynamically allocate objects all the time even for fundamental types (which are also immutable) there's no point in trying to replicate the edge-event buffer. Instead LineRequest.read_edge_event() just returns a list of events.

Quoted 9 lines
Quoted 2 lines

Bartosz Golaszewski (4):

I fixed the one nit from Andy. If there are no objections I'd like to apply this and squash the entire v2 patch series into one big commit and apply it to the master branch. This way we can stop keeping this temporary branch and continue the development (tools, rust, possible further tweaks to the API) on master.

Quoted 9 lines

I'm in the process of reviewing, so hold off for a bit if you can.

If not, at the very least IIIIpdII -> IIIIpkII in patch 4.

Otherwise you get this on 32 bit platforms:

$ gpioget.py /dev/gpiochip0 17
*** stack smashing detected ***: terminated

I'll wait in this case, thanks. Wanted to start testing the new tools but thought about getting this behind us first.

Bart

#29 lore raw
Quoted 40 lines · Bartosz Golaszewski
Quoted 39 lines · Kent Gibson
Quoted 23 lines · Bartosz Golaszewski
Quoted 22 lines · Bartosz Golaszewski

This is the third iteration of python bindings for libgpiod but it really has very little in common with the previous version.

This time the code has been split into high-level python and low-level C layers with the latter only doing the bare minimum.

The data model is mostly based on the C++ one with the main difference being utilizing dynamic typing and keyword arguments in place of the builder pattern. That allows us to reduce the number of methods and objects.

Because python doesn't have RAII, unlike C++, we provide a module-level request_lines() helper as gpiod.Chip(path).request_lines(...) one-liner could lead to the chip left dangling even after the last reference is dropped.

Because python forces us to dynamically allocate objects all the time even for fundamental types (which are also immutable) there's no point in trying to replicate the edge-event buffer. Instead LineRequest.read_edge_event() just returns a list of events.

Quoted 9 lines
Quoted 2 lines

Bartosz Golaszewski (4):

I fixed the one nit from Andy. If there are no objections I'd like to apply this and squash the entire v2 patch series into one big commit and apply it to the master branch. This way we can stop keeping this temporary branch and continue the development (tools, rust, possible further tweaks to the API) on master.

I'm in the process of reviewing, so hold off for a bit if you can.

If not, at the very least IIIIpdII -> IIIIpkII in patch 4.

Quoted 9 lines
Quoted 5 lines

Otherwise you get this on 32 bit platforms:

$ gpioget.py /dev/gpiochip0 17
*** stack smashing detected ***: terminated

I'll wait in this case, thanks. Wanted to start testing the new tools but thought about getting this behind us first.

The idea of getting the tools patch out was to keep you busy while I reviewed the python. Unfortunately I got bogged down in corner cases so that took much longer to get out than I had intended.

I only started looking at the python today, and I don't want to commit to a completion time at this stage, but shouldn't be too long.

So go play with the tools for a bit.

Cheers,
Kent.