[libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option

Subsystems: the rest

2 messages, 2 authors, 2023-06-04 · open the first message on its own page

[libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option

From: Gabriel Matni <hidden>
Date: 2023-06-04 14:04:28

From: Gabriel Matni <redacted>

Add an idle timeout option. It allows the tool to gracefully exit upon
expiry.  This option is handy for scripting as it allows the developer to
take an action when no event has been detected for a given period.

Signed-off-by: Gabriel Matni <redacted>
---
V1 -> V2: Addressed the following review comments:
- Renamed timeout option to idle-timeout 
- Timeout value is now signed
- Reused print_period_help() for idle-timeout option
- Added the idle-timeout option to gpionotify for consistency

 tools/gpiomon.c    | 15 ++++++++++++++-
 tools/gpionotify.c | 16 +++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/tools/gpiomon.c b/tools/gpiomon.c
index c2684c2a4dd4..91e602cdbb5e 100644
--- a/tools/gpiomon.c
+++ b/tools/gpiomon.c
@@ -30,6 +30,7 @@ struct config {
 	const char *fmt;
 	enum gpiod_line_clock event_clock;
 	int timestamp_fmt;
+	int timeout;
 };
 
 static void print_help(void)
@@ -57,6 +58,8 @@ static void print_help(void)
 	printf("\t\t\tBy default 'realtime' is formatted as UTC, others as raw u64.\n");
 	printf("  -h, --help\t\tdisplay this help and exit\n");
 	printf("  -F, --format <fmt>\tspecify a custom output format\n");
+	printf("      --idle-timeout <period>\n");
+	printf("\t\t\texit gracefully if no events occurred during the specified period\n");
 	printf("  -l, --active-low\ttreat the line as active low, flipping the sense of\n");
 	printf("\t\t\trising and falling edges\n");
 	printf("      --localtime\tformat event timestamps as local time\n");
@@ -123,6 +126,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		{ "event-clock", required_argument, NULL,	'E' },
 		{ "format",	required_argument, NULL,	'F' },
 		{ "help",	no_argument,	NULL,		'h' },
+		{ "idle-timeout",	required_argument,	NULL,		'i' },
 		{ "localtime",	no_argument,	&cfg->timestamp_fmt,	2 },
 		{ "num-events",	required_argument, NULL,	'n' },
 		{ "quiet",	no_argument,	NULL,		'q' },
@@ -139,6 +143,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 	memset(cfg, 0, sizeof(*cfg));
 	cfg->edges = GPIOD_LINE_EDGE_BOTH;
 	cfg->consumer = "gpiomon";
+	cfg->timeout = -1;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -170,6 +175,9 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		case 'F':
 			cfg->fmt = optarg;
 			break;
+		case 'i':
+			cfg->timeout = parse_period_or_die(optarg) / 1000;
+			break;
 		case 'l':
 			cfg->active_low = true;
 			break;
@@ -443,11 +451,16 @@ int main(int argc, char **argv)
 		print_banner(argc, argv);
 
 	for (;;) {
+		int ret;
 		fflush(stdout);
 
-		if (poll(pollfds, resolver->num_chips, -1) < 0)
+		ret = poll(pollfds, resolver->num_chips, cfg.timeout);
+		if (ret < 0)
 			die_perror("error polling for events");
 
+		if (ret == 0)
+			goto done;
+
 		for (i = 0; i < resolver->num_chips; i++) {
 			if (pollfds[i].revents == 0)
 				continue;
diff --git a/tools/gpionotify.c b/tools/gpionotify.c
index 990ca04519b5..863c5d046542 100644
--- a/tools/gpionotify.c
+++ b/tools/gpionotify.c
@@ -23,6 +23,7 @@ struct config {
 	const char *chip_id;
 	const char *fmt;
 	int timestamp_fmt;
+	int timeout;
 };
 
 static void print_help(void)
@@ -43,6 +44,8 @@ static void print_help(void)
 	printf("\t\t\t(default is all events)\n");
 	printf("  -h, --help\t\tdisplay this help and exit\n");
 	printf("  -F, --format <fmt>\tspecify a custom output format\n");
+	printf("      --idle-timeout <period>\n");
+	printf("\t\t\texit gracefully if no events occurred during the specified period\n");
 	printf("      --localtime\tconvert event timestamps to local time\n");
 	printf("  -n, --num-events <num>\n");
 	printf("\t\t\texit after processing num events\n");
@@ -52,6 +55,7 @@ static void print_help(void)
 	printf("      --utc\t\tconvert event timestamps to UTC\n");
 	printf("  -v, --version\t\toutput version information and exit\n");
 	print_chip_help();
+	print_period_help();
 	printf("\n");
 	printf("Format specifiers:\n");
 	printf("  %%o   GPIO line offset\n");
@@ -89,6 +93,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		{ "event",	required_argument, NULL,	'e' },
 		{ "format",	required_argument, NULL,	'F' },
 		{ "help",	no_argument,	NULL,		'h' },
+		{ "idle-timeout",	required_argument,	NULL,		'i' },
 		{ "localtime",	no_argument,	&cfg->timestamp_fmt, 2 },
 		{ "num-events",	required_argument, NULL,	'n' },
 		{ "quiet",	no_argument,	NULL,		'q' },
@@ -103,6 +108,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 	int opti, optc;
 
 	memset(cfg, 0, sizeof(*cfg));
+	cfg->timeout = -1;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -125,6 +131,9 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		case 'F':
 			cfg->fmt = optarg;
 			break;
+		case 'i':
+			cfg->timeout = parse_period_or_die(optarg) / 1000;
+			break;
 		case 'n':
 			cfg->events_wanted = parse_uint_or_die(optarg);
 			break;
@@ -411,11 +420,16 @@ int main(int argc, char **argv)
 		print_banner(argc, argv);
 
 	for (;;) {
+		int ret;
 		fflush(stdout);
 
-		if (poll(pollfds, resolver->num_chips, -1) < 0)
+		ret = poll(pollfds, resolver->num_chips, cfg.timeout);
+		if (ret < 0)
 			die_perror("error polling for events");
 
+		if (ret == 0)
+			goto done;
+
 		for (i = 0; i < resolver->num_chips; i++) {
 			if (pollfds[i].revents == 0)
 				continue;
base-commit: b10c5b769978412e315507ae07fa554b09ca189f
-- 
2.25.1

Re: [libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option

From: Kent Gibson <warthog618@gmail.com>
Date: 2023-06-04 14:21:38

On Sun, Jun 04, 2023 at 02:04:21PM +0000, Gabriel Matni wrote:
From: Gabriel Matni <redacted>

Add an idle timeout option. It allows the tool to gracefully exit upon
expiry.  This option is handy for scripting as it allows the developer to
take an action when no event has been detected for a given period.
"exit upon expiry" is vague - one of the reasons I wanted the option
renamed --idle-timeout.

So maybe just

"Add an idle timeout option to gpiomon and gpionotify to exit gracefully
when no event has been detected for a given period."

A few other minor nits below.

Otherwise looks good to me.

Cheers,
Kent.
quoted hunk
Signed-off-by: Gabriel Matni <redacted>
---
V1 -> V2: Addressed the following review comments:
- Renamed timeout option to idle-timeout 
- Timeout value is now signed
- Reused print_period_help() for idle-timeout option
- Added the idle-timeout option to gpionotify for consistency

 tools/gpiomon.c    | 15 ++++++++++++++-
 tools/gpionotify.c | 16 +++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/tools/gpiomon.c b/tools/gpiomon.c
index c2684c2a4dd4..91e602cdbb5e 100644
--- a/tools/gpiomon.c
+++ b/tools/gpiomon.c
@@ -30,6 +30,7 @@ struct config {
 	const char *fmt;
 	enum gpiod_line_clock event_clock;
 	int timestamp_fmt;
+	int timeout;
 };
 
 static void print_help(void)
@@ -57,6 +58,8 @@ static void print_help(void)
 	printf("\t\t\tBy default 'realtime' is formatted as UTC, others as raw u64.\n");
 	printf("  -h, --help\t\tdisplay this help and exit\n");
 	printf("  -F, --format <fmt>\tspecify a custom output format\n");
+	printf("      --idle-timeout <period>\n");
+	printf("\t\t\texit gracefully if no events occurred during the specified period\n");
if no events occur for the period specified
quoted hunk
 	printf("  -l, --active-low\ttreat the line as active low, flipping the sense of\n");
 	printf("\t\t\trising and falling edges\n");
 	printf("      --localtime\tformat event timestamps as local time\n");
@@ -123,6 +126,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		{ "event-clock", required_argument, NULL,	'E' },
 		{ "format",	required_argument, NULL,	'F' },
 		{ "help",	no_argument,	NULL,		'h' },
+		{ "idle-timeout",	required_argument,	NULL,		'i' },
 		{ "localtime",	no_argument,	&cfg->timestamp_fmt,	2 },
 		{ "num-events",	required_argument, NULL,	'n' },
 		{ "quiet",	no_argument,	NULL,		'q' },
@@ -139,6 +143,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 	memset(cfg, 0, sizeof(*cfg));
 	cfg->edges = GPIOD_LINE_EDGE_BOTH;
 	cfg->consumer = "gpiomon";
+	cfg->timeout = -1;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -170,6 +175,9 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		case 'F':
 			cfg->fmt = optarg;
 			break;
+		case 'i':
+			cfg->timeout = parse_period_or_die(optarg) / 1000;
+			break;
 		case 'l':
 			cfg->active_low = true;
 			break;
@@ -443,11 +451,16 @@ int main(int argc, char **argv)
 		print_banner(argc, argv);
 
 	for (;;) {
+		int ret;
Declare at the top of the function.
Hang on - there is one there already for gpiomon, so you don't need this at all.
quoted hunk
 		fflush(stdout);
 
-		if (poll(pollfds, resolver->num_chips, -1) < 0)
+		ret = poll(pollfds, resolver->num_chips, cfg.timeout);
+		if (ret < 0)
 			die_perror("error polling for events");
 
+		if (ret == 0)
+			goto done;
+
 		for (i = 0; i < resolver->num_chips; i++) {
 			if (pollfds[i].revents == 0)
 				continue;
diff --git a/tools/gpionotify.c b/tools/gpionotify.c
index 990ca04519b5..863c5d046542 100644
--- a/tools/gpionotify.c
+++ b/tools/gpionotify.c
@@ -23,6 +23,7 @@ struct config {
 	const char *chip_id;
 	const char *fmt;
 	int timestamp_fmt;
+	int timeout;
 };
 
 static void print_help(void)
@@ -43,6 +44,8 @@ static void print_help(void)
 	printf("\t\t\t(default is all events)\n");
 	printf("  -h, --help\t\tdisplay this help and exit\n");
 	printf("  -F, --format <fmt>\tspecify a custom output format\n");
+	printf("      --idle-timeout <period>\n");
+	printf("\t\t\texit gracefully if no events occurred during the specified period\n");
as per gpiomon
quoted hunk
 	printf("      --localtime\tconvert event timestamps to local time\n");
 	printf("  -n, --num-events <num>\n");
 	printf("\t\t\texit after processing num events\n");
@@ -52,6 +55,7 @@ static void print_help(void)
 	printf("      --utc\t\tconvert event timestamps to UTC\n");
 	printf("  -v, --version\t\toutput version information and exit\n");
 	print_chip_help();
+	print_period_help();
 	printf("\n");
 	printf("Format specifiers:\n");
 	printf("  %%o   GPIO line offset\n");
@@ -89,6 +93,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		{ "event",	required_argument, NULL,	'e' },
 		{ "format",	required_argument, NULL,	'F' },
 		{ "help",	no_argument,	NULL,		'h' },
+		{ "idle-timeout",	required_argument,	NULL,		'i' },
 		{ "localtime",	no_argument,	&cfg->timestamp_fmt, 2 },
 		{ "num-events",	required_argument, NULL,	'n' },
 		{ "quiet",	no_argument,	NULL,		'q' },
@@ -103,6 +108,7 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 	int opti, optc;
 
 	memset(cfg, 0, sizeof(*cfg));
+	cfg->timeout = -1;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -125,6 +131,9 @@ static int parse_config(int argc, char **argv, struct config *cfg)
 		case 'F':
 			cfg->fmt = optarg;
 			break;
+		case 'i':
+			cfg->timeout = parse_period_or_die(optarg) / 1000;
+			break;
 		case 'n':
 			cfg->events_wanted = parse_uint_or_die(optarg);
 			break;
@@ -411,11 +420,16 @@ int main(int argc, char **argv)
 		print_banner(argc, argv);
 
 	for (;;) {
+		int ret;
Declare at the top of the function.
 		fflush(stdout);
 
-		if (poll(pollfds, resolver->num_chips, -1) < 0)
+		ret = poll(pollfds, resolver->num_chips, cfg.timeout);
+		if (ret < 0)
 			die_perror("error polling for events");
 
+		if (ret == 0)
+			goto done;
+
 		for (i = 0; i < resolver->num_chips; i++) {
 			if (pollfds[i].revents == 0)
 				continue;

base-commit: b10c5b769978412e315507ae07fa554b09ca189f
-- 
2.25.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help