Monitor udev for hidraw devices instead of hid devices.

device/hid/hid_service_linux.cc

 // 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.
 
 #include "device/hid/hid_service_linux.h"
 
 #include <linux/hidraw.h>
 #include <sys/ioctl.h>
 #include <stdint.h>
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "device/hid/hid_connection_linux.h"
 #include "device/hid/hid_device_info.h"
 #include "device/hid/hid_report_descriptor.h"
 #include "device/udev_linux/udev.h"
 
 namespace device {
 
 namespace {
 
-const char kHIDSubSystem[] = "hid";
-const char kHidrawSubsystem[] = "hidraw";
+const char kHidrawSubSystem[] = "hidraw";

[mschilder1, 2014/07/17 01:52:38]

as const, should this be kHIDRAWSUBSYSTEM?

[Reilly Grant (use Gerrit), 2014/07/17 03:22:53]

Google C++ style says kHidrawSubSystem (or kHidrawSubsystem) and Chromium style
says HIDRAW_SUBSYSTEM. I've seen the Google style in a bunch of places so for
consistency I think this should be left as is.

 const char kHIDID[] = "HID_ID";
 const char kHIDName[] = "HID_NAME";
 const char kHIDUnique[] = "HID_UNIQ";
 
 }  // namespace
 
 HidServiceLinux::HidServiceLinux() {
   DeviceMonitorLinux* monitor = DeviceMonitorLinux::GetInstance();
   monitor->AddObserver(this);
   monitor->Enumerate(
       base::Bind(&HidServiceLinux::OnDeviceAdded, base::Unretained(this)));
 }
 
 scoped_refptr<HidConnection> HidServiceLinux::Connect(
     const HidDeviceId& device_id) {
   HidDeviceInfo device_info;
   if (!GetDeviceInfo(device_id, &device_info))
     return NULL;
 
   ScopedUdevDevicePtr device =
       DeviceMonitorLinux::GetInstance()->GetDeviceFromPath(
           device_info.device_id);
 
   if (device) {
-    std::string dev_node;
-    if (!FindHidrawDevNode(device.get(), &dev_node)) {
-      LOG(ERROR) << "Cannot open HID device as hidraw device.";
-      return NULL;
-    }
+    std::string dev_node = udev_device_get_devnode(device.get());
     return new HidConnectionLinux(device_info, dev_node);
   }
 
   return NULL;
 }
 
 HidServiceLinux::~HidServiceLinux() {
   if (DeviceMonitorLinux::HasInstance())
     DeviceMonitorLinux::GetInstance()->RemoveObserver(this);
 }
 
 void HidServiceLinux::OnDeviceAdded(udev_device* device) {
   if (!device)
     return;
 
   const char* device_path = udev_device_get_syspath(device);
   if (!device_path)
     return;
   const char* subsystem = udev_device_get_subsystem(device);
-  if (!subsystem || strcmp(subsystem, kHIDSubSystem) != 0)
+  if (!subsystem || strcmp(subsystem, kHidrawSubSystem) != 0)
     return;
 
   HidDeviceInfo device_info;
   device_info.device_id = device_path;
 
   uint32_t int_property = 0;
   const char* str_property = NULL;
 
-  const char* hid_id = udev_device_get_property_value(device, kHIDID);
+  udev_device *parent = udev_device_get_parent(device);
+  if (!parent) {

[mschilder1, 2014/07/17 01:52:38]

inconsistent if(){ expr; } vs. if () expr; in this file / method.
What do the rules say again?
Certainly not tolerate inconsistency ;-)

[Reilly Grant (use Gerrit), 2014/07/17 03:22:53]

The rules say it is my choice. I dislike inconsistency but also have a strong
inclination to avoid single-line ifs.

+    return;
+  }
+
+  const char* hid_id = udev_device_get_property_value(parent, kHIDID);
   if (!hid_id)
     return;
 
   std::vector<std::string> parts;
   base::SplitString(hid_id, ':', &parts);
   if (parts.size() != 3) {
     return;
   }
 
   if (HexStringToUInt(base::StringPiece(parts[1]), &int_property)) {
     device_info.vendor_id = int_property;
   }
 
   if (HexStringToUInt(base::StringPiece(parts[2]), &int_property)) {
     device_info.product_id = int_property;
   }
 
-  str_property = udev_device_get_property_value(device, kHIDUnique);
+  str_property = udev_device_get_property_value(parent, kHIDUnique);
   if (str_property != NULL)
     device_info.serial_number = str_property;
 
-  str_property = udev_device_get_property_value(device, kHIDName);
+  str_property = udev_device_get_property_value(parent, kHIDName);
   if (str_property != NULL)
     device_info.product_name = str_property;
 
-  std::string dev_node;
-  if (!FindHidrawDevNode(device, &dev_node)) {
-    LOG(ERROR) << "Cannot find device node for HID device.";
-    return;
-  }
-
-  int flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
+  const std::string dev_node = udev_device_get_devnode(device);
+  const int flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
 
   base::File device_file(base::FilePath(dev_node), flags);
   if (!device_file.IsValid()) {
     LOG(ERROR) << "Cannot open '" << dev_node << "': "
         << base::File::ErrorToString(device_file.error_details());
     return;
   }
 
   int desc_size = 0;
   int res = ioctl(device_file.GetPlatformFile(), HIDIOCGRDESCSIZE, &desc_size);
@@ skipping 23 matching lines @@
 
   AddDevice(device_info);
 }
 
 void HidServiceLinux::OnDeviceRemoved(udev_device* device) {
   const char* device_path = udev_device_get_syspath(device);;
   if (device_path)
     RemoveDevice(device_path);
 }
 
-bool HidServiceLinux::FindHidrawDevNode(udev_device* parent,
-                                        std::string* result) {
-  udev* udev = udev_device_get_udev(parent);
-  if (!udev) {
-    return false;
-  }
-  ScopedUdevEnumeratePtr enumerate(udev_enumerate_new(udev));
-  if (!enumerate) {
-    return false;
-  }
-  if (udev_enumerate_add_match_subsystem(enumerate.get(), kHidrawSubsystem)) {
-    return false;
-  }
-  if (udev_enumerate_scan_devices(enumerate.get())) {
-    return false;
-  }
-  std::string parent_path(udev_device_get_devpath(parent));
-  if (parent_path.length() == 0 || *parent_path.rbegin() != '/')
-    parent_path += '/';
-  udev_list_entry* devices = udev_enumerate_get_list_entry(enumerate.get());
-  for (udev_list_entry* i = devices; i != NULL;
-       i = udev_list_entry_get_next(i)) {
-    ScopedUdevDevicePtr hid_dev(
-        udev_device_new_from_syspath(udev, udev_list_entry_get_name(i)));
-    const char* raw_path = udev_device_get_devnode(hid_dev.get());
-    std::string device_path = udev_device_get_devpath(hid_dev.get());
-    if (raw_path &&
-        !device_path.compare(0, parent_path.length(), parent_path)) {
-      std::string sub_path = device_path.substr(parent_path.length());
-      if (sub_path.substr(0, sizeof(kHidrawSubsystem) - 1) ==
-          kHidrawSubsystem) {
-        *result = raw_path;
-        return true;
-      }
-    }
-  }
-
-  return false;
-}
-
 }  // namespace device