| Message ID | 20240409093333.138408-3-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | gpio-tools: allow specifying longer time periods | expand |
On Tue, Apr 09, 2024 at 11:33:33AM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We currently store time as milliseconds in 32-bit integers and allow > seconds as the longest time unit when parsing command-line arguments > limiting the time period possible to specify when passing arguments such > as --hold-period to 35 minutes. Let's use 64-bit integers to vastly > increase that. > I don't think all timers should be extended, only where it makes sense to do so, so gpioset (toggle and hold periods). And maybe gpiomon (idle timeout), though you haven't extended that one, cos poll()? Maybe switch that to ppoll()? More on this below. > Use nanosleep() instead of usleep() to extend the possible sleep time > range. > > Reported-by: Gunnar Thörnqvist <gunnar@igl.se> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > configure.ac | 2 ++ > tools/gpioget.c | 4 ++-- > tools/gpiomon.c | 19 ++++++++++++++----- > tools/gpioset.c | 16 ++++++++-------- > tools/tools-common.c | 22 ++++++++++++++++------ > tools/tools-common.h | 5 +++-- > 6 files changed, 45 insertions(+), 23 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 3b5bbf2..a2370c5 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -120,6 +120,8 @@ AS_IF([test "x$with_tools" = xtrue], > AC_CHECK_FUNC([asprintf], [], [FUNC_NOT_FOUND_TOOLS([asprintf])]) > AC_CHECK_FUNC([scandir], [], [FUNC_NOT_FOUND_TOOLS([scandir])]) > AC_CHECK_FUNC([versionsort], [], [FUNC_NOT_FOUND_TOOLS([versionsort])]) > + AC_CHECK_FUNC([strtoull], [], [FUNC_NOT_FOUND_TOOLS([strtoull])]) > + AC_CHECK_FUNC([nanosleep], [], [FUNC_NOT_FOUND_TOOLS([nanosleep])]) > AS_IF([test "x$with_gpioset_interactive" = xtrue], > [PKG_CHECK_MODULES([LIBEDIT], [libedit >= 3.1])]) > ]) > diff --git a/tools/gpioget.c b/tools/gpioget.c > index f611737..bad7667 100644 > --- a/tools/gpioget.c > +++ b/tools/gpioget.c > @@ -19,7 +19,7 @@ struct config { > bool unquoted; > enum gpiod_line_bias bias; > enum gpiod_line_direction direction; > - unsigned int hold_period_us; > + unsigned long long hold_period_us; > const char *chip_id; > const char *consumer; > }; > @@ -205,7 +205,7 @@ int main(int argc, char **argv) > die_perror("unable to request lines"); > > if (cfg.hold_period_us) > - usleep(cfg.hold_period_us); > + sleep_us(cfg.hold_period_us); Got a use case where a hold period is measured in more than seconds? Specifically for a get. > > ret = gpiod_line_request_get_values(request, values); > if (ret) > diff --git a/tools/gpiomon.c b/tools/gpiomon.c > index e3abb2d..a8a3302 100644 > --- a/tools/gpiomon.c > +++ b/tools/gpiomon.c > @@ -5,6 +5,7 @@ > #include <getopt.h> > #include <gpiod.h> > #include <inttypes.h> > +#include <limits.h> > #include <poll.h> > #include <stdio.h> > #include <stdlib.h> > @@ -24,13 +25,13 @@ struct config { > enum gpiod_line_bias bias; > enum gpiod_line_edge edges; > int events_wanted; > - unsigned int debounce_period_us; > + unsigned long long debounce_period_us; > const char *chip_id; > const char *consumer; > const char *fmt; > enum gpiod_line_clock event_clock; > int timestamp_fmt; > - int timeout; > + long long timeout; Can we rename this to idle_timeout? A variable named "timeout" is lacking context. > }; > > static void print_help(void) > @@ -389,9 +390,17 @@ int main(int argc, char **argv) > if (cfg.active_low) > gpiod_line_settings_set_active_low(settings, true); > > - if (cfg.debounce_period_us) > + if (cfg.debounce_period_us) { > + if (cfg.debounce_period_us > UINT_MAX) > + die("invalid debounce period: %llu", > + cfg.debounce_period_us); > + > gpiod_line_settings_set_debounce_period_us( > - settings, cfg.debounce_period_us); > + settings, (unsigned long)cfg.debounce_period_us); > + } > + > + if (cfg.timeout > INT_MAX) > + die("invalid idle timeout: %llu", cfg.timeout); > Not a fan of parsing to long, only to do a smaller range check here. How about providing two parsers - one for int sized periods and one for long periods, e.g. parse_long_period(). Cheers, Kent.
On Tue, Apr 9, 2024 at 2:56 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Tue, Apr 09, 2024 at 11:33:33AM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > We currently store time as milliseconds in 32-bit integers and allow > > seconds as the longest time unit when parsing command-line arguments > > limiting the time period possible to specify when passing arguments such > > as --hold-period to 35 minutes. Let's use 64-bit integers to vastly > > increase that. > > > > I don't think all timers should be extended, only where it > makes sense to do so, so gpioset (toggle and hold periods). > And maybe gpiomon (idle timeout), though you haven't extended that one, > cos poll()? Maybe switch that to ppoll()? > > More on this below. Makes sense. > > > Use nanosleep() instead of usleep() to extend the possible sleep time > > range. > > > > Reported-by: Gunnar Thörnqvist <gunnar@igl.se> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > --- > > configure.ac | 2 ++ > > tools/gpioget.c | 4 ++-- > > tools/gpiomon.c | 19 ++++++++++++++----- > > tools/gpioset.c | 16 ++++++++-------- > > tools/tools-common.c | 22 ++++++++++++++++------ > > tools/tools-common.h | 5 +++-- > > 6 files changed, 45 insertions(+), 23 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index 3b5bbf2..a2370c5 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -120,6 +120,8 @@ AS_IF([test "x$with_tools" = xtrue], > > AC_CHECK_FUNC([asprintf], [], [FUNC_NOT_FOUND_TOOLS([asprintf])]) > > AC_CHECK_FUNC([scandir], [], [FUNC_NOT_FOUND_TOOLS([scandir])]) > > AC_CHECK_FUNC([versionsort], [], [FUNC_NOT_FOUND_TOOLS([versionsort])]) > > + AC_CHECK_FUNC([strtoull], [], [FUNC_NOT_FOUND_TOOLS([strtoull])]) > > + AC_CHECK_FUNC([nanosleep], [], [FUNC_NOT_FOUND_TOOLS([nanosleep])]) > > AS_IF([test "x$with_gpioset_interactive" = xtrue], > > [PKG_CHECK_MODULES([LIBEDIT], [libedit >= 3.1])]) > > ]) > > diff --git a/tools/gpioget.c b/tools/gpioget.c > > index f611737..bad7667 100644 > > --- a/tools/gpioget.c > > +++ b/tools/gpioget.c > > @@ -19,7 +19,7 @@ struct config { > > bool unquoted; > > enum gpiod_line_bias bias; > > enum gpiod_line_direction direction; > > - unsigned int hold_period_us; > > + unsigned long long hold_period_us; > > const char *chip_id; > > const char *consumer; > > }; > > @@ -205,7 +205,7 @@ int main(int argc, char **argv) > > die_perror("unable to request lines"); > > > > if (cfg.hold_period_us) > > - usleep(cfg.hold_period_us); > > + sleep_us(cfg.hold_period_us); > > Got a use case where a hold period is measured in more than seconds? > Specifically for a get. > Yeah, like Gunnar responded, he needs to hold the line for an hour. I think it makes sense. > > > > ret = gpiod_line_request_get_values(request, values); > > if (ret) > > diff --git a/tools/gpiomon.c b/tools/gpiomon.c > > index e3abb2d..a8a3302 100644 > > --- a/tools/gpiomon.c > > +++ b/tools/gpiomon.c > > @@ -5,6 +5,7 @@ > > #include <getopt.h> > > #include <gpiod.h> > > #include <inttypes.h> > > +#include <limits.h> > > #include <poll.h> > > #include <stdio.h> > > #include <stdlib.h> > > @@ -24,13 +25,13 @@ struct config { > > enum gpiod_line_bias bias; > > enum gpiod_line_edge edges; > > int events_wanted; > > - unsigned int debounce_period_us; > > + unsigned long long debounce_period_us; > > const char *chip_id; > > const char *consumer; > > const char *fmt; > > enum gpiod_line_clock event_clock; > > int timestamp_fmt; > > - int timeout; > > + long long timeout; > > Can we rename this to idle_timeout? A variable named "timeout" is > lacking context. > Sure but it's a different patch. Also: it's your code, just send me the patch. :) > > }; > > > > static void print_help(void) > > @@ -389,9 +390,17 @@ int main(int argc, char **argv) > > if (cfg.active_low) > > gpiod_line_settings_set_active_low(settings, true); > > > > - if (cfg.debounce_period_us) > > + if (cfg.debounce_period_us) { > > + if (cfg.debounce_period_us > UINT_MAX) > > + die("invalid debounce period: %llu", > > + cfg.debounce_period_us); > > + > > gpiod_line_settings_set_debounce_period_us( > > - settings, cfg.debounce_period_us); > > + settings, (unsigned long)cfg.debounce_period_us); > > + } > > + > > + if (cfg.timeout > INT_MAX) > > + die("invalid idle timeout: %llu", cfg.timeout); > > > > Not a fan of parsing to long, only to do a smaller range check here. > How about providing two parsers - one for int sized periods and > one for long periods, e.g. parse_long_period(). I actually prefer to parse the larger range and then limit the max size. I would be fine with adding a limit argument to parse_period() like long long parse_period(const char *option, long long limit); Bartosz > > Cheers, > Kent.
On Tue, Apr 09, 2024 at 04:58:52PM +0200, Gunnar Thörnqvist wrote: > Hi, Got a use case where a hold period is measured in more than seconds? > Specifically for a get.::: > > I can see a large number of use cases where the time can be hours, days and > weeks. In my case, pin 17 controls a relay that heats water when electricity > is cheapest. It is ok to only have seconds as unit but the range must be > larger. /Gunnar > I was asking specifically about the case for gpioget, where a long hold period makes absolutely no sense. Cheers, Kent.
On Tue, Apr 9, 2024 at 6:05 PM Kent Gibson <warthog618@gmail.com> wrote: > > On Tue, Apr 09, 2024 at 04:58:52PM +0200, Gunnar Thörnqvist wrote: > > Hi, Got a use case where a hold period is measured in more than seconds? > > Specifically for a get.::: > > > > I can see a large number of use cases where the time can be hours, days and > > weeks. In my case, pin 17 controls a relay that heats water when electricity > > is cheapest. It is ok to only have seconds as unit but the range must be > > larger. /Gunnar > > > > I was asking specifically about the case for gpioget, where a long hold > period makes absolutely no sense. > One could argue that this option doesn't make sense at all for gpioget. :) I don't think it hurts to support a longer period of time even if only for code reuse and less surface for bugs. Bart
On Tue, Apr 09, 2024 at 05:59:59PM +0200, Bartosz Golaszewski wrote: > On Tue, Apr 9, 2024 at 2:56 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Tue, Apr 09, 2024 at 11:33:33AM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > We currently store time as milliseconds in 32-bit integers and allow > > > seconds as the longest time unit when parsing command-line arguments > > > limiting the time period possible to specify when passing arguments such > > > as --hold-period to 35 minutes. Let's use 64-bit integers to vastly > > > increase that. > > > > > > > I don't think all timers should be extended, only where it > > makes sense to do so, so gpioset (toggle and hold periods). > > And maybe gpiomon (idle timeout), though you haven't extended that one, > > cos poll()? Maybe switch that to ppoll()? > > > > More on this below. > > Makes sense. > > > > > > Use nanosleep() instead of usleep() to extend the possible sleep time > > > range. > > > > > > Reported-by: Gunnar Thörnqvist <gunnar@igl.se> > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > --- > > > configure.ac | 2 ++ > > > tools/gpioget.c | 4 ++-- > > > tools/gpiomon.c | 19 ++++++++++++++----- > > > tools/gpioset.c | 16 ++++++++-------- > > > tools/tools-common.c | 22 ++++++++++++++++------ > > > tools/tools-common.h | 5 +++-- > > > 6 files changed, 45 insertions(+), 23 deletions(-) > > > > > > diff --git a/configure.ac b/configure.ac > > > index 3b5bbf2..a2370c5 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -120,6 +120,8 @@ AS_IF([test "x$with_tools" = xtrue], > > > AC_CHECK_FUNC([asprintf], [], [FUNC_NOT_FOUND_TOOLS([asprintf])]) > > > AC_CHECK_FUNC([scandir], [], [FUNC_NOT_FOUND_TOOLS([scandir])]) > > > AC_CHECK_FUNC([versionsort], [], [FUNC_NOT_FOUND_TOOLS([versionsort])]) > > > + AC_CHECK_FUNC([strtoull], [], [FUNC_NOT_FOUND_TOOLS([strtoull])]) > > > + AC_CHECK_FUNC([nanosleep], [], [FUNC_NOT_FOUND_TOOLS([nanosleep])]) > > > AS_IF([test "x$with_gpioset_interactive" = xtrue], > > > [PKG_CHECK_MODULES([LIBEDIT], [libedit >= 3.1])]) > > > ]) > > > diff --git a/tools/gpioget.c b/tools/gpioget.c > > > index f611737..bad7667 100644 > > > --- a/tools/gpioget.c > > > +++ b/tools/gpioget.c > > > @@ -19,7 +19,7 @@ struct config { > > > bool unquoted; > > > enum gpiod_line_bias bias; > > > enum gpiod_line_direction direction; > > > - unsigned int hold_period_us; > > > + unsigned long long hold_period_us; > > > const char *chip_id; > > > const char *consumer; > > > }; > > > @@ -205,7 +205,7 @@ int main(int argc, char **argv) > > > die_perror("unable to request lines"); > > > > > > if (cfg.hold_period_us) > > > - usleep(cfg.hold_period_us); > > > + sleep_us(cfg.hold_period_us); > > > > Got a use case where a hold period is measured in more than seconds? > > Specifically for a get. > > > > Yeah, like Gunnar responded, he needs to hold the line for an hour. I > think it makes sense. > And as I noted, I was interested in the get, the point being is a long period always necessary and appropriate? > > > > > > ret = gpiod_line_request_get_values(request, values); > > > if (ret) > > > diff --git a/tools/gpiomon.c b/tools/gpiomon.c > > > index e3abb2d..a8a3302 100644 > > > --- a/tools/gpiomon.c > > > +++ b/tools/gpiomon.c > > > @@ -5,6 +5,7 @@ > > > #include <getopt.h> > > > #include <gpiod.h> > > > #include <inttypes.h> > > > +#include <limits.h> > > > #include <poll.h> > > > #include <stdio.h> > > > #include <stdlib.h> > > > @@ -24,13 +25,13 @@ struct config { > > > enum gpiod_line_bias bias; > > > enum gpiod_line_edge edges; > > > int events_wanted; > > > - unsigned int debounce_period_us; > > > + unsigned long long debounce_period_us; > > > const char *chip_id; > > > const char *consumer; > > > const char *fmt; > > > enum gpiod_line_clock event_clock; > > > int timestamp_fmt; > > > - int timeout; > > > + long long timeout; > > > > Can we rename this to idle_timeout? A variable named "timeout" is > > lacking context. > > > > Sure but it's a different patch. Also: it's your code, just send me > the patch. :) > Check the blame - NOT my code. > > > }; > > > > > > static void print_help(void) > > > @@ -389,9 +390,17 @@ int main(int argc, char **argv) > > > if (cfg.active_low) > > > gpiod_line_settings_set_active_low(settings, true); > > > > > > - if (cfg.debounce_period_us) > > > + if (cfg.debounce_period_us) { > > > + if (cfg.debounce_period_us > UINT_MAX) > > > + die("invalid debounce period: %llu", > > > + cfg.debounce_period_us); > > > + > > > gpiod_line_settings_set_debounce_period_us( > > > - settings, cfg.debounce_period_us); > > > + settings, (unsigned long)cfg.debounce_period_us); > > > + } > > > + > > > + if (cfg.timeout > INT_MAX) > > > + die("invalid idle timeout: %llu", cfg.timeout); > > > > > > > Not a fan of parsing to long, only to do a smaller range check here. > > How about providing two parsers - one for int sized periods and > > one for long periods, e.g. parse_long_period(). > > I actually prefer to parse the larger range and then limit the max > size. I would be fine with adding a limit argument to parse_period() > like long long parse_period(const char *option, long long limit); > I can live with that. Cheers, Kent.
On Tue, Apr 09, 2024 at 07:24:43PM +0200, Bartosz Golaszewski wrote: > On Tue, Apr 9, 2024 at 6:05 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Tue, Apr 09, 2024 at 04:58:52PM +0200, Gunnar Thörnqvist wrote: > > > Hi, Got a use case where a hold period is measured in more than seconds? > > > Specifically for a get.::: > > > > > > I can see a large number of use cases where the time can be hours, days and > > > weeks. In my case, pin 17 controls a relay that heats water when electricity > > > is cheapest. It is ok to only have seconds as unit but the range must be > > > larger. /Gunnar > > > > > > > I was asking specifically about the case for gpioget, where a long hold > > period makes absolutely no sense. > > > > One could argue that this option doesn't make sense at all for gpioget. :) > And one would be wrong. The point of the hold period for gets is to allow the line to settle after a config change before the get itself is performed. > I don't think it hurts to support a longer period of time even if only > for code reuse and less surface for bugs. > Well that is a complicated bit of code. Cheers, Kent.
On Tue, Apr 09, 2024 at 04:58:52PM +0200, Gunnar Thörnqvist wrote: > Hi, Got a use case where a hold period is measured in more than seconds? > Specifically for a get.::: > > I can see a large number of use cases where the time can be hours, days and > weeks. In my case, pin 17 controls a relay that heats water when electricity > is cheapest. It is ok to only have seconds as unit but the range must be > larger. /Gunnar > Also, releasing the line after an elapsed time, as -p does, is rather limiting. This case is actually better suited to Bart's upcoming daemon, where you could have some other entity, say cron or an automation script, command the daemon to change the relay state given certain conditions, including the wall clock time. If you can't wait for Bart's daemon, you can throw together a simple one using the interactive mode of gpioset. e.g. this script: #!/bin/bash pipe=/tmp/relayd mkfifo $pipe trap "rm -f $pipe" EXIT # as bash will block until something is written to the pipe... echo "" > $pipe & gpioset -i GPIO23=0 < $pipe > /dev/null will allow you to send commands to /tmp/relayd to control the state of your relay. e.g. echo toggle > /tmp/relayd or echo "set GPIO23=1" > /tmp/relayd Cheers, Kent.
On Wed, Apr 10, 2024 at 1:37 AM Kent Gibson <warthog618@gmail.com> wrote: > > On Tue, Apr 09, 2024 at 07:24:43PM +0200, Bartosz Golaszewski wrote: > > On Tue, Apr 9, 2024 at 6:05 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > On Tue, Apr 09, 2024 at 04:58:52PM +0200, Gunnar Thörnqvist wrote: > > > > Hi, Got a use case where a hold period is measured in more than seconds? > > > > Specifically for a get.::: > > > > > > > > I can see a large number of use cases where the time can be hours, days and > > > > weeks. In my case, pin 17 controls a relay that heats water when electricity > > > > is cheapest. It is ok to only have seconds as unit but the range must be > > > > larger. /Gunnar > > > > > > > > > > I was asking specifically about the case for gpioget, where a long hold > > > period makes absolutely no sense. > > > > > > > One could argue that this option doesn't make sense at all for gpioget. :) > > > > And one would be wrong. The point of the hold period for gets is to > allow the line to settle after a config change before the get itself is > performed. > One is indeed wrong. > > I don't think it hurts to support a longer period of time even if only > > for code reuse and less surface for bugs. > > > > Well that is a complicated bit of code. > I'll submit the daemon RFC tomorrow or on Friday. Maybe this change isn't even needed after all. Bart > Cheers, > Kent.
On Wed, Apr 10, 2024 at 09:53:49AM +0200, Bartosz Golaszewski wrote: > On Wed, Apr 10, 2024 at 1:37 AM Kent Gibson <warthog618@gmail.com> wrote: > > > > On Tue, Apr 09, 2024 at 07:24:43PM +0200, Bartosz Golaszewski wrote: > > > On Tue, Apr 9, 2024 at 6:05 PM Kent Gibson <warthog618@gmail.com> wrote: > > > > > > > > On Tue, Apr 09, 2024 at 04:58:52PM +0200, Gunnar Thörnqvist wrote: > > > > > Hi, Got a use case where a hold period is measured in more than seconds? > > > > > Specifically for a get.::: > > > > > > > > > > I can see a large number of use cases where the time can be hours, days and > > > > > weeks. In my case, pin 17 controls a relay that heats water when electricity > > > > > is cheapest. It is ok to only have seconds as unit but the range must be > > > > > larger. /Gunnar > > > > > > > > > > > > > I was asking specifically about the case for gpioget, where a long hold > > > > period makes absolutely no sense. > > > > > > > > > > One could argue that this option doesn't make sense at all for gpioget. :) > > > > > > > And one would be wrong. The point of the hold period for gets is to > > allow the line to settle after a config change before the get itself is > > performed. > > > > One is indeed wrong. > > > > I don't think it hurts to support a longer period of time even if only > > > for code reuse and less surface for bugs. > > > > > > > Well that is a complicated bit of code. > > > > I'll submit the daemon RFC tomorrow or on Friday. Maybe this change > isn't even needed after all. > I'm not completely averse to it, but it isn't the best solution to the use case described. It seems to me that if you want to hold the line for that long then the terminating condition you are after is unlikely to be elapsed time. If we do go ahead with it, I think you are right, we should switch to long periods throughout for consistency. Assuming a switch to ppoll(), the only wrinkle then being the debounce period which is constrained to 32bit by the uAPI. Returning an error if the debounce is greater than 32bit is probably best, as anyone trying to debounce for periods measured in minutes or longer is definitely doing something wrong. So basically your original patch, but with the switch to ppoll(). Cheers, Kent.
diff --git a/configure.ac b/configure.ac index 3b5bbf2..a2370c5 100644 --- a/configure.ac +++ b/configure.ac @@ -120,6 +120,8 @@ AS_IF([test "x$with_tools" = xtrue], AC_CHECK_FUNC([asprintf], [], [FUNC_NOT_FOUND_TOOLS([asprintf])]) AC_CHECK_FUNC([scandir], [], [FUNC_NOT_FOUND_TOOLS([scandir])]) AC_CHECK_FUNC([versionsort], [], [FUNC_NOT_FOUND_TOOLS([versionsort])]) + AC_CHECK_FUNC([strtoull], [], [FUNC_NOT_FOUND_TOOLS([strtoull])]) + AC_CHECK_FUNC([nanosleep], [], [FUNC_NOT_FOUND_TOOLS([nanosleep])]) AS_IF([test "x$with_gpioset_interactive" = xtrue], [PKG_CHECK_MODULES([LIBEDIT], [libedit >= 3.1])]) ]) diff --git a/tools/gpioget.c b/tools/gpioget.c index f611737..bad7667 100644 --- a/tools/gpioget.c +++ b/tools/gpioget.c @@ -19,7 +19,7 @@ struct config { bool unquoted; enum gpiod_line_bias bias; enum gpiod_line_direction direction; - unsigned int hold_period_us; + unsigned long long hold_period_us; const char *chip_id; const char *consumer; }; @@ -205,7 +205,7 @@ int main(int argc, char **argv) die_perror("unable to request lines"); if (cfg.hold_period_us) - usleep(cfg.hold_period_us); + sleep_us(cfg.hold_period_us); ret = gpiod_line_request_get_values(request, values); if (ret) diff --git a/tools/gpiomon.c b/tools/gpiomon.c index e3abb2d..a8a3302 100644 --- a/tools/gpiomon.c +++ b/tools/gpiomon.c @@ -5,6 +5,7 @@ #include <getopt.h> #include <gpiod.h> #include <inttypes.h> +#include <limits.h> #include <poll.h> #include <stdio.h> #include <stdlib.h> @@ -24,13 +25,13 @@ struct config { enum gpiod_line_bias bias; enum gpiod_line_edge edges; int events_wanted; - unsigned int debounce_period_us; + unsigned long long debounce_period_us; const char *chip_id; const char *consumer; const char *fmt; enum gpiod_line_clock event_clock; int timestamp_fmt; - int timeout; + long long timeout; }; static void print_help(void) @@ -389,9 +390,17 @@ int main(int argc, char **argv) if (cfg.active_low) gpiod_line_settings_set_active_low(settings, true); - if (cfg.debounce_period_us) + if (cfg.debounce_period_us) { + if (cfg.debounce_period_us > UINT_MAX) + die("invalid debounce period: %llu", + cfg.debounce_period_us); + gpiod_line_settings_set_debounce_period_us( - settings, cfg.debounce_period_us); + settings, (unsigned long)cfg.debounce_period_us); + } + + if (cfg.timeout > INT_MAX) + die("invalid idle timeout: %llu", cfg.timeout); gpiod_line_settings_set_event_clock(settings, cfg.event_clock); gpiod_line_settings_set_edge_detection(settings, cfg.edges); @@ -453,7 +462,7 @@ int main(int argc, char **argv) for (;;) { fflush(stdout); - ret = poll(pollfds, resolver->num_chips, cfg.timeout); + ret = poll(pollfds, resolver->num_chips, (int)cfg.timeout); if (ret < 0) die_perror("error polling for events"); diff --git a/tools/gpioset.c b/tools/gpioset.c index 863da4a..46dde07 100644 --- a/tools/gpioset.c +++ b/tools/gpioset.c @@ -28,8 +28,8 @@ struct config { enum gpiod_line_bias bias; enum gpiod_line_drive drive; int toggles; - unsigned int *toggle_periods; - unsigned int hold_period_us; + unsigned long long *toggle_periods; + unsigned long long hold_period_us; const char *chip_id; const char *consumer; }; @@ -94,10 +94,10 @@ static int parse_drive_or_die(const char *option) return 0; } -static int parse_periods_or_die(char *option, unsigned int **periods) +static int parse_periods_or_die(char *option, unsigned long long **periods) { int i, num_periods = 1; - unsigned int *pp; + unsigned long long *pp; char *end; for (i = 0; option[i] != '\0'; i++) @@ -376,7 +376,7 @@ static void toggle_all_lines(struct line_resolver *resolver) * and apply the values to the requests. * offset and values are scratch pads for working. */ -static void toggle_sequence(int toggles, unsigned int *toggle_periods, +static void toggle_sequence(int toggles, unsigned long long *toggle_periods, struct gpiod_line_request **requests, struct line_resolver *resolver, unsigned int *offsets, @@ -388,7 +388,7 @@ static void toggle_sequence(int toggles, unsigned int *toggle_periods, return; for (;;) { - usleep(toggle_periods[i]); + sleep_us(toggle_periods[i]); toggle_all_lines(resolver); apply_values(requests, resolver, offsets, values); @@ -826,7 +826,7 @@ static void interact(struct gpiod_line_request **requests, printf("invalid period: '%s'\n", words[1]); goto cmd_ok; } - usleep(period_us); + sleep_us(period_us); goto cmd_ok; } @@ -981,7 +981,7 @@ int main(int argc, char **argv) } if (cfg.hold_period_us) - usleep(cfg.hold_period_us); + sleep_us(cfg.hold_period_us); #ifdef GPIOSET_INTERACTIVE if (cfg.interactive) diff --git a/tools/tools-common.c b/tools/tools-common.c index 64592d3..500e9a2 100644 --- a/tools/tools-common.c +++ b/tools/tools-common.c @@ -112,12 +112,12 @@ int parse_bias_or_die(const char *option) return GPIOD_LINE_BIAS_DISABLED; } -int parse_period(const char *option) +long long parse_period(const char *option) { - unsigned long p, m = 0; + unsigned long long p, m = 0; char *end; - p = strtoul(option, &end, 10); + p = strtoull(option, &end, 10); switch (*end) { case 'u': @@ -147,15 +147,15 @@ int parse_period(const char *option) } p *= m; - if (*end != '\0' || p > INT_MAX) + if (*end != '\0' || p > LLONG_MAX) return -1; return p; } -unsigned int parse_period_or_die(const char *option) +unsigned long long parse_period_or_die(const char *option) { - int period = parse_period(option); + long long period = parse_period(option); if (period < 0) die("invalid period: %s", option); @@ -163,6 +163,16 @@ unsigned int parse_period_or_die(const char *option) return period; } +void sleep_us(unsigned long long period) +{ + struct timespec spec; + + spec.tv_sec = period / 1000000; + spec.tv_nsec = (period % 1000000) * 1000; + + nanosleep(&spec, NULL); +} + int parse_uint(const char *option) { unsigned long o; diff --git a/tools/tools-common.h b/tools/tools-common.h index c82317a..bc63080 100644 --- a/tools/tools-common.h +++ b/tools/tools-common.h @@ -87,8 +87,9 @@ void die(const char *fmt, ...) NORETURN PRINTF(1, 2); void die_perror(const char *fmt, ...) NORETURN PRINTF(1, 2); void print_version(void); int parse_bias_or_die(const char *option); -int parse_period(const char *option); -unsigned int parse_period_or_die(const char *option); +long long parse_period(const char *option); +unsigned long long parse_period_or_die(const char *option); +void sleep_us(unsigned long long period); int parse_uint(const char *option); unsigned int parse_uint_or_die(const char *option); void print_bias_help(void);