Merge pull request #4642

25d327e7 device: extended logging, refactored device selection code (xiphon)
825d836f device: fixed Ledger Nano S device selection (xiphon)

src/device/device_io_hid.cpp

@@ -13,6 +13,7 @@
 //
 #if defined(HAVE_HIDAPI) 
 
+#include <boost/scope_exit.hpp>
 #include "log.hpp"
 #include "device_io_hid.hpp"
 
@@ -69,11 +70,47 @@ namespace hw {
 
     void device_io_hid::connect(void *params) {
       hid_conn_params *p = (struct hid_conn_params*)params;
-      this->connect(p->vid, p->pid, p->interface_number, p->usage_page, p->interface_OR_page);
+      this->connect(p->vid, p->pid, p->interface_number, p->usage_page);
     }
 
-    void device_io_hid::connect(unsigned int vid, unsigned  int pid, unsigned int interface_number, unsigned int usage_page, bool interface_OR_page ) {
+    hid_device_info *device_io_hid::find_device(hid_device_info *devices_list, boost::optional<int> interface_number, boost::optional<unsigned short> usage_page) {
-      hid_device_info *hwdev_info, *hwdev_info_list;
+      bool select_any = !interface_number && !usage_page;
+
+      MDEBUG( "Looking for " <<
+              (select_any ? "any HID Device" : "HID Device with") <<
+              (interface_number ? (" interface_number " + std::to_string(interface_number.value())) : "") <<
+              ((interface_number && usage_page) ? " or" : "") <<
+              (usage_page ? (" usage_page " + std::to_string(usage_page.value())) : ""));
+
+      hid_device_info *result = nullptr;
+      for (; devices_list != nullptr; devices_list = devices_list->next) {
+        BOOST_SCOPE_EXIT(&devices_list, &result) {
+          MDEBUG( (result == devices_list ? "SELECTED" : "SKIPPED ") <<
+                  " HID Device" <<
+                  " path " << safe_hid_path(devices_list) <<
+                  " interface_number " << devices_list->interface_number <<
+                  " usage_page " << devices_list->usage_page);
+        }
+        BOOST_SCOPE_EXIT_END
+
+        if (result != nullptr) {
+          continue;
+        }
+
+        if (select_any) {
+          result = devices_list;
+        } else if (interface_number && devices_list->interface_number == interface_number.value()) {
+          result = devices_list;
+        } else if (usage_page && devices_list->usage_page == usage_page.value()) {
+          result = devices_list;
+        }
+      }
+
+      return result;
+    }
+
+    void device_io_hid::connect(unsigned int vid, unsigned  int pid, boost::optional<int> interface_number, boost::optional<unsigned short> usage_page) {
+      hid_device_info *hwdev_info_list;
       hid_device      *hwdev;
 
       this->disconnect();
@@ -81,17 +118,8 @@ namespace hw {
       hwdev_info_list = hid_enumerate(vid, pid);
       ASSERT_X(hwdev_info_list, "Unable to enumerate device "+std::to_string(vid)+":"+std::to_string(vid)+  ": "+ safe_hid_error(this->usb_device));
       hwdev = NULL;
-      hwdev_info = hwdev_info_list;
+      if (hid_device_info *device = find_device(hwdev_info_list, interface_number, usage_page)) {
-      while (hwdev_info) {
+        hwdev = hid_open_path(device->path);
-        if ((interface_OR_page && ((usage_page == 0xffa0) || (interface_number == 0))) ||
-                                  ((usage_page == 0xffa0) && (interface_number == 0)) ) {
-          MDEBUG("HID Device found: " << safe_hid_path(hwdev_info));
-          hwdev = hid_open_path(hwdev_info->path);
-          break;
-        } else {
-          MDEBUG("HID Device discard: " << safe_hid_path(hwdev_info) << "("+std::to_string(hwdev_info->usage_page) << "," << std::to_string(hwdev_info->interface_number) << ")");
-        }
-        hwdev_info = hwdev_info->next;
       }
       hid_free_enumeration(hwdev_info_list);
       ASSERT_X(hwdev, "Unable to open device "+std::to_string(pid)+":"+std::to_string(vid));

src/device/device_io_hid.hpp

@@ -29,6 +29,7 @@
 
 #if defined(HAVE_HIDAPI) 
 
+#include <boost/optional/optional.hpp>
 #include <hidapi/hidapi.h>
 #include "device_io.hpp"
 
@@ -52,9 +53,8 @@ namespace hw {
     struct hid_conn_params {
       unsigned int vid; 
       unsigned int pid; 
-      unsigned int interface_number;
+      int interface_number;
-      unsigned int usage_page;
+      unsigned short usage_page;
-      bool interface_OR_page ;
     };
     
 
@@ -82,13 +82,11 @@ namespace hw {
       unsigned int wrapCommand(const unsigned char *command, size_t command_len, unsigned char *out, size_t out_len);
       unsigned int unwrapReponse(const unsigned char *data, size_t data_len, unsigned char *out, size_t out_len);
  
+      hid_device_info *find_device(hid_device_info *devices_list, boost::optional<int> interface_number, boost::optional<unsigned short> usage_page);
  
     public:
       bool hid_verbose = false;
 
-      static const unsigned int  OR_SELECT = 1;
-      static const unsigned int  AND_SELECT = 2;
-
       static const unsigned short DEFAULT_CHANNEL     = 0x0001;
       static const unsigned char  DEFAULT_TAG         = 0x01;
       static const unsigned int   DEFAULT_PACKET_SIZE = 64;
@@ -100,7 +98,7 @@ namespace hw {
 
       void init();  
       void connect(void *params);
-      void connect(unsigned int vid, unsigned  int pid, unsigned int interface_number, unsigned int usage_page, bool interface_OR_page );
+      void connect(unsigned int vid, unsigned  int pid, boost::optional<int> interface_number, boost::optional<unsigned short> usage_page);
       bool connected() const;
       int  exchange(unsigned char *command, unsigned int cmd_len, unsigned char *response, unsigned int max_resp_len);
       void disconnect();

src/device/device_ledger.cpp

@@ -340,7 +340,7 @@ namespace hw {
 
     bool device_ledger::connect(void) {
       this->disconnect();
-      hw_device.connect(0x2c97,0x0001, 0, 0xffa0, hw_device.OR_SELECT);
+      hw_device.connect(0x2c97, 0x0001, 0, 0xffa0);
       this->reset();
       #ifdef DEBUG_HWDEVICE
       cryptonote::account_public_address pubkey;