| Message ID | 20221114040102.66031-1-warthog618@gmail.com |
|---|---|
| Headers | show |
| Series | tools: improvements for v2 | expand |
On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > This patch series is an optimistic reimagining of the tools intended to > simplify usage for well configured systems, i.e. for systems where lines > can be uniquely identified by name. In such systems the chip and offset > location of the line is no longer of relevance to the user, so the tools > should be able to operate without mentioning them. > e.g. > gpioget GPIO17 > > gpioset GPIO17=active > > gpiomon --localtime GPIO17 GPIO18 > > It is accepted that the kernel does not guarantee line name uniqueness > within the system, or even within a chip, and not all systems are well > configured, so the tools retain the option to identify lines by chip > and offset. The hope and expectation is that over time systems will > become more well configured, not less, and identification of GPIO lines > by name will become the norm. > > The core of the series is patch 1 which is a reworking of the tools to > support identifying lines by name, and to operate across multiple GPIO > chips if named lines are located on different chips. > The gpioset tool is extended to support toggling lines and interactive > control of line values, so some common use cases can be trivially > implemented from the command line. > e.g. > gpioset --toggle 500ms LED=on > > will blink the LED line at 1Hz, indefinitely. > More complex outputs can be generated by adding more entries to the > toggle sequence: > gpioset --toggle 1s,2s,1s,300ms LED=on > > Even more complex outputs can be generated by driving gpioset in > interactive mode from another script. > > Those are the major changes. A more complete list of the changes can be > found in the patch description. > > The core tool changes are contained in patch 2. To simplify review, > patch 1 removes old code replaced by that in patch 2 and 3. > Patch 1 also removes gpiofind, as that tools functionality is absorbed > by the other commands, particularly gpioinfo. > > Patch 3 updates and extends the tool tests to cover the reworked tools, > including demonstrating gpioset being driven interactively via a script. > > Patch 4 adds a gpionotify tool that monitors changes to the state line > information, similar to the gpio-watch tool in the kernel, and > patch 5 extends the test suite to cover it. > > Cheers, > Kent. > > Changes v3 -> v4: > - rebase on master following merge with next/libgpiod-2.0 > - C style comments - again. > - rename gpiowatch to gpionotify > - make gpioset interactive mode optional, enabled with > --enable-gpioset-interactive. > - gpioset does not exit by default > - add a banner option to gpioset as it can be long lived > - make some functions and variables static > - move parse_periods_or_die from tools-common to gpioset > - quote line and consumer names > - add option to not quote line and consumer names Eh... yeah, whatever. > - always use bool, not int, for command line flags > - add --consumer option to commands that request lines (get/set/mon) > - move parse_periods_or_die() from tools-common to gpioset > > Changes v2 -> v3: > - squash removal of gpiofind into patch 1 (was patch 6). > - rebase to C API line_config changes. > - rework line name to chip/offset resolution to improve clarity and > better handle corner cases. > - drop bias=as-is as a command line option as that is the default > behaviour. > - revise gpioinfo output format to combine the used flag and consumer > name, and to remove the brackets around the list of attributes. > - gpiowatch: rework so it is more like gpiomon than the Linux gpio-watch > tool. More details in patch 4. > - quote text from the command line when used in error messages. > - improve test suite coverage of corner cases. > - gpiomon: rename --edge option to --edges, and drop "-edges" from the > possible values, e.g. --edges=rising. > - add hte support to gpiomon. > - gpiomon: decouple selection of event clock from timestamp output > formatting. > > Changes v1 -> v2: > - code formatting, particularly trying to keep to the 80 character > limit and C style comments. > - move global config fields into the struct config for each tool. > - switch gpioset from readline to libedit. > - add tests for symlink chip path behaviour. > - long lived tools flush stdout before blocking. > - fix copyrights > - replace gpiosim attr lookup functions with cached values. > - remove gpiofind > > Kent Gibson (5): > tools: remove old code to simplify review > tools: line name focussed rework > tools: tests for line name focussed rework > tools: add gpionotify > tools: gpionotify tests > > configure.ac | 24 +- > man/Makefile.am | 2 +- > tools/.gitignore | 2 +- > tools/Makefile.am | 12 +- > tools/gpio-tools-test | 3 - > tools/gpio-tools-test.bats | 3079 ++++++++++++++++++++++++++++-------- > tools/gpiodetect.c | 122 +- > tools/gpiofind.c | 93 -- > tools/gpioget.c | 252 +-- > tools/gpioinfo.c | 388 ++--- > tools/gpiomon.c | 584 ++++--- > tools/gpionotify.c | 445 ++++++ > tools/gpioset.c | 1057 ++++++++++--- > tools/tools-common.c | 712 ++++++++- > tools/tools-common.h | 98 +- > 15 files changed, 5265 insertions(+), 1608 deletions(-) > delete mode 100644 tools/gpiofind.c > create mode 100644 tools/gpionotify.c > > -- > 2.38.1 > I played with the tools a bit and really like the way they look now. I think they're ready to hop into master, I'll do some more testing and they should be in this week. Just one last request from my side: would you mind updating the TOOLS section of the README? I'm aware it's not yet updated for v2 and I plan to do it soon but we could already start with the tools examples. You can send an incremental patch on top of this series. Bart
On Mon, Nov 14, 2022 at 4:12 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Mon, Nov 14, 2022 at 03:26:38PM +0100, Bartosz Golaszewski wrote: > > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > I played with the tools a bit and really like the way they look now. I > > think they're ready to hop into master, I'll do some more testing and > > they should be in this week. Just one last request from my side: would > > you mind updating the TOOLS section of the README? I'm aware it's not > > yet updated for v2 and I plan to do it soon but we could already start > > with the tools examples. You can send an incremental patch on top of > > this series. > > > > Good point - I totally forgot about the README. > I'll take a look at it. > > Cheers, > Kent. Thanks. Is it ok if I just squash the first three commits in the series when applying to keep it bisectable? Bartosz
On Mon, Nov 14, 2022 at 05:42:35PM +0100, Bartosz Golaszewski wrote: > On Mon, Nov 14, 2022 at 4:12 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Mon, Nov 14, 2022 at 03:26:38PM +0100, Bartosz Golaszewski wrote: > > > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > > I played with the tools a bit and really like the way they look now. I > > > think they're ready to hop into master, I'll do some more testing and > > > they should be in this week. Just one last request from my side: would > > > you mind updating the TOOLS section of the README? I'm aware it's not > > > yet updated for v2 and I plan to do it soon but we could already start > > > with the tools examples. You can send an incremental patch on top of > > > this series. > > > > > > > Good point - I totally forgot about the README. > > I'll take a look at it. > > > > Cheers, > > Kent. > > Thanks. Is it ok if I just squash the first three commits in the > series when applying to keep it bisectable? > Whatever works for you. Cheers, Kent.
On Mon, Nov 14, 2022 at 11:12:25PM +0800, Kent Gibson wrote: > On Mon, Nov 14, 2022 at 03:26:38PM +0100, Bartosz Golaszewski wrote: > > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > I played with the tools a bit and really like the way they look now. I > > think they're ready to hop into master, I'll do some more testing and > > they should be in this week. Just one last request from my side: would > > you mind updating the TOOLS section of the README? I'm aware it's not > > yet updated for v2 and I plan to do it soon but we could already start > > with the tools examples. You can send an incremental patch on top of > > this series. > > > > Good point - I totally forgot about the README. > I'll take a look at it. > And while collecting examples for the README I find the interactive mode tab completion is now broken. It seems I broke it when fixing the prompt behaviour when called from a script. And when fixing that I notice some other odd tab completion behaviour - seems there are at least two separate bugs in in_line_buffer(). All things that gpio-tools-tests doesn't currently test for, so I should extend the test coverage as well. And gpiomon --unquoted aborts as I forgot to cut-and-paste the parser handler - and forgot to add a test for it too. So another fix and more tests to add. I probably wont get that done for a day or two, so you might want to hold off on merging - or I can always provide patches later. I also note that interactive mode does not support quoted line names on the interactive command line yet. Adding that is sure to be entertaining. Do you want to wait for me to patch that, or add it to the TODO list? Cheers, Kent.
On Tue, Nov 15, 2022 at 4:44 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Mon, Nov 14, 2022 at 11:12:25PM +0800, Kent Gibson wrote: > > On Mon, Nov 14, 2022 at 03:26:38PM +0100, Bartosz Golaszewski wrote: > > > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > > I played with the tools a bit and really like the way they look now. I > > > think they're ready to hop into master, I'll do some more testing and > > > they should be in this week. Just one last request from my side: would > > > you mind updating the TOOLS section of the README? I'm aware it's not > > > yet updated for v2 and I plan to do it soon but we could already start > > > with the tools examples. You can send an incremental patch on top of > > > this series. > > > > > > > Good point - I totally forgot about the README. > > I'll take a look at it. > > > > And while collecting examples for the README I find the interactive mode > tab completion is now broken. It seems I broke it when fixing the prompt > behaviour when called from a script. > And when fixing that I notice some other odd tab completion behaviour - > seems there are at least two separate bugs in in_line_buffer(). > All things that gpio-tools-tests doesn't currently test for, so I > should extend the test coverage as well. > > And gpiomon --unquoted aborts as I forgot to cut-and-paste the parser > handler - and forgot to add a test for it too. > So another fix and more tests to add. > > I probably wont get that done for a day or two, so you might want to hold > off on merging - or I can always provide patches later. > > I also note that interactive mode does not support quoted line names on > the interactive command line yet. Adding that is sure to be entertaining. > Do you want to wait for me to patch that, or add it to the TODO list? > Yeah, I can wait. While at it: can you move the following test case: @test "gpioget: with consumer" { gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar gpiosim_chip sim1 num_lines=8 line_name=3:baz line_name=4:xyz dut_run gpionotify --banner -F "%l %E %C" foo baz run_tool gpioget --consumer gpio-tools-tests foo baz status_is 0 dut_read output_regex_match "foo requested gpio-tools-tests" output_regex_match "baz requested gpio-tools-tests" } to after you actually add gpionotify? Otherwise the test suite fails and bisectability is lost. Thanks! Bartosz
On Tue, Nov 15, 2022 at 02:08:40PM +0100, Bartosz Golaszewski wrote: > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > Rework the tools tests and expand to cover new functionality. > > > > Signed-off-by: Kent Gibson <warthog618@gmail.com> > > --- > > tools/gpio-tools-test | 3 - > > tools/gpio-tools-test.bats | 2369 ++++++++++++++++++++++++++++++++++-- > > 2 files changed, 2284 insertions(+), 88 deletions(-) > > > > diff --git a/tools/gpio-tools-test b/tools/gpio-tools-test > > index 234f9bd..1a012dc 100755 > > --- a/tools/gpio-tools-test > > +++ b/tools/gpio-tools-test > > @@ -37,9 +37,6 @@ check_prog() { > > # Check all required non-coreutils tools > > check_prog bats > > check_prog modprobe > > -check_prog rmmod > > -check_prog udevadm > > -check_prog timeout > > We do use timeout in the test-suite even after your changes so I'll > keep this line. > So we do - for the scripts to call themselves - missed that. Fair enough. Cheers, Kent.
On Tue, Nov 15, 2022 at 03:32:57PM +0100, Bartosz Golaszewski wrote: > On Tue, Nov 15, 2022 at 4:44 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Mon, Nov 14, 2022 at 11:12:25PM +0800, Kent Gibson wrote: > > > On Mon, Nov 14, 2022 at 03:26:38PM +0100, Bartosz Golaszewski wrote: > > > > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > > > > > I played with the tools a bit and really like the way they look now. I > > > > think they're ready to hop into master, I'll do some more testing and > > > > they should be in this week. Just one last request from my side: would > > > > you mind updating the TOOLS section of the README? I'm aware it's not > > > > yet updated for v2 and I plan to do it soon but we could already start > > > > with the tools examples. You can send an incremental patch on top of > > > > this series. > > > > > > > > > > Good point - I totally forgot about the README. > > > I'll take a look at it. > > > > > > > And while collecting examples for the README I find the interactive mode > > tab completion is now broken. It seems I broke it when fixing the prompt > > behaviour when called from a script. > > And when fixing that I notice some other odd tab completion behaviour - > > seems there are at least two separate bugs in in_line_buffer(). > > All things that gpio-tools-tests doesn't currently test for, so I > > should extend the test coverage as well. > > > > And gpiomon --unquoted aborts as I forgot to cut-and-paste the parser > > handler - and forgot to add a test for it too. > > So another fix and more tests to add. > > > > I probably wont get that done for a day or two, so you might want to hold > > off on merging - or I can always provide patches later. > > > > I also note that interactive mode does not support quoted line names on > > the interactive command line yet. Adding that is sure to be entertaining. > > Do you want to wait for me to patch that, or add it to the TODO list? > > > > Yeah, I can wait. > It seems that, as per the prompt, libedit doesn't provide tab completion when stdin/stdout isn't a tty, so automated testing of it will require a different approach such as a pty. Not sure how to do that from bash. Unless I have an epiphany, or you have a better suggestion, for the moment I'll just test it manually as it isn't critical functionality - it is just a nice to have. > While at it: can you move the following test case: > > @test "gpioget: with consumer" { > gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar > gpiosim_chip sim1 num_lines=8 line_name=3:baz line_name=4:xyz > > dut_run gpionotify --banner -F "%l %E %C" foo baz > > run_tool gpioget --consumer gpio-tools-tests foo baz > status_is 0 > > dut_read > output_regex_match "foo requested gpio-tools-tests" > output_regex_match "baz requested gpio-tools-tests" > } > > to after you actually add gpionotify? Otherwise the test suite fails > and bisectability is lost. > Well that is iritating. I could rework the test to not rely on gpionotify - it could use the settling period to make gpioget hold the line while gpioinfo checks - but it would be slower and uglier. So I'll add that test with the gpionotify tests, as you suggest. Cheers, Kent.
On Wed, Nov 16, 2022 at 1:12 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Tue, Nov 15, 2022 at 03:32:57PM +0100, Bartosz Golaszewski wrote: > > On Tue, Nov 15, 2022 at 4:44 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > On Mon, Nov 14, 2022 at 11:12:25PM +0800, Kent Gibson wrote: > > > > On Mon, Nov 14, 2022 at 03:26:38PM +0100, Bartosz Golaszewski wrote: > > > > > On Mon, Nov 14, 2022 at 5:01 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > > > > > > > > > I played with the tools a bit and really like the way they look now. I > > > > > think they're ready to hop into master, I'll do some more testing and > > > > > they should be in this week. Just one last request from my side: would > > > > > you mind updating the TOOLS section of the README? I'm aware it's not > > > > > yet updated for v2 and I plan to do it soon but we could already start > > > > > with the tools examples. You can send an incremental patch on top of > > > > > this series. > > > > > > > > > > > > > Good point - I totally forgot about the README. > > > > I'll take a look at it. > > > > > > > > > > And while collecting examples for the README I find the interactive mode > > > tab completion is now broken. It seems I broke it when fixing the prompt > > > behaviour when called from a script. > > > And when fixing that I notice some other odd tab completion behaviour - > > > seems there are at least two separate bugs in in_line_buffer(). > > > All things that gpio-tools-tests doesn't currently test for, so I > > > should extend the test coverage as well. > > > > > > And gpiomon --unquoted aborts as I forgot to cut-and-paste the parser > > > handler - and forgot to add a test for it too. > > > So another fix and more tests to add. > > > > > > I probably wont get that done for a day or two, so you might want to hold > > > off on merging - or I can always provide patches later. > > > > > > I also note that interactive mode does not support quoted line names on > > > the interactive command line yet. Adding that is sure to be entertaining. > > > Do you want to wait for me to patch that, or add it to the TODO list? > > > > > > > Yeah, I can wait. > > > > It seems that, as per the prompt, libedit doesn't provide tab completion > when stdin/stdout isn't a tty, so automated testing of it will require a > different approach such as a pty. Not sure how to do that from bash. > Unless I have an epiphany, or you have a better suggestion, for the moment > I'll just test it manually as it isn't critical functionality - it is just > a nice to have. > Sounds good, just add it to the TODO file in the repo. Bart > > While at it: can you move the following test case: > > > > @test "gpioget: with consumer" { > > gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar > > gpiosim_chip sim1 num_lines=8 line_name=3:baz line_name=4:xyz > > > > dut_run gpionotify --banner -F "%l %E %C" foo baz > > > > run_tool gpioget --consumer gpio-tools-tests foo baz > > status_is 0 > > > > dut_read > > output_regex_match "foo requested gpio-tools-tests" > > output_regex_match "baz requested gpio-tools-tests" > > } > > > > to after you actually add gpionotify? Otherwise the test suite fails > > and bisectability is lost. > > > > Well that is iritating. I could rework the test to not rely on > gpionotify - it could use the settling period to make gpioget hold the > line while gpioinfo checks - but it would be slower and uglier. > So I'll add that test with the gpionotify tests, as you suggest. > > Cheers, > Kent.
This patch series is an optimistic reimagining of the tools intended to simplify usage for well configured systems, i.e. for systems where lines can be uniquely identified by name. In such systems the chip and offset location of the line is no longer of relevance to the user, so the tools should be able to operate without mentioning them. e.g. gpioget GPIO17 gpioset GPIO17=active gpiomon --localtime GPIO17 GPIO18 It is accepted that the kernel does not guarantee line name uniqueness within the system, or even within a chip, and not all systems are well configured, so the tools retain the option to identify lines by chip and offset. The hope and expectation is that over time systems will become more well configured, not less, and identification of GPIO lines by name will become the norm. The core of the series is patch 1 which is a reworking of the tools to support identifying lines by name, and to operate across multiple GPIO chips if named lines are located on different chips. The gpioset tool is extended to support toggling lines and interactive control of line values, so some common use cases can be trivially implemented from the command line. e.g. gpioset --toggle 500ms LED=on will blink the LED line at 1Hz, indefinitely. More complex outputs can be generated by adding more entries to the toggle sequence: gpioset --toggle 1s,2s,1s,300ms LED=on Even more complex outputs can be generated by driving gpioset in interactive mode from another script. Those are the major changes. A more complete list of the changes can be found in the patch description. The core tool changes are contained in patch 2. To simplify review, patch 1 removes old code replaced by that in patch 2 and 3. Patch 1 also removes gpiofind, as that tools functionality is absorbed by the other commands, particularly gpioinfo. Patch 3 updates and extends the tool tests to cover the reworked tools, including demonstrating gpioset being driven interactively via a script. Patch 4 adds a gpionotify tool that monitors changes to the state line information, similar to the gpio-watch tool in the kernel, and patch 5 extends the test suite to cover it. Cheers, Kent. Changes v3 -> v4: - rebase on master following merge with next/libgpiod-2.0 - C style comments - again. - rename gpiowatch to gpionotify - make gpioset interactive mode optional, enabled with --enable-gpioset-interactive. - gpioset does not exit by default - add a banner option to gpioset as it can be long lived - make some functions and variables static - move parse_periods_or_die from tools-common to gpioset - quote line and consumer names - add option to not quote line and consumer names - always use bool, not int, for command line flags - add --consumer option to commands that request lines (get/set/mon) - move parse_periods_or_die() from tools-common to gpioset Changes v2 -> v3: - squash removal of gpiofind into patch 1 (was patch 6). - rebase to C API line_config changes. - rework line name to chip/offset resolution to improve clarity and better handle corner cases. - drop bias=as-is as a command line option as that is the default behaviour. - revise gpioinfo output format to combine the used flag and consumer name, and to remove the brackets around the list of attributes. - gpiowatch: rework so it is more like gpiomon than the Linux gpio-watch tool. More details in patch 4. - quote text from the command line when used in error messages. - improve test suite coverage of corner cases. - gpiomon: rename --edge option to --edges, and drop "-edges" from the possible values, e.g. --edges=rising. - add hte support to gpiomon. - gpiomon: decouple selection of event clock from timestamp output formatting. Changes v1 -> v2: - code formatting, particularly trying to keep to the 80 character limit and C style comments. - move global config fields into the struct config for each tool. - switch gpioset from readline to libedit. - add tests for symlink chip path behaviour. - long lived tools flush stdout before blocking. - fix copyrights - replace gpiosim attr lookup functions with cached values. - remove gpiofind Kent Gibson (5): tools: remove old code to simplify review tools: line name focussed rework tools: tests for line name focussed rework tools: add gpionotify tools: gpionotify tests configure.ac | 24 +- man/Makefile.am | 2 +- tools/.gitignore | 2 +- tools/Makefile.am | 12 +- tools/gpio-tools-test | 3 - tools/gpio-tools-test.bats | 3079 ++++++++++++++++++++++++++++-------- tools/gpiodetect.c | 122 +- tools/gpiofind.c | 93 -- tools/gpioget.c | 252 +-- tools/gpioinfo.c | 388 ++--- tools/gpiomon.c | 584 ++++--- tools/gpionotify.c | 445 ++++++ tools/gpioset.c | 1057 ++++++++++--- tools/tools-common.c | 712 ++++++++- tools/tools-common.h | 98 +- 15 files changed, 5265 insertions(+), 1608 deletions(-) delete mode 100644 tools/gpiofind.c create mode 100644 tools/gpionotify.c