| Message ID | 20220525140704.94983-4-brgl@bgdev.pl |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bindings: implement python bindings for libgpiod v2 | expand |
On Wed, May 25, 2022 at 04:07:02PM +0200, Bartosz Golaszewski wrote: > This adds the usual set of reimplementations of gpio-tools using the new > python module. > > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> > --- > 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 | 31 +++++++++++++++++++++++ > bindings/python/examples/gpioset.py | 35 ++++++++++++++++++++++++++ > 7 files changed, 179 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 > > diff --git a/bindings/python/examples/Makefile.am b/bindings/python/examples/Makefile.am > new file mode 100644 > index 0000000..4169469 > --- /dev/null > +++ b/bindings/python/examples/Makefile.am > @@ -0,0 +1,10 @@ > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +EXTRA_DIST = \ > + gpiodetect.py \ > + gpiofind.py \ > + gpioget.py \ > + gpioinfo.py \ > + gpiomon.py \ > + gpioset.py > diff --git a/bindings/python/examples/gpiodetect.py b/bindings/python/examples/gpiodetect.py > new file mode 100755 > index 0000000..08e586b > --- /dev/null > +++ b/bindings/python/examples/gpiodetect.py > @@ -0,0 +1,17 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +"""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) > + ) > diff --git a/bindings/python/examples/gpiofind.py b/bindings/python/examples/gpiofind.py > new file mode 100755 > index 0000000..e488a49 > --- /dev/null > +++ b/bindings/python/examples/gpiofind.py > @@ -0,0 +1,20 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +"""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.get_line_offset_from_name(sys.argv[1]) > + if offset is not None: > + print("{} {}".format(chip.get_info().name, offset)) > + sys.exit(0) > + > + sys.exit(1) > diff --git a/bindings/python/examples/gpioget.py b/bindings/python/examples/gpioget.py > new file mode 100755 > index 0000000..c509f38 > --- /dev/null > +++ b/bindings/python/examples/gpioget.py > @@ -0,0 +1,31 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +"""Simplified reimplementation of the gpioget tool in Python.""" > + > +import gpiod > +import sys > + > +Direction = gpiod.Line.Direction > +Value = gpiod.Line.Value > + > +if __name__ == "__main__": > + if len(sys.argv) < 3: > + raise TypeError("usage: gpioget.py <gpiochip> <offset1> <offset2> ...") > + > + path = sys.argv[1] > + offsets = [] > + for off in sys.argv[2:]: > + offsets.append(int(off)) > + > + with gpiod.request_lines( > + path, > + gpiod.RequestConfig(offsets=offsets, consumer="gpioget.py"), > + gpiod.LineConfig(direction=Direction.INPUT), > + ) as request: With request_lines() being the primary entry point to the gpiod module, consider extending it to allow the RequestConfig and LineConfig kwargs to be passed directly to request_lines(), and for those transient objects to be constructed within request_lines(). That way the average user does not need to concern themselves with those objects and the code is easier to read. i.e. with gpiod.request_lines( path, offsets=offsets, consumer="gpioget.py", direction=Direction.INPUT, ) as request: You can still support passing in complete RequestConfig and LineConfig as kwargs for cases where the user requires complex configuration. i.e. with gpiod.request_lines( path, req_cfg=gpiod.RequestConfig(offsets=offsets, consumer="gpioget.py"), line_cfg=gpiod.LineConfig(direction=Direction.INPUT), ) as request: Or for those use cases the user could use the Chip.request_lines() (which wouldn't have the kwarg sugar) instead. Would be good to have some examples with complex configuration as well, not just the tool reimplementations. > + vals = request.get_values() > + > + for val in vals: > + print("0" if val == Value.INACTIVE else "1", end=" ") > + print() > diff --git a/bindings/python/examples/gpioinfo.py b/bindings/python/examples/gpioinfo.py > new file mode 100755 > index 0000000..3097d10 > --- /dev/null > +++ b/bindings/python/examples/gpioinfo.py > @@ -0,0 +1,35 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +"""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", > + ) > + ) > diff --git a/bindings/python/examples/gpiomon.py b/bindings/python/examples/gpiomon.py > new file mode 100755 > index 0000000..b0f4b88 > --- /dev/null > +++ b/bindings/python/examples/gpiomon.py > @@ -0,0 +1,31 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +"""Simplified reimplementation of the gpiomon tool in Python.""" > + > +import gpiod > +import sys > + > +Edge = gpiod.Line.Edge > + > +if __name__ == "__main__": > + if len(sys.argv) < 3: > + raise TypeError("usage: gpiomon.py <gpiochip> <offset1> <offset2> ...") > + > + path = sys.argv[1] > + offsets = [] > + for off in sys.argv[2:]: > + offsets.append(int(off)) > + > + buf = gpiod.EdgeEventBuffer() > + > + with gpiod.request_lines( > + path, > + gpiod.RequestConfig(offsets=offsets, consumer="gpiomon.py"), > + gpiod.LineConfig(edge_detection=Edge.BOTH), > + ) as request: > + while True: > + request.read_edge_event(buf) > + for event in buf: > + print(event) Can you hide the buffer here to simplify the common case? How about having the request manage the buffer, and add a generator method that returns the next event, say edge_events()? For the uncommon case, add kwargs to allow the user to provide the buffer, or to specify the buffer size. If neither provided then the request constructs a default sized buffer. Then the code becomes: with gpiod.request_lines( path, offsets=offsets, consumer="gpiomon.py", edge_detection=Edge.BOTH, ) as request: for event in request.edge_events(): print(event) > diff --git a/bindings/python/examples/gpioset.py b/bindings/python/examples/gpioset.py > new file mode 100755 > index 0000000..3a8f8cc > --- /dev/null > +++ b/bindings/python/examples/gpioset.py > @@ -0,0 +1,35 @@ > +#!/usr/bin/env python3 > +# SPDX-License-Identifier: GPL-2.0-or-later > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > + > +"""Simplified reimplementation of the gpioset tool in Python.""" > + > +import gpiod > +import sys > + > +Value = gpiod.Line.Value > + > +if __name__ == "__main__": > + if len(sys.argv) < 3: > + raise TypeError("usage: gpioset.py <gpiochip> <offset1>=<value1> ...") > + > + path = sys.argv[1] > + values = dict() > + for arg in sys.argv[2:]: > + arg = arg.split("=") > + key = int(arg[0]) > + val = int(arg[1]) > + > + if val == 1: > + values[key] = Value.ACTIVE > + elif val == 0: > + values[key] = Value.INACTIVE > + else: > + raise ValueError("{} is an invalid value for GPIO lines".format(val)) > + > + with gpiod.request_lines( > + path, > + gpiod.RequestConfig(offsets=values.keys(), consumer="gpioset.py"), > + gpiod.LineConfig(direction=gpiod.Line.Direction.OUTPUT, output_values=values), > + ) as request: > + input() > -- > 2.34.1 > The focus of my comments above is to simplify the API for the most common case, and to make it a little more Pythonic rather than mirroring the C API, in both cases by hiding implementation details that the casual user doesn't need to know about. I'm pretty sure other minor things that I'm not 100% comfortable with are the same as with the C++ bindings, and the Python is in keeping with that, so I wont recover the same ground. I'm ok with the series overall. As per my C++ comments, it would be great to get some feedback from Python developers. Cheers, Kent.
On Fri, Jun 03, 2022 at 08:46:00PM +0800, Kent Gibson wrote: > On Wed, May 25, 2022 at 04:07:02PM +0200, Bartosz Golaszewski wrote: > > This adds the usual set of reimplementations of gpio-tools using the new > > python module. > > > > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> > > --- > > 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 | 31 +++++++++++++++++++++++ > > bindings/python/examples/gpioset.py | 35 ++++++++++++++++++++++++++ > > 7 files changed, 179 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 > > > > diff --git a/bindings/python/examples/Makefile.am b/bindings/python/examples/Makefile.am > > new file mode 100644 > > index 0000000..4169469 > > --- /dev/null > > +++ b/bindings/python/examples/Makefile.am > > @@ -0,0 +1,10 @@ > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +EXTRA_DIST = \ > > + gpiodetect.py \ > > + gpiofind.py \ > > + gpioget.py \ > > + gpioinfo.py \ > > + gpiomon.py \ > > + gpioset.py > > diff --git a/bindings/python/examples/gpiodetect.py b/bindings/python/examples/gpiodetect.py > > new file mode 100755 > > index 0000000..08e586b > > --- /dev/null > > +++ b/bindings/python/examples/gpiodetect.py > > @@ -0,0 +1,17 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +"""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) > > + ) > > diff --git a/bindings/python/examples/gpiofind.py b/bindings/python/examples/gpiofind.py > > new file mode 100755 > > index 0000000..e488a49 > > --- /dev/null > > +++ b/bindings/python/examples/gpiofind.py > > @@ -0,0 +1,20 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +"""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.get_line_offset_from_name(sys.argv[1]) > > + if offset is not None: > > + print("{} {}".format(chip.get_info().name, offset)) > > + sys.exit(0) > > + > > + sys.exit(1) > > diff --git a/bindings/python/examples/gpioget.py b/bindings/python/examples/gpioget.py > > new file mode 100755 > > index 0000000..c509f38 > > --- /dev/null > > +++ b/bindings/python/examples/gpioget.py > > @@ -0,0 +1,31 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +"""Simplified reimplementation of the gpioget tool in Python.""" > > + > > +import gpiod > > +import sys > > + > > +Direction = gpiod.Line.Direction > > +Value = gpiod.Line.Value > > + > > +if __name__ == "__main__": > > + if len(sys.argv) < 3: > > + raise TypeError("usage: gpioget.py <gpiochip> <offset1> <offset2> ...") > > + > > + path = sys.argv[1] > > + offsets = [] > > + for off in sys.argv[2:]: > > + offsets.append(int(off)) > > + > > + with gpiod.request_lines( > > + path, > > + gpiod.RequestConfig(offsets=offsets, consumer="gpioget.py"), > > + gpiod.LineConfig(direction=Direction.INPUT), > > + ) as request: > > With request_lines() being the primary entry point to the gpiod module, > consider extending it to allow the RequestConfig and LineConfig kwargs to > be passed directly to request_lines(), and for those transient objects to > be constructed within request_lines(). > That way the average user does not need to concern themselves with those > objects and the code is easier to read. > i.e. > with gpiod.request_lines( > path, > offsets=offsets, > consumer="gpioget.py", > direction=Direction.INPUT, > ) as request: > > You can still support passing in complete RequestConfig and LineConfig > as kwargs for cases where the user requires complex configuration. > i.e. > with gpiod.request_lines( > path, > req_cfg=gpiod.RequestConfig(offsets=offsets, consumer="gpioget.py"), > line_cfg=gpiod.LineConfig(direction=Direction.INPUT), > ) as request: > > Or for those use cases the user could use the Chip.request_lines() (which > wouldn't have the kwarg sugar) instead. > > Would be good to have some examples with complex configuration as well, > not just the tool reimplementations. > > > + vals = request.get_values() > > + > > + for val in vals: > > + print("0" if val == Value.INACTIVE else "1", end=" ") > > + print() > > diff --git a/bindings/python/examples/gpioinfo.py b/bindings/python/examples/gpioinfo.py > > new file mode 100755 > > index 0000000..3097d10 > > --- /dev/null > > +++ b/bindings/python/examples/gpioinfo.py > > @@ -0,0 +1,35 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +"""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", > > + ) > > + ) > > diff --git a/bindings/python/examples/gpiomon.py b/bindings/python/examples/gpiomon.py > > new file mode 100755 > > index 0000000..b0f4b88 > > --- /dev/null > > +++ b/bindings/python/examples/gpiomon.py > > @@ -0,0 +1,31 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +"""Simplified reimplementation of the gpiomon tool in Python.""" > > + > > +import gpiod > > +import sys > > + > > +Edge = gpiod.Line.Edge > > + > > +if __name__ == "__main__": > > + if len(sys.argv) < 3: > > + raise TypeError("usage: gpiomon.py <gpiochip> <offset1> <offset2> ...") > > + > > + path = sys.argv[1] > > + offsets = [] > > + for off in sys.argv[2:]: > > + offsets.append(int(off)) > > + > > + buf = gpiod.EdgeEventBuffer() > > + > > + with gpiod.request_lines( > > + path, > > + gpiod.RequestConfig(offsets=offsets, consumer="gpiomon.py"), > > + gpiod.LineConfig(edge_detection=Edge.BOTH), > > + ) as request: > > + while True: > > + request.read_edge_event(buf) > > + for event in buf: > > + print(event) > > Can you hide the buffer here to simplify the common case? > How about having the request manage the buffer, and add a generator > method that returns the next event, say edge_events()? > > For the uncommon case, add kwargs to allow the user to provide the buffer, > or to specify the buffer size. If neither provided then the request > constructs a default sized buffer. > > Then the code becomes: > > with gpiod.request_lines( > path, > offsets=offsets, > consumer="gpiomon.py", > edge_detection=Edge.BOTH, > ) as request: > for event in request.edge_events(): > print(event) > > > diff --git a/bindings/python/examples/gpioset.py b/bindings/python/examples/gpioset.py > > new file mode 100755 > > index 0000000..3a8f8cc > > --- /dev/null > > +++ b/bindings/python/examples/gpioset.py > > @@ -0,0 +1,35 @@ > > +#!/usr/bin/env python3 > > +# SPDX-License-Identifier: GPL-2.0-or-later > > +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> > > + > > +"""Simplified reimplementation of the gpioset tool in Python.""" > > + > > +import gpiod > > +import sys > > + > > +Value = gpiod.Line.Value > > + > > +if __name__ == "__main__": > > + if len(sys.argv) < 3: > > + raise TypeError("usage: gpioset.py <gpiochip> <offset1>=<value1> ...") > > + > > + path = sys.argv[1] > > + values = dict() > > + for arg in sys.argv[2:]: > > + arg = arg.split("=") > > + key = int(arg[0]) > > + val = int(arg[1]) > > + > > + if val == 1: > > + values[key] = Value.ACTIVE > > + elif val == 0: > > + values[key] = Value.INACTIVE > > + else: > > + raise ValueError("{} is an invalid value for GPIO lines".format(val)) > > + > > + with gpiod.request_lines( > > + path, > > + gpiod.RequestConfig(offsets=values.keys(), consumer="gpioset.py"), > > + gpiod.LineConfig(direction=gpiod.Line.Direction.OUTPUT, output_values=values), > > + ) as request: > > + input() > > -- > > 2.34.1 > > > > The focus of my comments above is to simplify the API for the most common > case, and to make it a little more Pythonic rather than mirroring the C > API, in both cases by hiding implementation details that the casual user > doesn't need to know about. > Further to this, and recalling our discussions on tool changes, it would be great if the Python API supported identification of line by name, not just (chip,offset). e.g. with gpiod.request_lines( lines=("GPIO17", "GPIO18"), edge_detection=Edge.BOTH, ) as request: for event in request.edge_events(): print(event) with the returned event extended to contain the line name if the line was identified by name in request_lines(). The lines kwarg replaces offsets, and could contain names (strings) or offsets (integers), or a combination. If any offsets are present then the chip path kwarg must also be provided. If the chip isn't provided, request_lines() would find the corresponding chip based on the line name. Cheers, Kent.
On Sat, Jun 04, 2022 at 10:41:31AM +0800, Kent Gibson wrote: > On Fri, Jun 03, 2022 at 08:46:00PM +0800, Kent Gibson wrote: > > On Wed, May 25, 2022 at 04:07:02PM +0200, Bartosz Golaszewski wrote: ... > > The focus of my comments above is to simplify the API for the most common > > case, and to make it a little more Pythonic rather than mirroring the C > > API, in both cases by hiding implementation details that the casual user > > doesn't need to know about. > > > > Further to this, and recalling our discussions on tool changes, it would > be great if the Python API supported identification of line by name, not > just (chip,offset). > > e.g. > with gpiod.request_lines( > lines=("GPIO17", "GPIO18"), > edge_detection=Edge.BOTH, > ) as request: > for event in request.edge_events(): > print(event) > > with the returned event extended to contain the line name if the line > was identified by name in request_lines(). > > The lines kwarg replaces offsets, and could contain names (strings) or > offsets (integers), or a combination. If any offsets are present then > the chip path kwarg must also be provided. If the chip isn't provided, > request_lines() would find the corresponding chip based on the line name.
On Mon, Jun 06, 2022 at 01:14:48PM +0300, Andy Shevchenko wrote: > On Sat, Jun 04, 2022 at 10:41:31AM +0800, Kent Gibson wrote: > > On Fri, Jun 03, 2022 at 08:46:00PM +0800, Kent Gibson wrote: > > > On Wed, May 25, 2022 at 04:07:02PM +0200, Bartosz Golaszewski wrote: > > ... > > > > The focus of my comments above is to simplify the API for the most common > > > case, and to make it a little more Pythonic rather than mirroring the C > > > API, in both cases by hiding implementation details that the casual user > > > doesn't need to know about. > > > > > > > Further to this, and recalling our discussions on tool changes, it would > > be great if the Python API supported identification of line by name, not > > just (chip,offset). > > > > e.g. > > with gpiod.request_lines( > > lines=("GPIO17", "GPIO18"), > > edge_detection=Edge.BOTH, > > ) as request: > > for event in request.edge_events(): > > print(event) > > > > with the returned event extended to contain the line name if the line > > was identified by name in request_lines(). > > > > The lines kwarg replaces offsets, and could contain names (strings) or > > offsets (integers), or a combination. If any offsets are present then > > the chip path kwarg must also be provided. If the chip isn't provided, > > request_lines() would find the corresponding chip based on the line name. > > From Python programmer perspective it's a good idea, but from GPIO (ABI) > perspective, it may be confusing. Line name is not unique (globally) and > basically not a part of ABI. > "basically not a part of the ABI"??? Damn - we should've removed it from the line info for uAPI v2 ;-). A common request from users is to be able to request lines by name. Of the libgpiod bindings, Python is the best suited to allow that possibility directly as part of its core API. It also happens to be the one most likely to be used by said users. While identifying line by name can't be guaranteed to work universally, that doesn't mean that we should automatically exclude the possibility. It is possible with the current ABI - it is awkward, but possible. In libgpiod v1, gpiod_ctxless_find_line(), gpiod_chip_find_line() et al., and in v2 gpiod_chip_get_line_offset_from_name(), do just that - I'm merely suggesting that similar functionality be incorporated into request_lines(). Line names should be unique in well configured systems, even if the kernel itself does not guarantee it. The binding would perform an exhaustive search to ensure the requested line name is unique, and throw if not (unlike the libgpiod v1 functions that return the first match - yikes). (We could always extend the GPIO uAPI to make the mapping process less painful, e.g. an ioctl to perform the name to offset mapping, including uniqueness check, for a chip.) For applications targetting systems that don't guarantee uniqueness, the (chip,offset) approach remains available. And if the line names are thought to be unique within a chip, the middle ground of (chip,name) is also available. Wrt confusion, the alternative would be to provide a separate name based API wrapper, or insist that the user jump through the name mapping hoops themselves prior to calling the offset based API. Are either of those less confusing? But if the purpose of the Python binding is purely to minimally wrap the C ABI, warts and all, then my suggestion should most certainly be ignored. Cheers, Kent.
On Tue, Jun 7, 2022 at 5:11 AM Kent Gibson <warthog618@gmail.com> wrote: > On Mon, Jun 06, 2022 at 01:14:48PM +0300, Andy Shevchenko wrote: > > On Sat, Jun 04, 2022 at 10:41:31AM +0800, Kent Gibson wrote: ... > > From Python programmer perspective it's a good idea, but from GPIO (ABI) > > perspective, it may be confusing. Line name is not unique (globally) and > > basically not a part of ABI. > > "basically not a part of the ABI"??? > Damn - we should've removed it from the line info for uAPI v2 ;-). Yep, names are just aliases for the line numbers, but _not_ unique in the system. You may have many lines with the same name alias, but number. I remember that we made it stricter in the kernel, but as far as I understand there is nothing prevents some drivers to behave badly. https://elixir.bootlin.com/linux/v5.19-rc1/source/drivers/gpio/gpiolib.c#L331 Also note, we don't validate names given by properties. Any DT may contain same names for several lines on the same chip. ... > Line names should be unique in well configured systems, even if the > kernel itself does not guarantee it. It's not about kernel, but like you said "well configured systems". If we push the strict rules, we might be punished by users who want actually to have ambiguous names. ... > But if the purpose of the Python binding is purely to minimally wrap the > C ABI, warts and all, then my suggestion should most certainly be ignored. As i said, it's very good suggestion, but pity we don't strict line naming in the kernel.
On Thu, Jun 09, 2022 at 06:06:04PM +0200, Bartosz Golaszewski wrote: > On Thu, Jun 9, 2022 at 3:21 PM Jiri Benc <jbenc@upir.cz> wrote: > > > > On Thu, 9 Jun 2022 10:42:44 +0200, Bartosz Golaszewski wrote: > > > On Thu, Jun 9, 2022 at 6:49 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > Agree that it would be easier to write a pythonic wrapper around the C > > > > API in Python, so no problem with that. > > > > However, the pythonic wrapper should the one named gpiod, as it is > > > > intended to be the primary interface for Python. Rename your existing > > > > to gpiod_c or gpiod_core or something. > > > > > > I don't agree. The module that wraps the C library should still be > > > called gpiod and be the primary interface. The pythonic module would > > > just offer helpers that would still use the gpiod data types for most > > > part. > > > > As a Python user, I'd much rather see the high level API being the > > primary interface and being named 'gpiod'. The easier to use and more > > Pythonic, the better. The low level library bindings and low level data > > types are just an implementation detail for me when coding in Python. > > If I wanted low level, I'd code everything directly in C. > > > > But Kent is not talking about a whole new "pythonic" layer on top of > the code that is the subject of this series. The bindings are already > quite pythonic in that you can get most stuff done with a "logical" > one-liner. The gpiod module doesn't map C API 1:1, it already > simplifies a bunch of interfaces. Kent's idea IIUC is about providing > a set of helpers that would produce the gpiod objects in shorter code > by hiding the details of intermediate objects being created. > Yeah, no, I'm saying that there should be one primary Python interface to gpiod, and it should be the most pythonic. And complete. The casual user should be able to get by with a few simple commands, but the complete functionality should still be accessible, via that same API, for more complicated cases. > Re the event buffer: yeah, I think in python (unlike C++ or future > Rust bindings) it makes sense to hide it within the request as we > can't profit from implicitly not copying the event objects. > That is a consequence of building on top of the gpiod C API, as you have to deal with two object models, and the related type conversions or lifecycle management. A native binding built on top of the ioctls can use native objects throughout can take better advantage of the language. e.g. here the equivalent of my Python suggestion using my Rust gpiocdev library (sadly without named lines support as I hadn't considered that at the time): // request the lines let req = Request::builder() .on_chip("/dev/gpiochip0") .with_lines(&[17,18]) .with_edge_detection(EdgeDetection::BothEdges) .request()?; // wait for line edge events for event in req.edge_events()? { println!("{:?}", event?); } That has a hidden event buffer within the request. The default size is 1, but add .with_user_event_buffer_size(N) and you get an N event buffer. There is no copying involved, just borrowing, as all the objects are visible to the borrow checker. > If anyone wants to create an even simpler, complete interface for > gpiod, then it's a task for a whole new project. Think: pydbus built > on top of GLib dbus bindings in python, built on top of glib's dbus > implementation. > Don't tempt me - though I would target the GPIO uAPI ioctls, not gpiod, for the reasons above. Cheers, Kent.
On Fri, Jun 10, 2022 at 6:23 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Thu, Jun 09, 2022 at 06:06:04PM +0200, Bartosz Golaszewski wrote: > > On Thu, Jun 9, 2022 at 3:21 PM Jiri Benc <jbenc@upir.cz> wrote: > > > > > > On Thu, 9 Jun 2022 10:42:44 +0200, Bartosz Golaszewski wrote: > > > > On Thu, Jun 9, 2022 at 6:49 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > Agree that it would be easier to write a pythonic wrapper around the C > > > > > API in Python, so no problem with that. > > > > > However, the pythonic wrapper should the one named gpiod, as it is > > > > > intended to be the primary interface for Python. Rename your existing > > > > > to gpiod_c or gpiod_core or something. > > > > > > > > I don't agree. The module that wraps the C library should still be > > > > called gpiod and be the primary interface. The pythonic module would > > > > just offer helpers that would still use the gpiod data types for most > > > > part. > > > > > > As a Python user, I'd much rather see the high level API being the > > > primary interface and being named 'gpiod'. The easier to use and more > > > Pythonic, the better. The low level library bindings and low level data > > > types are just an implementation detail for me when coding in Python. > > > If I wanted low level, I'd code everything directly in C. > > > > > > > But Kent is not talking about a whole new "pythonic" layer on top of > > the code that is the subject of this series. The bindings are already > > quite pythonic in that you can get most stuff done with a "logical" > > one-liner. The gpiod module doesn't map C API 1:1, it already > > simplifies a bunch of interfaces. Kent's idea IIUC is about providing > > a set of helpers that would produce the gpiod objects in shorter code > > by hiding the details of intermediate objects being created. > > > > Yeah, no, I'm saying that there should be one primary Python interface > to gpiod, and it should be the most pythonic. And complete. > The casual user should be able to get by with a few simple commands, but > the complete functionality should still be accessible, via that same > API, for more complicated cases. > > > Re the event buffer: yeah, I think in python (unlike C++ or future > > Rust bindings) it makes sense to hide it within the request as we > > can't profit from implicitly not copying the event objects. > > > > That is a consequence of building on top of the gpiod C API, as you have > to deal with two object models, and the related type conversions or > lifecycle management. > A native binding built on top of the ioctls can use native objects > throughout can take better advantage of the language. > e.g. here the equivalent of my Python suggestion using my Rust > gpiocdev library (sadly without named lines support as I hadn't > considered that at the time): > > // request the lines > let req = Request::builder() > .on_chip("/dev/gpiochip0") > .with_lines(&[17,18]) > .with_edge_detection(EdgeDetection::BothEdges) > .request()?; > So in Python you'd like to see an equivalent in the form of: req = gpiod.request_lines(chip="/dev/gpiochip0", lines=["GPIO17", "RESET_0"], edge_detection=Edge.BOTH) > // wait for line edge events > for event in req.edge_events()? { > println!("{:?}", event?); > } and: for event in req.read_edge_events(): print(event) Note that for the event reading: implementing yield in C API is not currently possible. I think the best way to implement the above example is to simply return a buffer filled with events that is already iterable. Ok I can work with that. It also makes sense to take keyword arguments by default in all methods. Thanks Bart > > That has a hidden event buffer within the request. The default size is 1, > but add .with_user_event_buffer_size(N) and you get an N event buffer. > There is no copying involved, just borrowing, as all the objects are > visible to the borrow checker. > > > If anyone wants to create an even simpler, complete interface for > > gpiod, then it's a task for a whole new project. Think: pydbus built > > on top of GLib dbus bindings in python, built on top of glib's dbus > > implementation. > > > > Don't tempt me - though I would target the GPIO uAPI ioctls, not gpiod, > for the reasons above. > > Cheers, > Kent.
diff --git a/bindings/python/examples/Makefile.am b/bindings/python/examples/Makefile.am new file mode 100644 index 0000000..4169469 --- /dev/null +++ b/bindings/python/examples/Makefile.am @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +EXTRA_DIST = \ + gpiodetect.py \ + gpiofind.py \ + gpioget.py \ + gpioinfo.py \ + gpiomon.py \ + gpioset.py diff --git a/bindings/python/examples/gpiodetect.py b/bindings/python/examples/gpiodetect.py new file mode 100755 index 0000000..08e586b --- /dev/null +++ b/bindings/python/examples/gpiodetect.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +"""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) + ) diff --git a/bindings/python/examples/gpiofind.py b/bindings/python/examples/gpiofind.py new file mode 100755 index 0000000..e488a49 --- /dev/null +++ b/bindings/python/examples/gpiofind.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +"""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.get_line_offset_from_name(sys.argv[1]) + if offset is not None: + print("{} {}".format(chip.get_info().name, offset)) + sys.exit(0) + + sys.exit(1) diff --git a/bindings/python/examples/gpioget.py b/bindings/python/examples/gpioget.py new file mode 100755 index 0000000..c509f38 --- /dev/null +++ b/bindings/python/examples/gpioget.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +"""Simplified reimplementation of the gpioget tool in Python.""" + +import gpiod +import sys + +Direction = gpiod.Line.Direction +Value = gpiod.Line.Value + +if __name__ == "__main__": + if len(sys.argv) < 3: + raise TypeError("usage: gpioget.py <gpiochip> <offset1> <offset2> ...") + + path = sys.argv[1] + offsets = [] + for off in sys.argv[2:]: + offsets.append(int(off)) + + with gpiod.request_lines( + path, + gpiod.RequestConfig(offsets=offsets, consumer="gpioget.py"), + gpiod.LineConfig(direction=Direction.INPUT), + ) as request: + vals = request.get_values() + + for val in vals: + print("0" if val == Value.INACTIVE else "1", end=" ") + print() diff --git a/bindings/python/examples/gpioinfo.py b/bindings/python/examples/gpioinfo.py new file mode 100755 index 0000000..3097d10 --- /dev/null +++ b/bindings/python/examples/gpioinfo.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +"""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", + ) + ) diff --git a/bindings/python/examples/gpiomon.py b/bindings/python/examples/gpiomon.py new file mode 100755 index 0000000..b0f4b88 --- /dev/null +++ b/bindings/python/examples/gpiomon.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +"""Simplified reimplementation of the gpiomon tool in Python.""" + +import gpiod +import sys + +Edge = gpiod.Line.Edge + +if __name__ == "__main__": + if len(sys.argv) < 3: + raise TypeError("usage: gpiomon.py <gpiochip> <offset1> <offset2> ...") + + path = sys.argv[1] + offsets = [] + for off in sys.argv[2:]: + offsets.append(int(off)) + + buf = gpiod.EdgeEventBuffer() + + with gpiod.request_lines( + path, + gpiod.RequestConfig(offsets=offsets, consumer="gpiomon.py"), + gpiod.LineConfig(edge_detection=Edge.BOTH), + ) as request: + while True: + request.read_edge_event(buf) + for event in buf: + print(event) diff --git a/bindings/python/examples/gpioset.py b/bindings/python/examples/gpioset.py new file mode 100755 index 0000000..3a8f8cc --- /dev/null +++ b/bindings/python/examples/gpioset.py @@ -0,0 +1,35 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# SPDX-FileCopyrightText: 2017-2021 Bartosz Golaszewski <bartekgola@gmail.com> + +"""Simplified reimplementation of the gpioset tool in Python.""" + +import gpiod +import sys + +Value = gpiod.Line.Value + +if __name__ == "__main__": + if len(sys.argv) < 3: + raise TypeError("usage: gpioset.py <gpiochip> <offset1>=<value1> ...") + + path = sys.argv[1] + values = dict() + for arg in sys.argv[2:]: + arg = arg.split("=") + key = int(arg[0]) + val = int(arg[1]) + + if val == 1: + values[key] = Value.ACTIVE + elif val == 0: + values[key] = Value.INACTIVE + else: + raise ValueError("{} is an invalid value for GPIO lines".format(val)) + + with gpiod.request_lines( + path, + gpiod.RequestConfig(offsets=values.keys(), consumer="gpioset.py"), + gpiod.LineConfig(direction=gpiod.Line.Direction.OUTPUT, output_values=values), + ) as request: + input()
This adds the usual set of reimplementations of gpio-tools using the new python module. Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl> --- 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 | 31 +++++++++++++++++++++++ bindings/python/examples/gpioset.py | 35 ++++++++++++++++++++++++++ 7 files changed, 179 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