chrome.hid : enrich device info with Top-Level collections usages

device/hid/hid_report_descriptor.h

Index: device/hid/hid_report_descriptor.h
diff --git a/device/hid/hid_report_descriptor.h b/device/hid/hid_report_descriptor.h
new file mode 100644
index 0000000000000000000000000000000000000000..64879f1641ff0970952cda68979fe2140959cabc
--- /dev/null
+++ b/device/hid/hid_report_descriptor.h
@@ -0,0 +1,41 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_HID_HID_REPORT_DESCRIPTOR_H_
+#define DEVICE_HID_HID_REPORT_DESCRIPTOR_H_
+
+#include <vector>
+
+#include "base/memory/linked_ptr.h"
+#include "device/hid/hid_report_descriptor_item.h"
+#include "device/hid/hid_usage_and_page.h"
+
+namespace device {
+
+// HID report descriptor.
+// See section 6.2.2 of HID specifications (v1.11).
+class HidReportDescriptor {
+
+ public:
+  HidReportDescriptor(const uint8_t* bytes, size_t size);
+  ~HidReportDescriptor();
+
+  const std::vector<linked_ptr<HidReportDescriptorItem> >& items() const {
+    return items_;
+  }
+
+  // Returns HID usages of top-level collections present in the descriptor.
+  void GetTopLevelCollections(
+      std::vector<HidUsageAndPage>* topLevelCollections);
+
+ private:
+  std::vector<linked_ptr<HidReportDescriptorItem> > items_;
+};
+
+std::ostream& operator<<(std::ostream& os,
+                         const HidReportDescriptor& descriptor);

[Ken Rockot(use gerrit already), 2014/04/21 21:53:53]

Two concerns:

1. It is really not idiomatic in chromium code to use these operators. I should
have stated this before (sorry), but please just implement a ToString() method
on the type if you really need stringification. But see #2.

2. Do you really even need stringification? If this is only here to make writing
tests easier, please remove it. It is much better to test individual bits of the
structure than to stringify the whole thing and test the string output.

[jracle (use Gerrit), 2014/04/23 13:06:47]

Since it is not idiomatic, and I don't see a wide usage of ToString() nor usage
out of my tests, I'm in favor of point 2.

Well, it'll be point 2' ;), move operator<< into unit tests. Purpose is to
maintain parsing of HID report descriptor, but only in tests. Looking at a
parsed report descriptor is very common and useful practice IMO. I prefer to
keep it, even if it does not directly concern our code. Indeed, I first looked
at report descriptor before I did implement the code to retrieve top-level
collection's usages. It was helpful to see report was correctly parsed.

+
+}  // namespace device
+
+#endif  // DEVICE_HID_HID_REPORT_DESCRIPTOR_H_