add GetBattorPathFromPhoneSerial to find_usb_devices

devil/devil/utils/find_usb_devices.py

Index: devil/devil/utils/find_usb_devices.py
diff --git a/devil/devil/utils/find_usb_devices.py b/devil/devil/utils/find_usb_devices.py
index 4982e46a66e0734c5a8b7db89ae950ccbcfc7a5c..4275ef751a734f6033dc52fffd9d66af7c3da3f7 100755
--- a/devil/devil/utils/find_usb_devices.py
+++ b/devil/devil/utils/find_usb_devices.py
@@ -7,8 +7,7 @@ import re
 import sys
 import argparse
 
-from devil.utils import cmd_helper
-from devil.utils import lsusb
+from devil.utils import cmd_helper, usb_hubs, lsusb

[rnephew (Reviews Here), 2016/03/22 18:43:14]

I do not think we do a whole lot of multiple imports per line. I think these
should be separated to keep consistency with the rest of the codebase.

[alexandermont, 2016/03/22 19:10:56]

Done

 
 # Note: In the documentation below, "virtual port" refers to the port number
 # as observed by the system (e.g. by usb-devices) and "physical port" refers
@@ -53,6 +52,8 @@ def IsBattor(tty_string, device_tree_map):
   node = device_tree_map[bus].FindDeviceNumber(device)
   return 'Future Technology Devices International' in node.desc
 
+class BattorError(Exception):
+  pass
 
 # Class to identify nodes in the USB topology. USB topology is organized as
 # a tree.
@@ -311,83 +312,12 @@ def GetBusNumberToDeviceTreeMap(fast=False):
   return tree
 
 
-class HubType(object):
-  def __init__(self, id_func, port_mapping):
-    """Defines a type of hub.
-
-    Args:
-      id_func: [USBNode -> bool] is a function that can be run on a node
-        to determine if the node represents this type of hub.
-      port_mapping: [dict(int:(int|dict))] maps virtual to physical port
-        numbers. For instance, {3:1, 1:2, 2:3} means that virtual port 3
-        corresponds to physical port 1, virtual port 1 corresponds to physical
-        port 2, and virtual port 2 corresponds to physical port 3. In the
-        case of hubs with "internal" topology, this is represented by nested
-        maps. For instance, {1:{1:1,2:2},2:{1:3,2:4}} means, e.g. that the
-        device plugged into physical port 3 will show up as being connected
-        to port 1, on a device which is connected to port 2 on the hub.
-    """
-    self._id_func = id_func
-    # v2p = "virtual to physical" ports
-    self._v2p_port = port_mapping
-
-  def IsType(self, node):
-    """Determines if the given Node is a hub of this type.
-
-    Args:
-      node: [USBNode] Node to check.
-    """
-    return self._id_func(node)
-
-  def GetPhysicalPortToNodeTuples(self, node):
-    """Gets devices connected to the physical ports on a hub of this type.
-
-    Args:
-      node: [USBNode] Node representing a hub of this type.
-
-    Yields:
-      A series of (int, USBNode) tuples giving a physical port
-      and the USBNode connected to it.
-
-    Raises:
-      ValueError: If the given node isn't a hub of this type.
-    """
-    if self.IsType(node):
-      for res in self._GppHelper(node, self._v2p_port):
-        yield res
-    else:
-      raise ValueError('Node must be a hub of this type')
-
-  def _GppHelper(self, node, mapping):
-    """Helper function for GetPhysicalPortToNodeMap.
-
-    Gets devices connected to physical ports, based on device tree
-    rooted at the given node and the mapping between virtual and physical
-    ports.
-
-    Args:
-      node: [USBNode] Root of tree to search for devices.
-      mapping: [dict] Mapping between virtual and physical ports.
-
-    Yields:
-      A series of (int, USBNode) tuples giving a physical port
-      and the Node connected to it.
-    """
-    for (virtual, physical) in mapping.iteritems():
-      if node.HasPort(virtual):
-        if isinstance(physical, dict):
-          for res in self._GppHelper(node.PortToDevice(virtual), physical):
-            yield res
-        else:
-          yield (physical, node.PortToDevice(virtual))
-
-
 def GetHubsOnBus(bus, hub_types):
   """Scans for all hubs on a bus of given hub types.
 
   Args:
     bus: [USBNode] Bus object.
-    hub_types: [iterable(HubType)] Possible types of hubs.
+    hub_types: [iterable(usb_hubs.HubType)] Possible types of hubs.
 
   Yields:
     Sequence of tuples representing (hub, type of hub)
@@ -402,7 +332,7 @@ def GetPhysicalPortToNodeMap(hub, hub_type):
   """Gets physical-port:node mapping for a given hub.
   Args:
     hub: [USBNode] Hub to get map for.
-    hub_type: [HubType] Which type of hub it is.
+    hub_type: [usb_hubs.HubType] Which type of hub it is.
 
   Returns:
     Dict of {physical port: node}
@@ -415,7 +345,7 @@ def GetPhysicalPortToBusDeviceMap(hub, hub_type):
   """Gets physical-port:(bus#, device#) mapping for a given hub.
   Args:
     hub: [USBNode] Hub to get map for.
-    hub_type: [HubType] Which type of hub it is.
+    hub_type: [usb_hubs.HubType] Which type of hub it is.
 
   Returns:
     Dict of {physical port: (bus number, device number)}
@@ -429,7 +359,7 @@ def GetPhysicalPortToSerialMap(hub, hub_type):
   """Gets physical-port:serial# mapping for a given hub.
   Args:
     hub: [USBNode] Hub to get map for.
-    hub_type: [HubType] Which type of hub it is.
+    hub_type: [usb_hubs.HubType] Which type of hub it is.
 
   Returns:
     Dict of {physical port: serial number)}
@@ -444,7 +374,7 @@ def GetPhysicalPortToTTYMap(device, hub_type):
   """Gets physical-port:tty-string mapping for a given hub.
   Args:
     hub: [USBNode] Hub to get map for.
-    hub_type: [HubType] Which type of hub it is.
+    hub_type: [usb_hubs.HubType] Which type of hub it is.
 
   Returns:
     Dict of {physical port: tty-string)}
@@ -460,7 +390,7 @@ def CollectHubMaps(hub_types, map_func, device_tree_map=None, fast=False):
   """Runs a function on all hubs in the system and collects their output.
 
   Args:
-    hub_types: [HubType] List of possible hub types.
+    hub_types: [usb_hubs.HubType] List of possible hub types.
     map_func: [string] Function to run on each hub.
     device_tree: Previously constructed device tree map, if any.
     fast: Whether to construct device tree fast, if not already provided
@@ -552,30 +482,10 @@ def GetBusDeviceToTTYMap():
 # 4 connects to another 'virtual' hub that itself has the
 # virtual-to-physical port mapping {1:4, 2:3, 3:2, 4:1}.
 
-PLUGABLE_7PORT_LAYOUT = {1:7,
-                         2:6,
-                         3:5,
-                         4:{1:4, 2:3, 3:2, 4:1}}
 
 def TestUSBTopologyScript():
   """Test display and hub identification."""
   # Identification criteria for Plugable 7-Port Hub
-  def _is_plugable_7port_hub(node):
-    """Check if a node is a Plugable 7-Port Hub
-    (Model USB2-HUB7BC)
-    The topology of this device is a 4-port hub,
-    with another 4-port hub connected on port 4.
-    """
-    if not isinstance(node, USBDeviceNode):
-      return False
-    if '4-Port HUB' not in node.desc:
-      return False
-    if not node.HasPort(4):
-      return False
-    return '4-Port HUB' in node.PortToDevice(4).desc
-
-  plugable_7port = HubType(_is_plugable_7port_hub,
-                           PLUGABLE_7PORT_LAYOUT)
   print '==== USB TOPOLOGY SCRIPT TEST ===='
 
   # Display devices
@@ -587,19 +497,80 @@ def TestUSBTopologyScript():
 
   # Display TTY information about devices plugged into hubs.
   print '==== TTY INFORMATION ===='
-  for port_map in GetAllPhysicalPortToTTYMaps([plugable_7port],
+  for port_map in GetAllPhysicalPortToTTYMaps([usb_hubs.plugable_7port],
                                               device_tree_map=device_trees):
     print port_map
   print
 
   # Display serial number information about devices plugged into hubs.
   print '==== SERIAL NUMBER INFORMATION ===='
-  for port_map in GetAllPhysicalPortToSerialMaps([plugable_7port],
+  for port_map in GetAllPhysicalPortToSerialMaps([usb_hubs.plugable_7port],
                                                  device_tree_map=device_trees):
     print port_map
   print ''
   return 0
 
+def GetBattorPathFromPhoneSerial(serial, hub_types=None):
+  """Gets the BattOr path to communicate with the BattOr. This is the BattOr
+  that is connected to the same physical port number, on a different hub, that
+  the phone is connected to.
+
+  Args:
+    serial: Serial number of phone connected on the same physical port that
+    the BattOr is connected to.
+    hub_types: List of hub types to check for.
+
+  Returns:
+    Device string used to communicate with device.
+
+  Raises:
+    ValueError: If serial number is not given.
+    BattorError: If BattOr not found or unexpected USB topology.
+  """
+  if hub_types is None:
+    hub_types = ['plugable_7port'] #default: use plugable 7-port hub

[rnephew (Reviews Here), 2016/03/22 18:43:14]

nit: Space between # and comment.

[alexandermont, 2016/03/22 19:10:56]

Done

+  # If there's only one BattOr connected to the system, just use that one.
+  # This allows for use on, e.g., a developer's workstation with no hubs.
+  devtree = GetBusNumberToDeviceTreeMap(fast=True)
+  all_battors = GetBattorList(devtree)
+  hub_types = [usb_hubs.GetHubType(x) for x in hub_types]
+  if len(all_battors) == 1:
+    return '/dev/' + all_battors[0]
+  if serial:

[rnephew (Reviews Here), 2016/03/22 18:43:14]

I think it would be more readable if it was written as 

if not serial and len(all_battors) > 1:
  raise BattorError('Two or more BattOrs connected, no serial provided')

then just get rid of the else clause all together and move everything else up
one indentation level.

Just my opinion though, so if you and/or other feel otherwise feel free to
ignore this.

[alexandermont, 2016/03/22 19:10:56]

Done

+    p2serial = GetAllPhysicalPortToSerialMaps(hub_types,
+                                              device_tree_map=devtree)
+    p2tty = GetAllPhysicalPortToTTYMaps(hub_types,
+                                        device_tree_map=devtree)
+
+    # get the port number of this device
+    port_num = -1
+    for hub in p2serial:
+      for (port, s) in hub.iteritems():
+        if serial == s:
+          if port_num != -1:
+            raise BattorError('Two devices with same serial number.')
+          else:
+            port_num = port
+    if port_num == -1:
+      raise BattorError('Device with given serial number not found.')
+
+    # get the tty for this port number
+    tty_string = None
+    for hub in p2tty:
+      if port_num in hub:
+        if tty_string:
+          raise BattorError('Two TTY devices with matching port number.')
+        else:
+          tty_string = hub[port_num]
+    if not tty_string:
+      raise BattorError('No BattOr detected on corresponding port.')
+    if IsBattor(tty_string, devtree):
+      return '/dev/' + tty_string
+    else:
+      raise BattorError('Device connected to matching port is not BattOr.')
+  else:
+    raise BattorError('Two or more BattOrs connected, no serial provided')
+
 def parse_options(argv):
   """Parses and checks the command-line options.
 
@@ -622,6 +593,7 @@ def parse_options(argv):
 
 def main():
   parse_options(sys.argv)
+  print GetBattorPathFromPhoneSerial('test')

[rnephew (Reviews Here), 2016/03/22 18:43:14]

Guessing this is leftover testing.

[alexandermont, 2016/03/22 19:10:56]

Removed

   TestUSBTopologyScript()
 
 if __name__ == "__main__":