public inbox for linux-gpio@vger.kernel.org
 help / color / mirror / Atom feed
* [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option
@ 2023-06-12  1:30 Gabriel Matni
  2023-06-12  2:03 ` Kent Gibson
  2023-06-12 16:05 ` Bartosz Golaszewski
  0 siblings, 2 replies; 5+ messages in thread
From: Gabriel Matni @ 2023-06-12  1:30 UTC (permalink / raw)
  To: linux-gpio; +Cc: warthog618, brgl, gabriel.matni, gabriel.matni

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

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

Signed-off-by: Gabriel Matni <gabriel.matni@exfo.com>
---
V3 -> V4: Submitting patch with git send-email 

V2 -> V3: Addressed the following review comments:
- Simplify the commit message
- Clarify the help description of the --idle-timeout option
- Declare ret variable at the top of function for gpionotify, not required for gpiomon

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    | 14 +++++++++++++-
 tools/gpionotify.c | 17 +++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/tools/gpiomon.c b/tools/gpiomon.c
index c2684c2..e3abb2d 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 occur for the period specified\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;
@@ -445,9 +453,13 @@ int main(int argc, char **argv)
 	for (;;) {
 		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 990ca04..2c56590 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 occur for the period specified\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;
@@ -362,7 +371,7 @@ static void event_print(struct gpiod_info_event *event,
 
 int main(int argc, char **argv)
 {
-	int i, j, events_done = 0, evtype;
+	int i, j, ret, events_done = 0, evtype;
 	struct line_resolver *resolver;
 	struct gpiod_info_event *event;
 	struct gpiod_chip **chips;
@@ -413,9 +422,13 @@ int main(int argc, char **argv)
 	for (;;) {
 		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: 9788bdd3d6791205431e65366dcc518446f7ca6a
-- 
2.25.1


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

* Re: [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option
  2023-06-12  1:30 [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option Gabriel Matni
@ 2023-06-12  2:03 ` Kent Gibson
  2023-06-12 16:05 ` Bartosz Golaszewski
  1 sibling, 0 replies; 5+ messages in thread
From: Kent Gibson @ 2023-06-12  2:03 UTC (permalink / raw)
  To: Gabriel Matni; +Cc: linux-gpio, brgl, gabriel.matni

On Sun, Jun 11, 2023 at 09:30:49PM -0400, Gabriel Matni wrote:
> From: Gabriel Matni <gabriel.matni@exfo.com>
> 
> Add an idle timeout option to gpiomon and gpionotify to exit gracefully 
> when no event has been detected for a given period.
> 
> Signed-off-by: Gabriel Matni <gabriel.matni@exfo.com>

That works for me.  Will post some tests that cover this shortly.

Reviewed-by: Kent Gibson <warthog618@gmail.com>
Tested-by: Kent Gibson <warthog618@gmail.com>

Cheers,
Kent.

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

* Re: [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option
  2023-06-12  1:30 [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option Gabriel Matni
  2023-06-12  2:03 ` Kent Gibson
@ 2023-06-12 16:05 ` Bartosz Golaszewski
  2023-06-12 16:08   ` Kent Gibson
  1 sibling, 1 reply; 5+ messages in thread
From: Bartosz Golaszewski @ 2023-06-12 16:05 UTC (permalink / raw)
  To: Gabriel Matni; +Cc: linux-gpio, warthog618, gabriel.matni

On Mon, Jun 12, 2023 at 3:31 AM Gabriel Matni <gabriel.matni@gmail.com> wrote:
>
> From: Gabriel Matni <gabriel.matni@exfo.com>
>
> Add an idle timeout option to gpiomon and gpionotify to exit gracefully
> when no event has been detected for a given period.
>
> Signed-off-by: Gabriel Matni <gabriel.matni@exfo.com>
> ---
> V3 -> V4: Submitting patch with git send-email
>
> V2 -> V3: Addressed the following review comments:
> - Simplify the commit message
> - Clarify the help description of the --idle-timeout option
> - Declare ret variable at the top of function for gpionotify, not required for gpiomon
>
> 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    | 14 +++++++++++++-
>  tools/gpionotify.c | 17 +++++++++++++++--
>  2 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/tools/gpiomon.c b/tools/gpiomon.c
> index c2684c2..e3abb2d 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 occur for the period specified\n");

I think it would be useful to state the time units used by this option here.

Bart

>         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;
> @@ -445,9 +453,13 @@ int main(int argc, char **argv)
>         for (;;) {
>                 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 990ca04..2c56590 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 occur for the period specified\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;
> @@ -362,7 +371,7 @@ static void event_print(struct gpiod_info_event *event,
>
>  int main(int argc, char **argv)
>  {
> -       int i, j, events_done = 0, evtype;
> +       int i, j, ret, events_done = 0, evtype;
>         struct line_resolver *resolver;
>         struct gpiod_info_event *event;
>         struct gpiod_chip **chips;
> @@ -413,9 +422,13 @@ int main(int argc, char **argv)
>         for (;;) {
>                 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: 9788bdd3d6791205431e65366dcc518446f7ca6a
> --
> 2.25.1
>

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

* Re: [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option
  2023-06-12 16:05 ` Bartosz Golaszewski
@ 2023-06-12 16:08   ` Kent Gibson
  2023-06-12 16:30     ` Bartosz Golaszewski
  0 siblings, 1 reply; 5+ messages in thread
From: Kent Gibson @ 2023-06-12 16:08 UTC (permalink / raw)
  To: Bartosz Golaszewski; +Cc: Gabriel Matni, linux-gpio, gabriel.matni

On Mon, Jun 12, 2023 at 06:05:11PM +0200, Bartosz Golaszewski wrote:
> On Mon, Jun 12, 2023 at 3:31 AM Gabriel Matni <gabriel.matni@gmail.com> wrote:
> >
> > From: Gabriel Matni <gabriel.matni@exfo.com>
> >
> > Add an idle timeout option to gpiomon and gpionotify to exit gracefully
> > when no event has been detected for a given period.
> >
> > Signed-off-by: Gabriel Matni <gabriel.matni@exfo.com>
> > ---
> > V3 -> V4: Submitting patch with git send-email
> >
> > V2 -> V3: Addressed the following review comments:
> > - Simplify the commit message
> > - Clarify the help description of the --idle-timeout option
> > - Declare ret variable at the top of function for gpionotify, not required for gpiomon
> >
> > 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    | 14 +++++++++++++-
> >  tools/gpionotify.c | 17 +++++++++++++++--
> >  2 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/gpiomon.c b/tools/gpiomon.c
> > index c2684c2..e3abb2d 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 occur for the period specified\n");
> 
> I think it would be useful to state the time units used by this option here.
> 

That is already covered by the period help:

Periods:
    Periods are taken as milliseconds unless units are specified. e.g. 10us.
    Supported units are 's', 'ms', and 'us'.

Cheers,
Kent.

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

* Re: [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option
  2023-06-12 16:08   ` Kent Gibson
@ 2023-06-12 16:30     ` Bartosz Golaszewski
  0 siblings, 0 replies; 5+ messages in thread
From: Bartosz Golaszewski @ 2023-06-12 16:30 UTC (permalink / raw)
  To: Kent Gibson; +Cc: Gabriel Matni, linux-gpio, gabriel.matni

On Mon, Jun 12, 2023 at 6:08 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 06:05:11PM +0200, Bartosz Golaszewski wrote:
> > On Mon, Jun 12, 2023 at 3:31 AM Gabriel Matni <gabriel.matni@gmail.com> wrote:
> > >
> > > From: Gabriel Matni <gabriel.matni@exfo.com>
> > >
> > > Add an idle timeout option to gpiomon and gpionotify to exit gracefully
> > > when no event has been detected for a given period.
> > >
> > > Signed-off-by: Gabriel Matni <gabriel.matni@exfo.com>
> > > ---
> > > V3 -> V4: Submitting patch with git send-email
> > >
> > > V2 -> V3: Addressed the following review comments:
> > > - Simplify the commit message
> > > - Clarify the help description of the --idle-timeout option
> > > - Declare ret variable at the top of function for gpionotify, not required for gpiomon
> > >
> > > 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    | 14 +++++++++++++-
> > >  tools/gpionotify.c | 17 +++++++++++++++--
> > >  2 files changed, 28 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/gpiomon.c b/tools/gpiomon.c
> > > index c2684c2..e3abb2d 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 occur for the period specified\n");
> >
> > I think it would be useful to state the time units used by this option here.
> >
>
> That is already covered by the period help:
>
> Periods:
>     Periods are taken as milliseconds unless units are specified. e.g. 10us.
>     Supported units are 's', 'ms', and 'us'.
>

Ah, I missed the print_period_help() call here. When actually running
gpiomon --help it's clearer.

Thanks, I'll apply it now.

Bart

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

end of thread, other threads:[~2023-06-12 16:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-12  1:30 [libgpiod][PATCH V4 1/1] tools: gpiomon/gpionotify: add idle-timeout option Gabriel Matni
2023-06-12  2:03 ` Kent Gibson
2023-06-12 16:05 ` Bartosz Golaszewski
2023-06-12 16:08   ` Kent Gibson
2023-06-12 16:30     ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox