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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, Jun 12, 2023 at 6:08 PM Kent Gibson <warthog618@xxxxxxxxx> 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@xxxxxxxxx> wrote:
> > >
> > > From: Gabriel Matni <gabriel.matni@xxxxxxxx>
> > >
> > > 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@xxxxxxxx>
> > > ---
> > > 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