| Message ID | 20220628084226.472035-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | bindings: implement python bindings for libgpiod v2 | expand |
On Tue, Jul 5, 2022 at 4:08 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Tue, Jun 28, 2022 at 10:42:25AM +0200, Bartosz Golaszewski wrote: > > This adds a python wrapper around libgpiosim and a set of test cases > > for the v2 API using python's standard unittest module. > > > > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> > > --- > > bindings/python/tests/Makefile.am | 14 + > > bindings/python/tests/cases/__init__.py | 12 + > > bindings/python/tests/cases/tests_chip.py | 157 +++++++ > > .../python/tests/cases/tests_chip_info.py | 59 +++ > > .../python/tests/cases/tests_edge_event.py | 279 +++++++++++ > > .../python/tests/cases/tests_info_event.py | 135 ++++++ > > .../python/tests/cases/tests_line_config.py | 254 ++++++++++ > > .../python/tests/cases/tests_line_info.py | 90 ++++ > > .../python/tests/cases/tests_line_request.py | 345 ++++++++++++++ > > bindings/python/tests/cases/tests_misc.py | 53 +++ > > .../tests/cases/tests_request_config.py | 77 ++++ > > bindings/python/tests/gpiod_py_test.py | 25 + > > bindings/python/tests/gpiosimmodule.c | 434 ++++++++++++++++++ > > 13 files changed, 1934 insertions(+) > > create mode 100644 bindings/python/tests/Makefile.am > > create mode 100644 bindings/python/tests/cases/__init__.py > > create mode 100644 bindings/python/tests/cases/tests_chip.py > > create mode 100644 bindings/python/tests/cases/tests_chip_info.py > > create mode 100644 bindings/python/tests/cases/tests_edge_event.py > > create mode 100644 bindings/python/tests/cases/tests_info_event.py > > create mode 100644 bindings/python/tests/cases/tests_line_config.py > > create mode 100644 bindings/python/tests/cases/tests_line_info.py > > create mode 100644 bindings/python/tests/cases/tests_line_request.py > > create mode 100644 bindings/python/tests/cases/tests_misc.py > > create mode 100644 bindings/python/tests/cases/tests_request_config.py > > create mode 100755 bindings/python/tests/gpiod_py_test.py > > create mode 100644 bindings/python/tests/gpiosimmodule.c > > > > diff --git a/bindings/python/tests/Makefile.am b/bindings/python/tests/Makefile.am > > new file mode 100644 > > index 0000000..099574f > > --- /dev/null > > +++ b/bindings/python/tests/Makefile.am > > @@ -0,0 +1,14 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > It is 2022? > > Which email address are you going with? gmail here and bgdev below. > These patches will be squashed together anyway. When I wrote this part I used this email and then switched to brgl@bgdev.pl. It's just copyright anyway. I can fix it up later. [snip!] > > + > > + def test_falling_edge_event(self): > > + with gpiod.request_lines( > > + self.sim.dev_path, > > + gpiod.RequestConfig(offsets=[6]), > > + gpiod.LineConfig(edge_detection=Edge.FALLING), > > + ) as req: > > + buf = gpiod.EdgeEventBuffer() > > + self.thread = threading.Thread( > > + target=partial(self.trigger_falling_and_rising_edge, 6) > > + ) > > + self.thread.start() > > + > > Benefit of the thread? (and elsewhere a background thread is used) > The sleeps therein are only necessary because it is run in the > background. > Just to make it similar to real-life applications. I did the same for C++ and C. And no: if I triggered multiple events without any sleeps in between, then some of them would risk not being registered. You can try it for yourself with gpiosim. It happens because when the kernel irq_work is busy adding an interrupt, new ones get ignored. [nsip] > > + def test_module_line_request_direction(self): > > + sim = gpiosim.Chip(num_lines=2) > > + > > + with gpiod.request_lines( > > + sim.dev_path, lines=[0, 1], direction=Direction.OUTPUT > > + ) as req: > > + with gpiod.Chip(sim.dev_path) as chip: > > + info = chip.get_line_info(0) > > + self.assertEqual(info.direction, Direction.OUTPUT) > > + self.assertTrue(info.used) > > + > > + def test_module_line_request_edge_detection(self): > > + sim = gpiosim.Chip() > > + > > + with gpiod.request_lines( > > + sim.dev_path, lines=[0], edge_detection=Edge.BOTH > > + ) as req: > > + sim.set_pull(0, Pull.PULL_UP) > > + self.assertTrue(req.wait_edge_event()) > > + self.assertEqual(req.read_edge_event()[0].line_offset, 0) > > + > > + > > +class RequestingLinesFailsWithInvalidArguments(unittest.TestCase): > > These tests should be in tests_chip.py, as they are testing the > Chip.request_lines() method. > I would argue that there's some overlap in where the test cases should live. For instance - if we moved the line watching out of tests_info_event into tests_chip then not much would be left. I would leave these here as they test the general idea of requesting lines rather than the functionality of class LineRequest. Same for the module level line requests. > And they should have module level equivalents (don't assume one wraps > the other). > Makes sense. > > + def setUp(self): > > + self.sim = gpiosim.Chip(num_lines=8) > > + self.chip = gpiod.Chip(self.sim.dev_path) > > + > > + def tearDown(self): > > + self.chip.close() > > + self.chip = None > > + self.sim = None > > + > > + def test_passing_invalid_types_as_configs(self): > > + with self.assertRaises(TypeError): > > + self.chip.request_lines("foobar", gpiod.LineConfig()) > > + > > + with self.assertRaises(TypeError): > > + self.chip.request_lines(gpiod.RequestConfig(offsets=[0]), "foobar") > > + > > + def test_no_offsets(self): > > + with self.assertRaises(ValueError): > > + self.chip.request_lines(gpiod.RequestConfig(), gpiod.LineConfig()) > > + > > + def test_duplicate_offsets(self): > > + with self.assertRaises(OSError) as ex: > > + self.chip.request_lines( > > + gpiod.RequestConfig(offsets=[2, 5, 1, 7, 5]), gpiod.LineConfig() > > + ) > > + > > + self.assertEqual(ex.exception.errno, errno.EBUSY) > > + > > + def test_offset_out_of_range(self): > > + with self.assertRaises(ValueError): > > + self.chip.request_lines( > > + gpiod.RequestConfig(offsets=[1, 0, 4, 8]), gpiod.LineConfig() > > + ) > > + > > [snip] > > +++ b/bindings/python/tests/cases/tests_misc.py > > @@ -0,0 +1,53 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2022 Bartosz Golaszewski <brgl@bgdev.pl> > > + > > The tests in this file are all module scope, and cover functions from > module.c, so rename to tests_module.py. > > > +import gpiod > > +import gpiosim > > +import os > > +import re > > +import unittest > > + > > + > > +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) > > + > > + > > +class IsGPIOChip(unittest.TestCase): > > + def test_is_gpiochip_bad(self): > > + self.assertFalse(gpiod.is_gpiochip_device("/dev/null")) > > + self.assertFalse(gpiod.is_gpiochip_device("/dev/nonexistent")) > > + > > + def test_is_gpiochip_good(self): > > + sim = gpiosim.Chip() > > + > > + self.assertTrue(gpiod.is_gpiochip_device(sim.dev_path)) > > + > > + def test_is_gpiochip_link_good(self): > > + link = "/tmp/gpiod-py-test-link.{}".format(os.getpid()) > > + sim = gpiosim.Chip() > > + > > + with LinkGuard(sim.dev_path, link): > > + self.assertTrue(gpiod.is_gpiochip_device(link)) > > + > > + def test_is_gpiochip_link_bad(self): > > + link = "/tmp/gpiod-py-test-link.{}".format(os.getpid()) > > + > > + with LinkGuard("/dev/null", link): > > + self.assertFalse(gpiod.is_gpiochip_device(link)) > > + > > + > > +class VersionString(unittest.TestCase): > > + def test_version_string(self): > > + self.assertTrue( > > + re.match( > > + "^[0-9][1-9]?\\.[0-9][1-9]?([\\.0-9]?|\\-devel)$", gpiod.__version__ > > + ) > > + ) > > diff --git a/bindings/python/tests/cases/tests_request_config.py b/bindings/python/tests/cases/tests_request_config.py > [snip] > > A complete audit of the test coverage would be beneficial. > I haven't attempted that - only pointed out any gaps I happened to notice. > Are there any coverage tools available for Python C modules? > One can use gcov as usual. I will do this but I don't expect to have 100% coverage in the first version, we can add more test cases once this is in master. Bart
On Thu, Jul 07, 2022 at 12:17:17PM +0200, Bartosz Golaszewski wrote: > On Tue, Jul 5, 2022 at 4:08 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Tue, Jun 28, 2022 at 10:42:25AM +0200, Bartosz Golaszewski wrote: > > > This adds a python wrapper around libgpiosim and a set of test cases > > > for the v2 API using python's standard unittest module. > > > > > > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> > > > --- > > > bindings/python/tests/Makefile.am | 14 + > > > bindings/python/tests/cases/__init__.py | 12 + > > > bindings/python/tests/cases/tests_chip.py | 157 +++++++ > > > .../python/tests/cases/tests_chip_info.py | 59 +++ > > > .../python/tests/cases/tests_edge_event.py | 279 +++++++++++ > > > .../python/tests/cases/tests_info_event.py | 135 ++++++ > > > .../python/tests/cases/tests_line_config.py | 254 ++++++++++ > > > .../python/tests/cases/tests_line_info.py | 90 ++++ > > > .../python/tests/cases/tests_line_request.py | 345 ++++++++++++++ > > > bindings/python/tests/cases/tests_misc.py | 53 +++ > > > .../tests/cases/tests_request_config.py | 77 ++++ > > > bindings/python/tests/gpiod_py_test.py | 25 + > > > bindings/python/tests/gpiosimmodule.c | 434 ++++++++++++++++++ > > > 13 files changed, 1934 insertions(+) > > > create mode 100644 bindings/python/tests/Makefile.am > > > create mode 100644 bindings/python/tests/cases/__init__.py > > > create mode 100644 bindings/python/tests/cases/tests_chip.py > > > create mode 100644 bindings/python/tests/cases/tests_chip_info.py > > > create mode 100644 bindings/python/tests/cases/tests_edge_event.py > > > create mode 100644 bindings/python/tests/cases/tests_info_event.py > > > create mode 100644 bindings/python/tests/cases/tests_line_config.py > > > create mode 100644 bindings/python/tests/cases/tests_line_info.py > > > create mode 100644 bindings/python/tests/cases/tests_line_request.py > > > create mode 100644 bindings/python/tests/cases/tests_misc.py > > > create mode 100644 bindings/python/tests/cases/tests_request_config.py > > > create mode 100755 bindings/python/tests/gpiod_py_test.py > > > create mode 100644 bindings/python/tests/gpiosimmodule.c > > > > > > diff --git a/bindings/python/tests/Makefile.am b/bindings/python/tests/Makefile.am > > > new file mode 100644 > > > index 0000000..099574f > > > --- /dev/null > > > +++ b/bindings/python/tests/Makefile.am > > > @@ -0,0 +1,14 @@ > > > +# SPDX-License-Identifier: GPL-2.0-or-later > > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > > + > > > > It is 2022? > > > > Which email address are you going with? gmail here and bgdev below. > > > > These patches will be squashed together anyway. When I wrote this part > I used this email and then switched to brgl@bgdev.pl. It's just > copyright anyway. I can fix it up later. > > [snip!] > > > > + > > > + def test_falling_edge_event(self): > > > + with gpiod.request_lines( > > > + self.sim.dev_path, > > > + gpiod.RequestConfig(offsets=[6]), > > > + gpiod.LineConfig(edge_detection=Edge.FALLING), > > > + ) as req: > > > + buf = gpiod.EdgeEventBuffer() > > > + self.thread = threading.Thread( > > > + target=partial(self.trigger_falling_and_rising_edge, 6) > > > + ) > > > + self.thread.start() > > > + > > > > Benefit of the thread? (and elsewhere a background thread is used) > > The sleeps therein are only necessary because it is run in the > > background. > > > > Just to make it similar to real-life applications. I did the same for > C++ and C. And no: if I triggered multiple events without any sleeps > in between, then some of them would risk not being registered. You can > try it for yourself with gpiosim. It happens because when the kernel > irq_work is busy adding an interrupt, new ones get ignored. > I know, and I still don't think that this is the place for that. I'd rather see some example code do that. If you want to add some threaded tests in then sure, do that, but the tests do not really need it - it just makes them more complicated than you require. Sure, you can't issue multiple events on a single gpio-sim line without waiting for the result, but you never need to. You toggle a line and check the result. Toggle a line, check a result. All from the main thread. And yeah, it is a bit disconcerting that userspace can toggle the gpio-sim line faster than the interrupt handling in the kernel can manage. But I can live with that. > [nsip] > > > > > These tests should be in tests_chip.py, as they are testing the > > Chip.request_lines() method. > > > > I would argue that there's some overlap in where the test cases should > live. For instance - if we moved the line watching out of > tests_info_event into tests_chip then not much would be left. I would > leave these here as they test the general idea of requesting lines > rather than the functionality of class LineRequest. Same for the > module level line requests. > And I would argue the reverse - that overlap is imaginary. This is just basic discoverability. I looked in the tests_chip.py for the tests for Chip.request_lines(), so a method on a Chip and implemented in chip.c, and found nothing. Putting them in tests_line_request.py because that is what they construct is a wee bit unintuitive. The tests in test_line_request.py will certainly need to call Chip.request_lines(), as that is effectively the constructor, but I would only epxect to see successful Chip.request_lines() there as part of the test setup, not the test proper. All the failure cases should be in tests_chip.py, and of course some success cases as well. But that isn't overlap. In general the tests in tests_<blah>.py should be for the methods implemented in <blah>.c. In the case of InfoEvent, that might not be much, but you get that - it is a tiny module. Those tests being lonely is not a good reason to move tests in from tests_chip.c. Cheers, Kent.
This series adds python bindings for libgpiod v2. The series is split into several patches for easier review. In general the python bindings follow what we did for C++ in terms of class layout except that we leverage python's flexibility to reduce the number of method variants by allowing different types of arguments. 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 make the EdgeEventBuffer avoid copying the events like we did in C++ for better performance. Python simply isn't designed around speed. v1 -> v2: - all methods now accept keyword arguments even for mandatory positional ones - added a global request_lines() function that enables easy one-liner liner requests - improve and unify the pydoc format - many smaller tweaks and fixes Bartosz Golaszewski (5): bindings: python: remove old version bindings: python: enum: add a piece of common code for using python's enums from C bindings: python: add examples for v2 API bindings: python: add tests for v2 API bindings: python: add the implementation for v2 API bindings/python/.gitignore | 1 + bindings/python/Makefile.am | 19 +- bindings/python/chip-info.c | 126 + bindings/python/chip.c | 606 ++++ bindings/python/edge-event-buffer.c | 330 ++ bindings/python/edge-event.c | 191 ++ bindings/python/enum/Makefile.am | 9 + bindings/python/enum/enum.c | 208 ++ bindings/python/enum/enum.h | 24 + bindings/python/examples/gpiodetect.py | 13 +- bindings/python/examples/gpiofind.py | 12 +- bindings/python/examples/gpioget.py | 28 +- bindings/python/examples/gpioinfo.py | 39 +- bindings/python/examples/gpiomon.py | 53 +- bindings/python/examples/gpioset.py | 36 +- bindings/python/exception.c | 182 ++ bindings/python/gpiodmodule.c | 2662 ----------------- bindings/python/info-event.c | 175 ++ bindings/python/line-config.c | 1373 +++++++++ bindings/python/line-info.c | 286 ++ bindings/python/line-request.c | 803 +++++ bindings/python/line.c | 239 ++ bindings/python/module.c | 557 ++++ bindings/python/module.h | 58 + bindings/python/request-config.c | 320 ++ bindings/python/tests/Makefile.am | 15 +- bindings/python/tests/cases/__init__.py | 12 + bindings/python/tests/cases/tests_chip.py | 157 + .../python/tests/cases/tests_chip_info.py | 59 + .../python/tests/cases/tests_edge_event.py | 279 ++ .../python/tests/cases/tests_info_event.py | 135 + .../python/tests/cases/tests_line_config.py | 254 ++ .../python/tests/cases/tests_line_info.py | 90 + .../python/tests/cases/tests_line_request.py | 345 +++ bindings/python/tests/cases/tests_misc.py | 53 + .../tests/cases/tests_request_config.py | 77 + bindings/python/tests/gpiod_py_test.py | 827 +---- bindings/python/tests/gpiomockupmodule.c | 309 -- bindings/python/tests/gpiosimmodule.c | 434 +++ configure.ac | 3 +- 40 files changed, 7517 insertions(+), 3882 deletions(-) create mode 100644 bindings/python/.gitignore create mode 100644 bindings/python/chip-info.c create mode 100644 bindings/python/chip.c create mode 100644 bindings/python/edge-event-buffer.c create mode 100644 bindings/python/edge-event.c create mode 100644 bindings/python/enum/Makefile.am create mode 100644 bindings/python/enum/enum.c create mode 100644 bindings/python/enum/enum.h create mode 100644 bindings/python/exception.c delete mode 100644 bindings/python/gpiodmodule.c create mode 100644 bindings/python/info-event.c create mode 100644 bindings/python/line-config.c create mode 100644 bindings/python/line-info.c create mode 100644 bindings/python/line-request.c create mode 100644 bindings/python/line.c create mode 100644 bindings/python/module.c create mode 100644 bindings/python/module.h create mode 100644 bindings/python/request-config.c create mode 100644 bindings/python/tests/cases/__init__.py create mode 100644 bindings/python/tests/cases/tests_chip.py create mode 100644 bindings/python/tests/cases/tests_chip_info.py create mode 100644 bindings/python/tests/cases/tests_edge_event.py create mode 100644 bindings/python/tests/cases/tests_info_event.py create mode 100644 bindings/python/tests/cases/tests_line_config.py create mode 100644 bindings/python/tests/cases/tests_line_info.py create mode 100644 bindings/python/tests/cases/tests_line_request.py create mode 100644 bindings/python/tests/cases/tests_misc.py create mode 100644 bindings/python/tests/cases/tests_request_config.py delete mode 100644 bindings/python/tests/gpiomockupmodule.c create mode 100644 bindings/python/tests/gpiosimmodule.c