mbox series

[libgpiod,v2,v5,0/6] tools: improvements for v2

Message ID 20221121102253.38306-1-warthog618@gmail.com
Headers show
Series tools: improvements for v2 | expand

Message

Kent Gibson Nov. 21, 2022, 10:22 a.m. UTC
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.

Patch 6 updates the README with examples for the updated tools.

Cheers,
Kent.

Changes v4 -> v5:
  - fix gpiomon --unquoted, which was only partially implemented in v4.
  - add tests for --unquoted for those tools overlooked in v4 (gpioinfo,
    gpiomon, gpionotify).
  - restore check for timeout in gpio-tools-test.
  - allow gpioset --daemonize --interactive, in which case stdio files
    are inherited from the parent rather than being redirected to /dev/null.
    Not sure how useful this mode is, but don't see any harm in allowing it.
  - move test for gpioget --consumer from patch 3 to patch 5, as it requires
    gpionotify.
  - add support for quoted line names in commands to gpioset --interactive.
  - fix bugs in gpioset --interactive tab completion.  Tab completion for
    quoted line names is still a WIP and tab completion currently has no
    automated tests, as mentioned in the TODO.
  - Update tool examples in README (patch 6).

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 (6):
  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
  README: update Tools section for v2 tools

 README                     |  176 +-
 TODO                       |   12 +
 configure.ac               |   24 +-
 man/Makefile.am            |    2 +-
 tools/.gitignore           |    2 +-
 tools/Makefile.am          |   11 +-
 tools/gpio-tools-test      |    2 -
 tools/gpio-tools-test.bats | 3101 ++++++++++++++++++++++++++++--------
 tools/gpiodetect.c         |  122 +-
 tools/gpiofind.c           |   93 --
 tools/gpioget.c            |  252 +--
 tools/gpioinfo.c           |  388 ++---
 tools/gpiomon.c            |  586 ++++---
 tools/gpionotify.c         |  445 ++++++
 tools/gpioset.c            | 1117 ++++++++++---
 tools/tools-common.c       |  712 ++++++++-
 tools/tools-common.h       |   98 +-
 17 files changed, 5495 insertions(+), 1648 deletions(-)
 delete mode 100644 tools/gpiofind.c
 create mode 100644 tools/gpionotify.c

Comments

Bartosz Golaszewski Nov. 21, 2022, 8:25 p.m. UTC | #1
On Mon, Nov 21, 2022 at 11:23 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.
>
> Patch 6 updates the README with examples for the updated tools.
>
> Cheers,
> Kent.
>
> Changes v4 -> v5:
>   - fix gpiomon --unquoted, which was only partially implemented in v4.
>   - add tests for --unquoted for those tools overlooked in v4 (gpioinfo,
>     gpiomon, gpionotify).
>   - restore check for timeout in gpio-tools-test.
>   - allow gpioset --daemonize --interactive, in which case stdio files
>     are inherited from the parent rather than being redirected to /dev/null.
>     Not sure how useful this mode is, but don't see any harm in allowing it.
>   - move test for gpioget --consumer from patch 3 to patch 5, as it requires
>     gpionotify.
>   - add support for quoted line names in commands to gpioset --interactive.
>   - fix bugs in gpioset --interactive tab completion.  Tab completion for
>     quoted line names is still a WIP and tab completion currently has no
>     automated tests, as mentioned in the TODO.
>   - Update tool examples in README (patch 6).
>
> 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 (6):
>   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
>   README: update Tools section for v2 tools
>
>  README                     |  176 +-
>  TODO                       |   12 +
>  configure.ac               |   24 +-
>  man/Makefile.am            |    2 +-
>  tools/.gitignore           |    2 +-
>  tools/Makefile.am          |   11 +-
>  tools/gpio-tools-test      |    2 -
>  tools/gpio-tools-test.bats | 3101 ++++++++++++++++++++++++++++--------
>  tools/gpiodetect.c         |  122 +-
>  tools/gpiofind.c           |   93 --
>  tools/gpioget.c            |  252 +--
>  tools/gpioinfo.c           |  388 ++---
>  tools/gpiomon.c            |  586 ++++---
>  tools/gpionotify.c         |  445 ++++++
>  tools/gpioset.c            | 1117 ++++++++++---
>  tools/tools-common.c       |  712 ++++++++-
>  tools/tools-common.h       |   98 +-
>  17 files changed, 5495 insertions(+), 1648 deletions(-)
>  delete mode 100644 tools/gpiofind.c
>  create mode 100644 tools/gpionotify.c
>
> --
> 2.38.1
>

Thank you Kent for your great work. I applied this, after squashing
the first three patches into one (while keeping the commit message
from 2/6) and applying some additional changes from my side (mostly
tweaking coding-style more to my liking, nothing functional).

I also applied Viresh' rust bindings and at this point I think we have
all the features in for v2.0. I think that the DBus API will have to
wait for v2.1.

I still have a couple things lined up for v2.0 so I'll send those soon
and then let's work towards stabilizing the API, tagging some RCs and
let's get that thing released finally. :)

Bart