fix potential glitches in bindings example gpiosets
cover fix potential glitches in bindings example gpiosets
A couple of minor fixes to the bindings examples. Noticed the python one in passing, and fixed that, then checked the other bindings to see if they had the same problem.
The rust example is ok, but the cxx example has the same issue, so fixed that as well.
The checkin comments are virtually identical, as they fix the same thing for each of the bindings, but the alternative of combining them in one patch seemed weird.
Cheers,
Kent.
Kent Gibson (2):
bindings: python: examples: fix potential glitch in gpioset.py
bindings: cxx: examples: fix potential glitch in gpiosetcxx
bindings/cxx/examples/gpiosetcxx.cpp | 25 ++++++++++--------------- bindings/python/examples/gpioset.py | 10 +++++----- 2 files changed, 15 insertions(+), 20 deletions(-)
Signature · 1 line
2.40.1
PATCH 1/2 bindings: python: examples: fix potential glitch in gpioset.py
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.
--- bindings/python/examples/gpioset.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
bindings/python/examples/gpioset.py +5 -5
@@ -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()
Signature · 1 line
2.40.1
Re: [libgpiod][PATCH 1/2] bindings: python: examples: fix potential glitch in gpioset.py
Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti:
Quoted 10 lines
gpioset.pyrequests 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?
...
Quoted 1 line
+ config =
dict([(l, settings(v)) for (l, v) in lvs])
Aren't [] not needed?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [libgpiod][PATCH 1/2] bindings: python: examples: fix potential glitch in gpioset.py
Quoted 13 lines · On Fri, Jun 9, 2023 at 10:19 PM <[email protected]> wrote:
Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti:
Quoted 10 lines
gpioset.pyrequests 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.
Quoted 8 lines
Do we need a comment in the code to specify this?
...
Quoted 1 line
+ 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:
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
config =
dict([(1, 2), (3, 4)])
config
{1: 2, 3: 4}
Bart
Quoted 5 lines
--
With Best Regards,
Andy Shevchenko
Re: [libgpiod][PATCH 1/2] bindings: python: examples: fix potential glitch in gpioset.py
Quoted 2 lines · Bartosz Golaszewski
Quoted 1 line · On Fri, Jun 9, 2023 at 10:19 PM <[email protected]> wrote:
Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti:
...
Quoted 9 lines
Quoted 3 lines
Quoted 1 line
+ 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:
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
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 :-)
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [libgpiod][PATCH 1/2] bindings: python: examples: fix potential glitch in gpioset.py
In reply to Bartosz GolaszewskiQuoted 8 lines · Bartosz Golaszewski
Quoted 7 lines · On Fri, Jun 9, 2023 at 10:19 PM <[email protected]> wrote:
Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti:
Quoted 5 lines
gpioset.pyrequests 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.
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
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.
Quoted 6 lines
Quoted 6 lines
...
Quoted 1 line
+ config =
dict([(l, settings(v)) for (l, v) in lvs])Aren't [] not needed?
Ok, but now I did get this one:
Quoted 7 lines
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.
Re: [libgpiod][PATCH 1/2] bindings: python: examples: fix potential glitch in gpioset.py
Quoted 43 lines · Kent Gibson
Quoted 8 lines · Bartosz Golaszewski
Quoted 7 lines · On Fri, Jun 9, 2023 at 10:19 PM <[email protected]> wrote:
Fri, Jun 09, 2023 at 11:36:06PM +0800, Kent Gibson kirjoitti:
Quoted 5 lines
gpioset.pyrequests 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.Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
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.
Quoted 6 lines
Quoted 6 lines
...
Quoted 1 line
+ config =
dict([(l, settings(v)) for (l, v) in lvs])Aren't [] not needed?
Ok, but now I did get this one:
Quoted 7 lines
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 😄
Quoted 7 lines
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
Re: [libgpiod][PATCH 1/2] bindings: python: examples: fix potential glitch in gpioset.py
In reply to Kent GibsonQuoted 22 lines · Kent Gibson
gpioset.pyrequests 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 <[email protected]>--- bindings/python/examples/gpioset.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)bindings/python/examples/gpioset.py +0
-0
bindings/python/examples/gpioset.py if __name__ == "__main__": · line 24
Quoted 4 lines
bindings/python/examples/gpioset.py +1
-0@@ -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
Quoted 20 lines
bindings/python/examples/gpioset.py +4
-6+ 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
PATCH 2/2 bindings: cxx: examples: fix potential glitch in gpiosetcxx
In reply to Kent Gibsongpiosetcxx 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.
--- bindings/cxx/examples/gpiosetcxx.cpp | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)
bindings/cxx/examples/gpiosetcxx.cpp +10 -15
@@ -16,8 +16,8 @@ int main(int argc, char **argv) return EXIT_FAILURE; } - ::gpiod::line::offsets offsets; - ::gpiod::line::values values; + auto builder = ::gpiod::chip(argv[1]).prepare_request(); + builder.set_consumer("gpiosetcxx"); for (int i = 2; i < argc; i++) { ::std::string arg(argv[i]);
@@ -31,22 +31,17 @@ int main(int argc, char **argv) throw ::std::invalid_argument("invalid offset=value mapping: " + ::std::string(argv[i])); - offsets.push_back(::std::stoul(offset)); - values.push_back(::std::stoul(value) ? ::gpiod::line::value::ACTIVE : - ::gpiod::line::value::INACTIVE); - } - - auto request = ::gpiod::chip(argv[1]) - .prepare_request() - .set_consumer("gpiosetcxx") - .add_line_settings( - offsets, + ::gpiod::line::value v = ::std::stoul(value) ? + ::gpiod::line::value::ACTIVE : + ::gpiod::line::value::INACTIVE; + builder.add_line_settings( + ::std::stoul(offset), ::gpiod::line_settings() .set_direction(::gpiod::line::direction::OUTPUT) - ) - .do_request(); + .set_output_value(v)); + } - request.set_values(values); + auto request = builder.do_request(); ::std::cin.get();
Signature · 1 line
2.40.1
Re: [libgpiod][PATCH 2/2] bindings: cxx: examples: fix potential glitch in gpiosetcxx
Fri, Jun 09, 2023 at 11:36:07PM +0800, Kent Gibson kirjoitti:
Quoted 10 lines
gpiosetcxx 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.
Same question about commenting in the code.
Signature · 2 lines
With Best Regards, Andy Shevchenko