Re: [v3][libgpiod][PATCH 1/1] gpio-tools-test.bats: modify delays in toggle test

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Jun 6, 2023 at 5:43 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Mon, Jun 05, 2023 at 01:43:35PM -0700, joe.slater@xxxxxxxxxxxxx wrote:
> > From: Joe Slater <joe.slater@xxxxxxxxxxxxx>
> >
> > The test "gpioset: toggle (continuous)" uses fixed delays to test
> > toggling values.  This is not reliable, so we switch to looking
> > for transitions from one value to another.
> >
> > We wait for a transition up to 1.5 seconds.
> >
>
> For future reference, the subject line should've been
> "[libgpiod][PATCH v3]".
> The version goes within the [PATCH], and 1/1 is optional unless you have
> a cover letter.
>
> > Signed-off-by: Joe Slater <joe.slater@xxxxxxxxxxxxx>
> > ---
>
> Here you would normally list the changes between revisions.
> So I'm not sure what has actually changed since v1.
> The loop limit went from 10 to 15?
>
> >  tools/gpio-tools-test.bats | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/gpio-tools-test.bats b/tools/gpio-tools-test.bats
> > index c83ca7d..05d7138 100755
> > --- a/tools/gpio-tools-test.bats
> > +++ b/tools/gpio-tools-test.bats
> > @@ -141,6 +141,20 @@ gpiosim_check_value() {
> >       [ "$VAL" = "$EXPECTED" ]
> >  }
> >
> > +gpiosim_wait_value() {
> > +     local OFFSET=$2
> > +     local EXPECTED=$3
> > +     local DEVNAME=${GPIOSIM_DEV_NAME[$1]}
> > +     local CHIPNAME=${GPIOSIM_CHIP_NAME[$1]}
> > +     local PORT=$GPIOSIM_SYSFS/$DEVNAME/$CHIPNAME/sim_gpio$OFFSET/value
> > +
> > +     for i in {1..15}; do
> > +             [ "$(<$PORT)" = "$EXPECTED" ] && return
> > +             sleep 0.1
> > +     done
> > +     return 1
> > +}
> > +
> >  gpiosim_cleanup() {
> >       for CHIP in ${!GPIOSIM_CHIP_NAME[@]}
> >       do
> > @@ -1567,15 +1581,15 @@ request_release_line() {
> >       gpiosim_check_value sim0 4 0
> >       gpiosim_check_value sim0 7 0
> >
> > -     sleep 1
> > -
> > -     gpiosim_check_value sim0 1 0
> > +     # sleeping fixed amounts can be unreliable, so we
> > +     # sync to the toggles
> > +     #
>
> You said you would get rid of this comment.
>
>
> The patch works for me, so I'm otherwise fine with it.
>
> Cheers,
> Kent.

Patch looks fine but interestingly, I'm not seeing any improvement in
terms of execution times. Is this to be expected?

Bart