Skip to contentSkip to conversation

tools: improvements for v2

PATCH Superseded Kent Gibson 2 participants 16 messages Nov 14, 2022 – Nov 16, 2022 GPIO

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Kent Gibson

author
#0 lore raw

v4 cover tools: improvements for v2

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
Signature · 1 line
2.38.1
author
#1 lore raw

PATCH v4 1/5 tools: remove old code to simplify review

Remove functions that are so heavily modified in the subsequent patch that they are effectively complete re-writes, so the old code just obfuscates the new.

The gpiofind tool is removed completely, as its functionality is absorbed into the other tools.

Signed-off-by Kent Gibson <[email protected]>
---
 man/Makefile.am            |   2 +-
 tools/.gitignore           |   1 -
 tools/Makefile.am          |   4 +-
 tools/gpio-tools-test.bats | 893 -------------------------------------
 tools/gpiodetect.c         |  79 ----
 tools/gpiofind.c           |  93 ----
 tools/gpioget.c            | 153 -------
 tools/gpioinfo.c           | 240 ----------
 tools/gpiomon.c            | 314 -------------
 tools/gpioset.c            | 330 --------------
 tools/tools-common.c       |  81 ----
 11 files changed, 2 insertions(+), 2188 deletions(-)
 delete mode 100644 tools/gpiofind.c
man/Makefile.am +1 -1
index 4d2c29b..8d1d9b3 100644
@@ -3,7 +3,7 @@
 
 if WITH_MANPAGES
 
-dist_man1_MANS = gpiodetect.man gpioinfo.man gpioget.man gpioset.man gpiofind.man gpiomon.man
+dist_man1_MANS = gpiodetect.man gpioinfo.man gpioget.man gpioset.man gpiomon.man
 
 %.man: $(top_builddir)/tools/$(*F)
 	help2man $(top_builddir)/tools/$(*F) --include=$(srcdir)/template --output=$(builddir)/$@ --no-info
tools/.gitignore +0 -1
index 0d53de9..d6b2f44 100644
@@ -6,4 +6,3 @@ gpioinfo
 gpioget
 gpioset
 gpiomon
-gpiofind
tools/Makefile.am +1 -3
index 4a13266..fc074b9 100644
@@ -9,7 +9,7 @@ libtools_common_la_SOURCES = tools-common.c tools-common.h
 
 LDADD = libtools-common.la $(top_builddir)/lib/libgpiod.la
 
-bin_PROGRAMS = gpiodetect gpioinfo gpioget gpioset gpiomon gpiofind
+bin_PROGRAMS = gpiodetect gpioinfo gpioget gpioset gpiomon
 
 gpiodetect_SOURCES = gpiodetect.c
 

@@ -21,8 +21,6 @@ gpioset_SOURCES = gpioset.c
 
 gpiomon_SOURCES = gpiomon.c
 
-gpiofind_SOURCES = gpiofind.c
-
 EXTRA_DIST = gpio-tools-test gpio-tools-test.bats
 
 if WITH_TESTS
tools/gpio-tools-test.bats +0 -893

896 lines not on this page · raw message

tools/gpiodetect.c +0 -79

82 lines not on this page · raw message

tools/gpiofind.c deleted +0 -93

93 lines not on this page · raw message

tools/gpioget.c +0 -153

156 lines not on this page · raw message

tools/gpioinfo.c +0 -240

243 lines not on this page · raw message

tools/gpiomon.c +0 -314

317 lines not on this page · raw message

tools/gpioset.c +0 -330

333 lines not on this page · raw message

tools/tools-common.c +0 -81

96 lines not on this page · raw message

Signature · 1 line
2.38.1
author
#2 lore raw

PATCH v4 2/5 tools: line name focussed rework

In reply to Kent Gibson

Rework the tool suite to support identifying lines by name and to support operating on the GPIO lines available to the user at once, rather than on one particular GPIO chip.

All tools, other than gpiodetect, now provide the name to (chip,offset) mapping that was previously only performed by gpiofind. As names are not guaranteed to be unique, a --strict option is provided for all tools to either abort the operation or report all lines with the matching name, as appropriate.
By default the tools operate on the first line found with a matching name.

Selection of line by (chip,offset) is still supported with a --chip option, though it restricts the scope of the operation to an individual chip. When the --chip option is specified, the lines are assumed to be identified by offset where they parse as an integer, else by name. To cater for the unusual case where a line name parses as an integer, but is different from the offset, the --by-name option forces the lines to be identified by name.

The updated tools are intentionally NOT backwardly compatible with the previous tools. Using old command lines with the updated tools will almost certainly fail, though migrating old command lines is generally as simple as adding a '-c' before the chip.

In addition the individual tools are modified as follows:

gpiodetect:

Add the option to select individual chips.

gpioinfo:

Change the focus from chips to lines, so the scope can be an individual line, a subset of lines, all lines on a particular chip, or all the lines available to the user. For line scope a single line summary is output for each line. For chip scope the existing format displaying a summary of the chip and each of its lines is retained.

Line attributes are consolidated into a list format, and are extended to cover all attributes supported by uAPI v2.

gpioget:

The default output format is becomes line=value, as per the input for gpioset, and the value is reported as active or inactive, rather than 0 or 1.
The previous format is available using the --numeric option.

Add an optional hold period between requesting a line and reading the value to allow the line to settle once the requested configuration has been applied (e.g. bias).

gpiomon:

Consolidate the edge options into a single option.

Add a debounce period option.

Add options to report event times as UTC or localtime.

Add format specifiers for GPIO chip path, line name, stringified event type, and event time as a datetime.

Rearrange default output format to place fields with more predicable widths to the left, and to separate major field groups with tabs. Lines are identified consistent with the command line.

gpioset:

Add a hold period option that specifies the minimum period the line value must be held for. This applies to all set options.

Support line values specified as active/inactive, on/off and true/false, as well as 1/0.

Add a toggle option that specifies a time sequence over which the requested lines should be toggled. If the sequence is 0 terminated then gpioset exits when the sequence completes, else it repeats the sequence. This allows for anything from simple blinkers to bit bashing from the command line. e.g. gpioset -t 500ms LED=on

Add an interactive option to provide a shell-like interface to allow manual or scripted manipulation of requested lines. A basic command set allows lines to be get, set, or toggled, and to insert sleeps between operations.

Remove the --mode, --sec, and --usec options.
The combination of hold period and interactive mode provide functionality equivalent to the old --mode options. By default gpioset now holds the line indefinitely, rather than exiting immediately. The old exit behaviour can be emulated with a "-t 0" option.

Signed-off-by Kent Gibson <[email protected]>
---
 configure.ac         |  24 +-
 tools/Makefile.am    |   8 +-
 tools/gpiodetect.c   | 113 +++++-
 tools/gpioget.c      | 225 ++++++++++-
 tools/gpioinfo.c     | 250 +++++++++++-
 tools/gpiomon.c      | 468 +++++++++++++++++++++-
 tools/gpioset.c      | 915 ++++++++++++++++++++++++++++++++++++++++++-
 tools/tools-common.c | 693 +++++++++++++++++++++++++++++++-
 tools/tools-common.h |  98 ++++-
 9 files changed, 2745 insertions(+), 49 deletions(-)
configure.ac +13 -11
index 048b2ac..23d9060 100644
@@ -103,18 +103,20 @@ AM_CONDITIONAL([WITH_TOOLS], [test "x$with_tools" = xtrue])
 AC_DEFUN([FUNC_NOT_FOUND_TOOLS],
 	[ERR_NOT_FOUND([$1()], [tools])])
 
-AC_DEFUN([HEADER_NOT_FOUND_TOOLS],
-	[ERR_NOT_FOUND([$1 header], [tools])])
-
-if test "x$with_tools" = xtrue
-then
-	# These are only needed to build tools
-	AC_CHECK_FUNC([basename], [], [FUNC_NOT_FOUND_TOOLS([basename])])
+AC_ARG_ENABLE([gpioset-interactive],
+	[AS_HELP_STRING([--enable-gpioset-interactive],
+		[enable gpioset interactive mode [default=no]])],
+	[if test "x$enableval" = xyes; then with_gpioset_interactive=true; fi],
+	[with_gpioset_interactive=false])
+AM_CONDITIONAL([WITH_GPIOSET_INTERACTIVE],
+	[test "x$with_gpioset_interactive" = xtrue])
+
+AS_IF([test "x$with_tools" = xtrue],
+	[# These are only needed to build tools
 	AC_CHECK_FUNC([daemon], [], [FUNC_NOT_FOUND_TOOLS([daemon])])
-	AC_CHECK_FUNC([signalfd], [], [FUNC_NOT_FOUND_TOOLS([signalfd])])
-	AC_CHECK_FUNC([setlinebuf], [], [FUNC_NOT_FOUND_TOOLS([setlinebuf])])
-	AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])])
-fi
+	AS_IF([test "x$with_gpioset_interactive" = xtrue],
+		[PKG_CHECK_MODULES([LIBEDIT], [libedit >= 3.1])])
+	])
 
 AC_ARG_ENABLE([tests],
 	[AS_HELP_STRING([--enable-tests],[enable libgpiod tests [default=no]])],
tools/Makefile.am +7 -1
index fc074b9..3fafec6 100644
@@ -4,10 +4,16 @@
 AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h
 AM_CFLAGS += -Wall -Wextra -g -std=gnu89
 
+if WITH_GPIOSET_INTERACTIVE
+
+AM_CFLAGS += -DGPIOSET_INTERACTIVE
+
+endif
+
 noinst_LTLIBRARIES = libtools-common.la
 libtools_common_la_SOURCES = tools-common.c tools-common.h
 
-LDADD = libtools-common.la $(top_builddir)/lib/libgpiod.la
+LDADD = libtools-common.la $(top_builddir)/lib/libgpiod.la $(LIBEDIT_LIBS)
 
 bin_PROGRAMS = gpiodetect gpioinfo gpioget gpioset gpiomon
tools/gpiodetect.c +111 -2

122 lines not on this page · raw message

tools/gpioget.c +224 -1

236 lines not on this page · raw message

tools/gpioinfo.c +248 -2

259 lines not on this page · raw message

tools/gpiomon.c +464 -4

483 lines not on this page · raw message

tools/gpioset.c +913 -2

928 lines not on this page · raw message

tools/tools-common.c +674 -19

742 lines not on this page · raw message

tools/tools-common.h +91 -7

117 lines not on this page · raw message

Signature · 1 line
2.38.1
author
#3 lore raw

PATCH v4 3/5 tools: tests for line name focussed rework

In reply to Kent Gibson

Rework the tools tests and expand to cover new functionality.

Signed-off-by Kent Gibson <[email protected]>
---
 tools/gpio-tools-test      |    3 -
 tools/gpio-tools-test.bats | 2369 ++++++++++++++++++++++++++++++++++--
 2 files changed, 2284 insertions(+), 88 deletions(-)
tools/gpio-tools-test +0 -3
index 234f9bd..1a012dc 100755
@@ -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
 
 # Check if we're running a kernel at the required version or later
 check_kernel $MIN_KERNEL_VERSION
tools/gpio-tools-test.bats +2284 -85

2,470 lines not on this page · raw message

Signature · 1 line
2.38.1
#4 lore raw

Re: [libgpiod v2][PATCH v4 3/5] tools: tests for line name focussed rework

Quoted 14 lines · Kent Gibson

Rework the tools tests and expand to cover new functionality.

Signed-off-by Kent Gibson <[email protected]>
---
 tools/gpio-tools-test      |    3 -
 tools/gpio-tools-test.bats | 2369 ++++++++++++++++++++++++++++++++++--
 2 files changed, 2284 insertions(+), 88 deletions(-)
tools/gpio-tools-test +0 -0
index 234f9bd..1a012dc 100755

tools/gpio-tools-test check_prog() { · line 42

Quoted 6 lines
tools/gpio-tools-test +0 -3
@@ -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.

Bart

author
#5 lore raw

Re: [libgpiod v2][PATCH v4 3/5] tools: tests for line name focussed rework

Quoted 10 lines · Bartosz Golaszewski
Quoted 9 lines · Kent Gibson

Rework the tools tests and expand to cover new functionality.

Signed-off-by Kent Gibson <[email protected]>
---
 tools/gpio-tools-test      |    3 -
 tools/gpio-tools-test.bats | 2369 ++++++++++++++++++++++++++++++++++--
 2 files changed, 2284 insertions(+), 88 deletions(-)
Quoted 15 lines

tools/gpio-tools-test check_prog() { · line 42

Quoted 11 lines
tools/gpio-tools-test +0 -3
index 234f9bd..1a012dc 100755
@@ -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.

author
#6 lore raw

PATCH v4 4/5 tools: add gpionotify

In reply to Kent Gibson

Add a gpionotify tool, based on gpiomon, to report line info change events read from chip file descriptors.

Inspired by the gpio-watch tool in the linux kernel, but with gpiomon features such as custom formatted output, filtering events of interest and exiting after a number of events, so more useful for scripting.

Default output is minimalist, so just time, event type and line id. Full event details are available using the custom formatted output.

Signed-off-by Kent Gibson <[email protected]>
---
 man/Makefile.am    |   2 +-
 tools/.gitignore   |   1 +
 tools/Makefile.am  |   4 +-
 tools/gpionotify.c | 445 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 450 insertions(+), 2 deletions(-)
 create mode 100644 tools/gpionotify.c
man/Makefile.am +1 -1
index 8d1d9b3..201a52b 100644
@@ -3,7 +3,7 @@
 
 if WITH_MANPAGES
 
-dist_man1_MANS = gpiodetect.man gpioinfo.man gpioget.man gpioset.man gpiomon.man
+dist_man1_MANS = gpiodetect.man gpioinfo.man gpioget.man gpioset.man gpiomon.man gpionotify.man
 
 %.man: $(top_builddir)/tools/$(*F)
 	help2man $(top_builddir)/tools/$(*F) --include=$(srcdir)/template --output=$(builddir)/$@ --no-info
tools/.gitignore +1 -0
index d6b2f44..dfdbc0d 100644
@@ -6,3 +6,4 @@ gpioinfo
 gpioget
 gpioset
 gpiomon
+gpionotify
tools/Makefile.am +3 -1
index 3fafec6..b6e8f34 100644
@@ -15,7 +15,7 @@ libtools_common_la_SOURCES = tools-common.c tools-common.h
 
 LDADD = libtools-common.la $(top_builddir)/lib/libgpiod.la $(LIBEDIT_LIBS)
 
-bin_PROGRAMS = gpiodetect gpioinfo gpioget gpioset gpiomon
+bin_PROGRAMS = gpiodetect gpioinfo gpioget gpioset gpiomon gpionotify
 
 gpiodetect_SOURCES = gpiodetect.c
 

@@ -27,6 +27,8 @@ gpioset_SOURCES = gpioset.c
 
 gpiomon_SOURCES = gpiomon.c
 
+gpionotify_SOURCES = gpionotify.c
+
 EXTRA_DIST = gpio-tools-test gpio-tools-test.bats
 
 if WITH_TESTS
tools/gpionotify.c new file +445 -0

445 lines not on this page · raw message

Signature · 1 line
2.38.1
author
#7 lore raw

PATCH v4 5/5 tools: gpionotify tests

In reply to Kent Gibson

Extend the tool test suite to cover the gpionotify tool.

Signed-off-by Kent Gibson <[email protected]>
---
 tools/gpio-tools-test.bats | 503 +++++++++++++++++++++++++++++++++++++
 1 file changed, 503 insertions(+)
tools/gpio-tools-test.bats +503 -0
index 78b0959..d73cf9b 100755
@@ -2419,3 +2419,506 @@ request_release_line() {
 	dut_read
 	output_is "%x"
 }
+
+#
+# gpionotify test cases
+#
+
+@test "gpionotify: by name" {
+	gpiosim_chip sim0 num_lines=8 line_name=4:foo
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner foo
+	dut_regex_match "Watching line .*"
+
+	request_release_line $sim0 4
+
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"foo\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"foo\""
+	# tools currently have no way to generate a reconfig event
+}
+
+@test "gpionotify: by offset" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --chip $sim0 4
+	dut_regex_match "Watching line .*"
+
+	request_release_line $sim0 4
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 4"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 4"
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: by symlink" {
+	gpiosim_chip sim0 num_lines=8
+	gpiosim_chip_symlink sim0 .
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --chip $GPIOSIM_CHIP_LINK 4
+	dut_regex_match "Watching line .*"
+
+	request_release_line $sim0 4
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0\\s+4"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0\\s+4"
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: by chip and name" {
+	gpiosim_chip sim0 num_lines=8 line_name=4:foo
+	gpiosim_chip sim1 num_lines=8 line_name=2:foo
+
+	local sim1=${GPIOSIM_CHIP_NAME[sim1]}
+
+	dut_run gpionotify --banner --chip $sim1 foo
+	dut_regex_match "Watching line .*"
+
+	request_release_line $sim1 2
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim1 2 \"foo\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim1 2 \"foo\""
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: first matching named line" {
+	gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar \
+				      line_name=3:foobar
+	gpiosim_chip sim1 num_lines=8 line_name=0:baz line_name=2:foobar \
+				      line_name=4:xyz line_name=7:foobar
+	gpiosim_chip sim2 num_lines=16
+
+	dut_run gpionotify --banner foobar
+	dut_regex_match "Watching line .*"
+
+	request_release_line ${GPIOSIM_CHIP_NAME[sim0]} 3
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"foobar\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"foobar\""
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: with requested" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	gpiosim_set_pull sim0 4 pull-up
+
+	dut_run gpionotify --banner --event=requested --chip $sim0 4
+	dut_flush
+
+	request_release_line ${GPIOSIM_CHIP_NAME[sim0]} 4
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 4"
+	assert_fail dut_readable
+}
+
+@test "gpionotify: with released" {
+	gpiosim_chip sim0 num_lines=8
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	gpiosim_set_pull sim0 4 pull-down
+
+	dut_run gpionotify --banner --event=released --chip $sim0 4
+	dut_flush
+
+	request_release_line ${GPIOSIM_CHIP_NAME[sim0]} 4
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 4"
+	assert_fail dut_readable
+}
+
+@test "gpionotify: with quiet mode" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --quiet --chip $sim0 4
+	dut_flush
+
+	request_release_line ${GPIOSIM_CHIP_NAME[sim0]} 4
+	assert_fail dut_readable
+}
+
+@test "gpionotify: with num-events" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	# redirect, as gpionotify exits after 4 events
+	dut_run_redirect gpionotify --num-events=4 --chip $sim0 3 4
+
+
+	request_release_line ${GPIOSIM_CHIP_NAME[sim0]} 4
+	request_release_line ${GPIOSIM_CHIP_NAME[sim0]} 3
+
+	dut_wait
+	status_is 0
+	dut_read_redirect
+
+	regex_matches "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 4" "${lines[0]}"
+	regex_matches "[0-9]+\.[0-9]+\\s+released\\s+$sim0 4" "${lines[1]}"
+	regex_matches "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 3" "${lines[2]}"
+	regex_matches "[0-9]+\.[0-9]+\\s+released\\s+$sim0 3" "${lines[3]}"
+	num_lines_is 4
+}
+
+@test "gpionotify: multiple lines" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --chip $sim0 1 2 3 4 5
+	dut_regex_match "Watching lines .*"
+
+	request_release_line $sim0 2
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 2"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 2"
+
+	request_release_line $sim0 3
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 3"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 3"
+
+	request_release_line $sim0 4
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 4"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 4"
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: multiple lines by name and offset" {
+	gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --chip $sim0 bar foo 3
+	dut_regex_match "Watching lines .*"
+
+	request_release_line $sim0 2
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 2\\s+\"bar\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 2\\s+\"bar\""
+
+	request_release_line $sim0 1
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 1\\s+\"foo\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 1\\s+\"foo\""
+
+	request_release_line $sim0 3
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 3"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 3"
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: multiple lines across multiple chips" {
+	gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar
+	gpiosim_chip sim1 num_lines=8 line_name=0:baz line_name=4:xyz
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+	local sim1=${GPIOSIM_CHIP_NAME[sim1]}
+
+	dut_run gpionotify --banner baz bar foo xyz
+	dut_regex_match "Watching lines .*"
+
+	request_release_line $sim0 2
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"bar\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"bar\""
+
+	request_release_line $sim0 1
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"foo\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"foo\""
+
+	request_release_line $sim1 4
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"xyz\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"xyz\""
+
+	request_release_line $sim1 0
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+\"baz\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+\"baz\""
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: exit after SIGINT" {
+	gpiosim_chip sim0 num_lines=8
+
+	dut_run gpionotify --banner --chip ${GPIOSIM_CHIP_NAME[sim0]} 4
+	dut_regex_match "Watching line .*"
+
+	dut_kill -SIGINT
+	dut_wait
+
+	status_is 130
+}
+
+@test "gpionotify: exit after SIGTERM" {
+	gpiosim_chip sim0 num_lines=8
+
+	dut_run gpionotify --banner --chip ${GPIOSIM_CHIP_NAME[sim0]} 4
+	dut_regex_match "Watching line .*"
+
+	dut_kill -SIGTERM
+	dut_wait
+
+	status_is 143
+}
+
+@test "gpionotify: with nonexistent line" {
+	run_tool gpionotify nonexistent-line
+
+	status_is 1
+	output_regex_match ".*cannot find line 'nonexistent-line'"
+}
+
+@test "gpionotify: with same line twice" {
+	gpiosim_chip sim0 num_lines=8 line_name=1:foo
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	# by offset
+	run_tool gpionotify --chip $sim0 0 0
+
+	output_regex_match ".*lines '0' and '0' are the same line"
+	num_lines_is 1
+	status_is 1
+
+	# by name
+	run_tool gpionotify foo foo
+
+	output_regex_match ".*lines 'foo' and 'foo' are the same line"
+	num_lines_is 1
+	status_is 1
+
+	# by name and offset
+	run_tool gpionotify --chip $sim0 1 foo
+
+	output_regex_match ".*lines '1' and 'foo' are the same line"
+	num_lines_is 1
+	status_is 1
+}
+
+@test "gpionotify: with strict named line check" {
+	gpiosim_chip sim0 num_lines=4 line_name=1:foo line_name=2:bar \
+				      line_name=3:foobar
+	gpiosim_chip sim1 num_lines=8 line_name=0:baz line_name=2:foobar \
+				      line_name=4:xyz line_name=7:foobar
+	gpiosim_chip sim2 num_lines=16
+
+	run_tool gpionotify --strict foobar
+
+	output_regex_match ".*line 'foobar' is not unique"
+	status_is 1
+}
+
+@test "gpionotify: with lines by offset" {
+	# not suggesting this setup makes any sense
+	# - just test that we can deal with it
+	gpiosim_chip sim0 num_lines=8 line_name=1:6 line_name=6:1
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --chip $sim0 1
+	dut_flush
+
+	request_release_line $sim0 1
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 1"
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 1"
+
+	request_release_line $sim0 6
+
+	assert_fail dut_readable
+}
+
+@test "gpionotify: with lines strictly by name" {
+	# not suggesting this setup makes any sense
+	# - just test that we can deal with it
+	gpiosim_chip sim0 num_lines=8 line_name=1:6 line_name=6:1
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --by-name --chip $sim0 1
+	dut_flush
+
+	request_release_line $sim0 6
+	dut_regex_match "[0-9]+\.[0-9]+\\s+requested\\s+$sim0 6 \"1\""
+	dut_regex_match "[0-9]+\.[0-9]+\\s+released\\s+$sim0 6 \"1\""
+
+	request_release_line $sim0 1
+	assert_fail dut_readable
+}
+
+@test "gpionotify: with no arguments" {
+	run_tool gpionotify
+
+	output_regex_match ".*at least one GPIO line must be specified"
+	status_is 1
+}
+
+@test "gpionotify: with no line specified" {
+	gpiosim_chip sim0 num_lines=8
+
+	run_tool gpionotify --chip ${GPIOSIM_CHIP_NAME[sim0]}
+
+	output_regex_match ".*at least one GPIO line must be specified"
+	status_is 1
+}
+
+@test "gpionotify: with offset out of range" {
+	gpiosim_chip sim0 num_lines=4
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	run_tool gpionotify --chip $sim0 5
+
+	output_regex_match ".*offset 5 is out of range on chip '$sim0'"
+	status_is 1
+}
+
+@test "gpionotify: with custom format (event type + offset)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=%e %o" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "1 4"
+}
+
+@test "gpionotify: with custom format (event type + offset joined)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=%e%o" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "14"
+}
+
+@test "gpionotify: with custom format (event, chip and line)" {
+	gpiosim_chip sim0 num_lines=8 line_name=4:baz
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=released \
+		"--format=%e %o %E %c %l" -c $sim0 baz
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_regex_match "2 4 released $sim0 baz"
+}
+
+@test "gpionotify: with custom format (seconds timestamp)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=%e %o %S" \
+		-c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_regex_match "1 4 [0-9]+\\.[0-9]+"
+}
+
+@test "gpionotify: with custom format (UTC timestamp)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=released \
+		"--format=%U %e %o" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_regex_match \
+"[0-9][0-9][0-9][0-9]-[0-1][0-9]-[0-3][0-9]T[0-2][0-9]:[0-5][0-9]:[0-5][0-9]\\.[0-9]+Z 2 4"
+}
+
+@test "gpionotify: with custom format (localtime timestamp)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=released \
+		"--format=%L %e %o" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_regex_match \
+"[0-9][0-9][0-9][0-9]-[0-1][0-9]-[0-3][0-9]T[0-2][0-9]:[0-5][0-9]:[0-5][0-9]\\.[0-9]+ 2 4"
+}
+
+@test "gpionotify: with custom format (double percent sign)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=start%%end" \
+		-c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "start%end"
+}
+
+@test "gpionotify: with custom format (double percent sign + event type specifier)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=%%e" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "%e"
+}
+
+@test "gpionotify: with custom format (single percent sign)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=%" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "%"
+}
+
+@test "gpionotify: with custom format (single percent sign between other characters)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=foo % bar" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "foo % bar"
+}
+
+@test "gpionotify: with custom format (unknown specifier)" {
+	gpiosim_chip sim0 num_lines=8
+
+	local sim0=${GPIOSIM_CHIP_NAME[sim0]}
+
+	dut_run gpionotify --banner --event=requested "--format=%x" -c $sim0 4
+	dut_flush
+
+	request_release_line $sim0 4
+	dut_read
+	output_is "%x"
+}
Signature · 1 line
2.38.1
#8 lore raw
In reply to Kent Gibson
Quoted 55 lines · Kent Gibson

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.

Quoted 12 lines

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.

Quoted 60 lines

- 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
Quoted 3 lines

--
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

author
#9 lore raw
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

#10 lore raw
Quoted 14 lines · Kent Gibson
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

Quoted 5 lines

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

author
#11 lore raw
Quoted 15 lines · Bartosz Golaszewski
Quoted 14 lines · Kent Gibson
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

Quoted 9 lines
Quoted 5 lines

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.

author
#12 lore raw
In reply to Kent Gibson
Quoted 13 lines · Kent Gibson
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

Quoted 3 lines

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.

#13 lore raw
Quoted 31 lines · Kent Gibson
Quoted 13 lines · Kent Gibson
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

Quoted 3 lines

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.

Quoted 7 lines

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

author
#14 lore raw
Quoted 35 lines · Bartosz Golaszewski
Quoted 34 lines · Kent Gibson
Quoted 13 lines · Kent Gibson
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

Quoted 3 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 11 lines

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

Quoted 8 lines

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.

#15 lore raw
Quoted 45 lines · Kent Gibson
Quoted 35 lines · Bartosz Golaszewski
Quoted 34 lines · Kent Gibson
Quoted 13 lines · Kent Gibson
Quoted 11 lines · Bartosz Golaszewski
Quoted 1 line · Kent Gibson

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.

Quoted 3 lines

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.

Quoted 7 lines
Quoted 4 lines

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.

Quoted 7 lines

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

Quoted 27 lines
Quoted 11 lines

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

Quoted 8 lines

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.