[PATCH libevdev] tools: print the mean frequency together with the max frequency

[Benjamin Tissoires]

On Mon, Sep 19, 2016 at 3:03 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> And if they're 30% out, print a warning. On the ThinkPad X1 Wireless Touch
> Mouse (when connected via bluetooth) we get a bunch of events at the start of
> the movement, all less than 1ms apart. Best guess is that the device goes to
> low-power, then notices the movement and buffers the event until the BT
> connection is back up. Then it sends all events at once. Usually they're less

This reminds me the exact same bug we had on USB. The internal device
output queue filled in while the device being put on suspend, and when
resumed, the queue was emptied and spurious events were sent. We got
it fixed by ignoring the first few events:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b905811a49bcd6e6726ce5bbb591f57aaddfd3be

I wonder if the same should not be done for bluetooth devices.

> than 1ms apart, but at one recording showed a 37ms delay before we go back to
> the normal 70ms (~15Hz) the mouse has otherwise.
>
> This is unpredictable enough that we can't just work around it so instead
> print a warning to the user so they can go investigate.

Well, if this is the same type of bug we had in USB, we could have the
same in this tool. If the first reports are obviously too close to
each other, discard them.

>
> https://bugs.freedesktop.org/show_bug.cgi?id=97812
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  tools/mouse-dpi-tool.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)
>
> diff --git a/tools/mouse-dpi-tool.c b/tools/mouse-dpi-tool.c
> index 30d689a..852ff07 100644
> --- a/tools/mouse-dpi-tool.c
> +++ b/tools/mouse-dpi-tool.c
> @@ -44,6 +44,9 @@
>  struct measurements {
>         int distance;
>         double max_frequency;
> +       double *frequencies;
> +       size_t frequencies_sz;
> +       size_t nfrequencies;
>         uint64_t us;
>  };
>
> @@ -70,6 +73,21 @@ get_frequency(uint64_t last, uint64_t current)
>         return 1000000.0/(current - last);
>  }
>
> +static inline void
> +push_frequency(struct measurements *m, double freq)
> +{
> +       if (m->nfrequencies == m->frequencies_sz) {
> +               m->frequencies_sz += 100;
> +               m->frequencies = realloc(m->frequencies,
> +                                        m->frequencies_sz * sizeof *m->frequencies);

That's going to be a lot of realloc if the user goes slow :/

> +               if (!m->frequencies)
> +                       abort();
> +       }
> +
> +       m->frequencies[m->nfrequencies] = freq;
> +       m->nfrequencies++;
> +}
> +
>  static int
>  print_current_values(const struct measurements *m)
>  {
> @@ -109,8 +127,8 @@ handle_event(struct measurements *m, const struct input_event *ev)
>                         m->distance = 0;
>                 } else {
>                         double freq = get_frequency(last_us, m->us);
> -                       if (freq < 1200)
> -                               m->max_frequency = max(freq, m->max_frequency);
> +                       push_frequency(m, freq);
> +                       m->max_frequency = max(freq, m->max_frequency);
>                         return print_current_values(m);
>                 }
>
> @@ -166,12 +184,29 @@ mainloop(struct libevdev *dev, struct measurements *m) {
>         return 0;
>  }
>
> +static inline double
> +mean_frequency(struct measurements *m)
> +{
> +       int idx;
> +
> +       idx = m->nfrequencies/2;
> +       return m->frequencies[idx];
> +}
> +
>  static void
>  print_summary(struct measurements *m)
>  {
>         int res;
> +       int max_freq = (int)m->max_frequency,
> +           mean_freq = (int)mean_frequency(m);
> +
> +       printf("Estimated sampling frequency: %dHz (mean %dHz)\n",
> +              max_freq, mean_freq);

Isn't the mean frequency the most interesting?
I am sure you can also directly discard some frequencies depending on
the bus used: USB max should be around 1000Hz, maybe slightly more,
Bluetooth not more than 500Hz I'd say (I'll need to check on the
spec).
So if you remove the obviously wrong ones (20000 is simply not
possible, the mean frequency should be closer to the real one).

Cheers,
Benjamin

> +
> +       if (max_freq > mean_freq * 1.3)
> +               printf("WARNING: Max frequency is more than 30%% higher "
> +                      "than mean frequency. Manual verification required!\n");
>
> -       printf("Estimated sampling frequency: %dHz\n", (int)m->max_frequency);
>         printf("To calculate resolution, measure physical distance covered\n"
>                "and look up the matching resolution in the table below\n");
>
> @@ -213,7 +248,7 @@ main (int argc, char **argv) {
>         int fd;
>         const char *path;
>         struct libevdev *dev;
> -       struct measurements measurements = {0, 0, 0};
> +       struct measurements measurements = {0};
>
>         if (argc < 2)
>                 return usage();
> --
> 2.7.4
>
> _______________________________________________
> Input-tools mailing list
> Input-tools at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/input-tools