hidapi icon indicating copy to clipboard operation
hidapi copied to clipboard

hidapi/linux: retry hid_send_feature_report() if the ioctl() fails with EPIPE (e.g. the device stalled)

Open slouken opened this issue 2 years ago • 6 comments

slouken avatar May 26 '23 16:05 slouken

Just wondering what is the device which has this issue. Thanks. Retry count of 50 seems to be quite high.

mcuee avatar May 27 '23 01:05 mcuee

I don't remember offhand, this fix is several years old, but it's 50 retries without any delay, so I believe this was a very fast loop.

slouken avatar May 27 '23 03:05 slouken

50 is definitely too much (that is a lot of flood on the driver/bus in some cases when EPIPE is even expected. I cannot imagine a case, where the ioctl fails for 47 times and then suddenly succeeds for the 48-th time. By my experience - if it fails for 2-3 times in a row - it will not recover on its own.

And this workaround is too specific - why only send_feature_report? What about input/output reports, etc.?

And if we're to handle the return codes of ioctl - we better handle not only EPIPE, but at least EINTR (often a debugger attachement) as well.

In one of my projects I use:

template <typename ...ARGS>
inline int xioctl(int fd, unsigned long int request, ARGS&&... args) {
    int result = 0;

    int fails = 5;

    do {
        result = ::ioctl(fd, request, std::forward<ARGS>(args)...);

        if (result < 0) {
            const auto err = errno;
            
            if (err == EINTR // interruptions, like debugger attachments, etc.
                    || err == EPIPE) {
                if (fails-- > 0) {
                    continue;
                }
            }
        }

        break;
    }
    while(true);

    return result;
}
}

I suggest we adapt it (make a C macro instead of C++ template) for our case, and use in all report related functions.

And 5 attempts should be enough, unless you have specific measurements where 50 is justified.

Youw avatar Jun 01 '23 09:06 Youw

5 was definitely not enough. I ran into this on the Steam Link hardware, which is fairly slow, and I think the actual number in that case was the low 20s and I doubled it for safety. My goal was to provide the lowest latency possible while still handling EPIPE conditions. This happens only rarely, so my assumption was that leaving the number high was fine.

In any case, I just wanted to share in case this was helpful. If you go with a different approach, no worries.

slouken avatar Jun 01 '23 14:06 slouken

In that case maybe make the number of retries configurable. Make it small (5?) by default and give some means to configure it for projects like SDL.

This happens only rarely

The thing is that I encounter EPIPE quite often and consistently with some devices in some scenarios and in all of those - it is not recoverable (due to HW/FW bug/inconsistency, driver error, etc.). And too many retries would just flood the logs with no real improvement(s).

Youw avatar Jun 01 '23 14:06 Youw

Making a draft for now, until we have a better implementation.

Youw avatar Apr 06 '24 10:04 Youw