Add report descriptor parser

[cvuchener]

@mrubli2 Here is my first attempt at using the parsed report descriptor for checking HID++ devices. I did not add the support for the new usages yet.

As I said in #12, I failed to get the report count and size because of Windows "buttons". Is this enough? Do you think there is more data that I should (and can) add to the report descriptor?

[mrubli2]

I just gave this a test with a few devices that use the legacy scheme and it seems to work great on both Linux and Windows. (I did a native Windows build, which required a bit of patching here and there but I'll help fix that in an upcoming PR once this change here is in.)

While you're implementing this it would be great if you could also spare a few thoughts on how this information (at minimum a bitmask of short|long|verylong, at best legacy vs. modern as well) could be exposed both to the library itself as well as to library clients. The library itself needs that information, so it can pick a supported report and clients need to know what length reports they're allowed to send.

[cvuchener]

I think it may be ready. @mrubli2 could you test this? I don't have any device using the modern scheme. If you could reread my implementation of Dispatcher::checkReportDescriptor, it would be nice too.

[mrubli2]

Sure, I'll test it for you. Give me a couple of days, though.

[mrubli2]

I just gave it a try with my libhidpp-based tool and it seems to work well. There's one more place, however, where you also need to check for supported request types:

--- a/src/libhidpp/hidpp/Device.cpp
+++ b/src/libhidpp/hidpp/Device.cpp
@@ -76,7 +76,14 @@ Device::Device (Dispatcher *dispatcher, DeviceIndex device_index):

        // Check protocol version
        static constexpr unsigned int software_id = 1;
-       Report request (Report::Short, _device_index, HIDPP20::IRoot::index, HIDPP20::IRoot::Ping, software_id);
+       static constexpr std::array Types = { HIDPP::Report::Short, HIDPP::Report::Long, HIDPP::Report::VeryLong };
+       auto report_info = _dispatcher->reportInfo ();
+       auto type = std::find_if (Types.begin (), Types.end (), [=] (auto type) {
+                       return report_info.hasReport (type);
+       });
+       if (type == Types.end ())
+               throw std::logic_error ("Parameters too long");         // should never happen, our parameters are 0 bytes long
+       Report request (*type, _device_index, HIDPP20::IRoot::index, HIDPP20::IRoot::Ping, software_id);
        auto response = _dispatcher->sendCommand (std::move (request));
        try {
                auto report = response->get (is_wireless ? 2000 : 500); // use longer timeout for wireless devices that can be sleeping.

Without this you get a "readReport timed out" error early on during the protocol version check. At this point it may make sense to factor that logic out, e.g. into ReportInfo since there are already two places using it.

I need to do some more testing with other devices and I'm going to go through the new code to see if anything stands out. Either way, I'll leave comments.

As for you not having a device using the modern scheme I'll send you an email. I think we can remedy that. :-)

[mrubli2]

Are you aware of how if constexpr helps with variants? https://en.cppreference.com/w/cpp/utility/variant/visit has an example of that pattern which I find very helpful.

[cvuchener]

I prefer to use separate functions if they don't share code. So either as I did or using overloaded (which is sadly not part of the standard library).

[mrubli2]

Typo: "delimieter" (and again below)

[mrubli2]

Nice work, thanks! Do you think this would be a good time to bump the version number and tag the repository? There's also this patch here that improves versioning for clients: mrubli@67113ba

[cvuchener]

this would be a good time to bump the version number

You mean start having version numbers. This is too experimental to have nice semantic version number with a stable API. But I could tag some 0.x versions, if you need. When #19 is done and #24 merged? I'm thinking about making some changes to the dispatcher, it may be a good idea to create tag before that.

[mrubli2]

Yeah, I was also just thinking about bumping the minor version, e.g. from 0.1 to 0.2. The top-level CMakeLists.txt currently does say 0.1, although this isn't usable by the clients. Given how useful this merge is for modern HID++ devices it would be great if clients were able to require 0.2 (or whatever it turns out to be).

Semver-lawyer-wise you'd be on the safe side because:

Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable.

As for the timing, sure, let's wait for those two PRs to be merged. I should have some time in the coming days to merge my Windows build fixes with the latest code.