linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option
@ 2023-06-04 14:04 Gabriel Matni
  2023-06-04 14:21 ` Kent Gibson
  0 siblings, 1 reply; 2+ messages in thread
From: Gabriel Matni @ 2023-06-04 14:04 UTC (permalink / raw)
  To: linux-gpio@vger.kernel.org; +Cc: Kent Gibson, Bartosz Golaszewski

From: Gabriel Matni <gabriel.matni@exfo.com>

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 <gabriel.matni@exfo.com>
---
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

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option
  2023-06-04 14:04 [libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option Gabriel Matni
@ 2023-06-04 14:21 ` Kent Gibson
  0 siblings, 0 replies; 2+ messages in thread
From: Kent Gibson @ 2023-06-04 14:21 UTC (permalink / raw)
  To: Gabriel Matni; +Cc: linux-gpio@vger.kernel.org, Bartosz Golaszewski

On Sun, Jun 04, 2023 at 02:04:21PM +0000, Gabriel Matni wrote:
> From: Gabriel Matni <gabriel.matni@exfo.com>
> 
> 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.

> Signed-off-by: Gabriel Matni <gabriel.matni@exfo.com>
> ---
> 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

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

>  		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

>  	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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-06-04 14:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-04 14:04 [libgpiod][PATCH V2] tools: gpiomon/gpionotify: add idle-timeout option Gabriel Matni
2023-06-04 14:21 ` Kent Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).