| Message ID | 20230609153607.133379-2-warthog618@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix potential glitches in bindings example gpiosets | expand |
Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti: > gpioset.py requests lines without setting their output value, and so > sets them all inactive, and subsequently sets them to their requested > value. This can result in glitches on lines which were active and > are set active. > > As this is example code, it is also important to demonstrate that the > output value can be set by the request itself. > > Request the lines with the correct output values set in the request > itself. Do we need a comment in the code to specify this? ... > + config = dict([(l, settings(v)) for (l, v) in lvs]) Aren't [] not needed?
On Fri, Jun 9, 2023 at 10:19 PM <andy.shevchenko@gmail.com> wrote: > > Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti: > > gpioset.py requests lines without setting their output value, and so > > sets them all inactive, and subsequently sets them to their requested > > value. This can result in glitches on lines which were active and > > are set active. > > > > As this is example code, it is also important to demonstrate that the > > output value can be set by the request itself. > > > > Request the lines with the correct output values set in the request > > itself. > > Do we need a comment in the code to specify this? > > ... > > > + config = dict([(l, settings(v)) for (l, v) in lvs]) > > Aren't [] not needed? > This is a list comprehension used to create the dictionary. Think: >>> config = dict([(1, 2), (3, 4)]) >>> config {1: 2, 3: 4} Bart > -- > With Best Regards, > Andy Shevchenko > >
On Fri, Jun 9, 2023 at 5:36 PM Kent Gibson <warthog618@gmail.com> wrote: > > gpioset.py requests lines without setting their output value, and so > sets them all inactive, and subsequently sets them to their requested > value. This can result in glitches on lines which were active and > are set active. > > As this is example code, it is also important to demonstrate that the > output value can be set by the request itself. > > Request the lines with the correct output values set in the request > itself. > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > --- > bindings/python/examples/gpioset.py | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/bindings/python/examples/gpioset.py b/bindings/python/examples/gpioset.py > index 372a9a8..b36b376 100755 > --- a/bindings/python/examples/gpioset.py > +++ b/bindings/python/examples/gpioset.py > @@ -21,16 +21,16 @@ if __name__ == "__main__": > x, y = arg.split("=") > return (x, Value(int(y))) > > + def settings(v): I renamed this to make_settings() when applying. Thanks. Bart > + return gpiod.LineSettings(direction=Direction.OUTPUT, output_value=v) > + > lvs = [parse_value(arg) for arg in sys.argv[2:]] > - lines = [x[0] for x in lvs] > - values = dict(lvs) > + config = dict([(l, settings(v)) for (l, v) in lvs]) > > request = gpiod.request_lines( > path, > consumer="gpioset.py", > - config={tuple(lines): gpiod.LineSettings(direction=Direction.OUTPUT)}, > + config=config, > ) > > - vals = request.set_values(values) > - > input() > -- > 2.40.1 >
On Mon, Jun 12, 2023 at 5:26 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > On Fri, Jun 9, 2023 at 10:19 PM <andy.shevchenko@gmail.com> wrote: > > Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti: ... > > > + config = dict([(l, settings(v)) for (l, v) in lvs]) > > > > Aren't [] not needed? > > This is a list comprehension used to create the dictionary. Think: > > >>> config = dict([(1, 2), (3, 4)]) > >>> config > {1: 2, 3: 4} Think about it in dynamic: In [1]: x= [(1,2),(2,4)] In [2]: dict((a,b)for a,b in x) Out[2]: {1: 2, 2: 4} [] are redundant, so I remembered that correctly :-)
On Mon, Jun 12, 2023 at 04:26:46PM +0200, Bartosz Golaszewski wrote: > On Fri, Jun 9, 2023 at 10:19 PM <andy.shevchenko@gmail.com> wrote: > > > > Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti: > > > gpioset.py requests lines without setting their output value, and so > > > sets them all inactive, and subsequently sets them to their requested > > > value. This can result in glitches on lines which were active and > > > are set active. > > > > > > As this is example code, it is also important to demonstrate that the > > > output value can be set by the request itself. > > > > > > Request the lines with the correct output values set in the request > > > itself. > > > > Do we need a comment in the code to specify this? > > Andy, I'm not ignoring you - I'm still not getting mail from you, and I hadn't looked on the list for replies. Weird. In answer to your point - yes and no. The code is not doing anything unusual, so no. OTOH it does serve as example code, so a bit of commentary wouldn't hurt. > > ... > > > > > + config = dict([(l, settings(v)) for (l, v) in lvs]) > > > > Aren't [] not needed? > > Ok, but now I did get this one: > Think about it in dynamic: > In [1]: x= [(1,2),(2,4)] > In [2]: dict((a,b)for a,b in x) > Out[2]: {1: 2, 2: 4} > [] are redundant, so I remembered that correctly 😄 Terrible example - which 2 is which? 1,2,3,4 would've been better. True - dict() accepts an iterable, so the [] are redundant in thise case. Cheers, Kent.
On Mon, Jun 12, 2023 at 4:54 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Mon, Jun 12, 2023 at 04:26:46PM +0200, Bartosz Golaszewski wrote: > > On Fri, Jun 9, 2023 at 10:19 PM <andy.shevchenko@gmail.com> wrote: > > > > > > Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti: > > > > gpioset.py requests lines without setting their output value, and so > > > > sets them all inactive, and subsequently sets them to their requested > > > > value. This can result in glitches on lines which were active and > > > > are set active. > > > > > > > > As this is example code, it is also important to demonstrate that the > > > > output value can be set by the request itself. > > > > > > > > Request the lines with the correct output values set in the request > > > > itself. > > > > > > Do we need a comment in the code to specify this? > > > > > Andy, I'm not ignoring you - I'm still not getting mail from you, and I > hadn't looked on the list for replies. Weird. > > In answer to your point - yes and no. The code is not doing anything > unusual, so no. OTOH it does serve as example code, so a bit of > commentary wouldn't hurt. > > > > ... > > > > > > > + config = dict([(l, settings(v)) for (l, v) in lvs]) > > > > > > Aren't [] not needed? > > > > > Ok, but now I did get this one: > > > Think about it in dynamic: > > > In [1]: x= [(1,2),(2,4)] > > In [2]: dict((a,b)for a,b in x) > > Out[2]: {1: 2, 2: 4} > > > [] are redundant, so I remembered that correctly 😄 > > Terrible example - which 2 is which? > 1,2,3,4 would've been better. > > True - dict() accepts an iterable, so the [] are redundant in thise case. > > Cheers, > Kent. Ok, I will add a follow-up commit. Bart
diff --git a/bindings/python/examples/gpioset.py b/bindings/python/examples/gpioset.py index 372a9a8..b36b376 100755 --- a/bindings/python/examples/gpioset.py +++ b/bindings/python/examples/gpioset.py @@ -21,16 +21,16 @@ if __name__ == "__main__": x, y = arg.split("=") return (x, Value(int(y))) + def settings(v): + return gpiod.LineSettings(direction=Direction.OUTPUT, output_value=v) + lvs = [parse_value(arg) for arg in sys.argv[2:]] - lines = [x[0] for x in lvs] - values = dict(lvs) + config = dict([(l, settings(v)) for (l, v) in lvs]) request = gpiod.request_lines( path, consumer="gpioset.py", - config={tuple(lines): gpiod.LineSettings(direction=Direction.OUTPUT)}, + config=config, ) - vals = request.set_values(values) - input()
gpioset.py requests lines without setting their output value, and so sets them all inactive, and subsequently sets them to their requested value. This can result in glitches on lines which were active and are set active. As this is example code, it is also important to demonstrate that the output value can be set by the request itself. Request the lines with the correct output values set in the request itself. Signed-off-by: Kent Gibson <warthog618@gmail.com> --- bindings/python/examples/gpioset.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)