Bug 624001 Update android devicemanager with better error handling r=jmaher,mcote a=NPOTB
authorClint Talbert <ctalbert@mozilla.com>
Thu, 13 Jan 2011 12:00:23 -0600
changeset 60453 1511d70f1e4229732089b14b95b46626a93cdf46
parent 60452 bb4bbfbb6074f9df3549e8be592220096795c952
child 60454 46446fa861b5bf99eb84b0e8555422fdddaa7b68
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjmaher, mcote, NPOTB
bugs624001
milestone2.0b10pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 624001 Update android devicemanager with better error handling r=jmaher,mcote a=NPOTB
build/mobile/devicemanager.py
build/mobile/remoteautomation.py
testing/mochitest/runtestsremote.py
--- a/build/mobile/devicemanager.py
+++ b/build/mobile/devicemanager.py
@@ -16,16 +16,17 @@
 # The Initial Developer of the Original Code is Joel Maher.
 #
 # Portions created by the Initial Developer are Copyright (C) 2009
 # the Initial Developer. All Rights Reserved.
 #
 # Contributor(s):
 #   Joel Maher <joel.maher@gmail.com> (Original Developer)
 #   Clint Talbert <cmtalbert@gmail.com>
+#   Mark Cote <mcote@mozilla.com>
 #
 # Alternatively, the contents of this file may be used under the terms of
 # either the GNU General Public License Version 2 or later (the "GPL"), or
 # the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
 # in which case the provisions of the GPL or the LGPL are applicable instead
 # of those above. If you wish to allow use of your version of this file only
 # under the terms of either the GPL or the LGPL, and not to allow others to
 # use your version of this file under the terms of the MPL, indicate your
@@ -51,33 +52,50 @@ class FileError(Exception):
   " Signifies an error which occurs while doing a file operation."
 
   def __init__(self, msg = ''):
     self.msg = msg
 
   def __str__(self):
     return self.msg
 
+class DMError(Exception):
+  "generic devicemanager exception."
+
+  def __init__(self, msg= ''):
+    self.msg = msg
+
+  def __str__(self):
+    return self.msg
+
+
 class DeviceManager:
   host = ''
   port = 0
   debug = 2 
-  _redo = False
-  deviceRoot = None
+  retries = 0
   tempRoot = os.getcwd()
   base_prompt = '$>'
   base_prompt_re = '\$\>'
   prompt_sep = '\x00'
   prompt_regex = '.*(' + base_prompt_re + prompt_sep + ')'
   agentErrorRE = re.compile('^##AGENT-WARNING##.*')
 
+  # TODO: member variable to indicate error conditions.
+  # This should be set to a standard error from the errno module.
+  # So, for example, when an error occurs because of a missing file/directory,
+  # before returning, the function would do something like 'self.error = errno.ENOENT'.
+  # The error would be set where appropriate--so sendCMD() could set socket errors,
+  # pushFile() and other file-related commands could set filesystem errors, etc.
 
-  def __init__(self, host, port = 20701):
+  def __init__(self, host, port = 20701, retrylimit = 5):
     self.host = host
     self.port = port
+    self.retrylimit = retrylimit
+    self.retries = 0
     self._sock = None
     self.getDeviceRoot()
 
   def cmdNeedsResponse(self, cmd):
     """ Not all commands need a response from the agent:
         * if the cmd matches the pushRE then it is the first half of push
           and therefore we want to wait until the second half before looking
           for a response
@@ -111,79 +129,105 @@ class DeviceManager:
                          re.compile('^uninst .*$')]
 
     for c in socketClosingCmds:
       if (c.match(cmd)):
         return True
 
     return False
 
-  def sendCMD(self, cmdline, newline = True):
+  # convenience function to enable checks for agent errors
+  def verifySendCMD(self, cmdline, newline = True):
+    return self.sendCMD(cmdline, newline, False)
+
+
+  #
+  # create a wrapper for sendCMD that loops up to self.retrylimit iterations.
+  # this allows us to move the retry logic outside of the _doCMD() to make it 
+  # easier for debugging in the future.
+  # note that since cmdline is a list of commands, they will all be retried if
+  # one fails.  this is necessary in particular for pushFile(), where we don't want
+  # to accidentally send extra data if a failure occurs during data transmission.
+  #
+  def sendCMD(self, cmdline, newline = True, ignoreAgentErrors = True):
+    done = False
+    while (not done):
+      retVal = self._doCMD(cmdline, newline)
+      if (retVal is None):
+        self.retries += 1
+      else:
+        self.retries = 0
+        if ignoreAgentErrors == False:
+          if (self.agentErrorRE.match(retVal)):
+            raise DMError("error on the agent executing '%s'" % cmdline)
+        return retVal
+
+      if (self.retries >= self.retrylimit):
+        done = True
+
+    raise DMError("unable to connect to %s after %s attempts" % (self.host, self.retrylimit))        
+
+  def _doCMD(self, cmdline, newline = True):
     promptre = re.compile(self.prompt_regex + '$')
     data = ""
     shouldCloseSocket = False
     recvGuard = 1000
 
     if (self._sock == None):
       try:
         if (self.debug >= 1):
           print "reconnecting socket"
         self._sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
       except:
-        self._redo = True
         self._sock = None
         if (self.debug >= 2):
           print "unable to create socket"
         return None
       
       try:
         self._sock.connect((self.host, int(self.port)))
         self._sock.recv(1024)
       except:
-        self._redo = True
         self._sock.close()
         self._sock = None
         if (self.debug >= 2):
           print "unable to connect socket"
         return None
     
     for cmd in cmdline:
       if newline: cmd += '\r\n'
       
       try:
         numbytes = self._sock.send(cmd)
         if (numbytes != len(cmd)):
           print "ERROR: our cmd was " + str(len(cmd)) + " bytes and we only sent " + str(numbytes)
           return None
         if (self.debug >= 4): print "send cmd: " + str(cmd)
       except:
-        self._redo = True
         self._sock.close()
         self._sock = None
         return None
       
       # Check if the command should close the socket
       shouldCloseSocket = self.shouldCmdCloseSocket(cmd)
 
       # Handle responses from commands
       if (self.cmdNeedsResponse(cmd)):
         found = False
         loopguard = 0
-        # TODO: We had an old sleep here but we don't need it
 
         while (found == False and (loopguard < recvGuard)):
           temp = ''
           if (self.debug >= 4): print "recv'ing..."
 
           # Get our response
           try:
             temp = self._sock.recv(1024)
             if (self.debug >= 4): print "response: " + str(temp)
           except:
-            self._redo = True
             self._sock.close()
             self._sock = None
             return None
 
           # If something goes wrong in the agent it will send back a string that
           # starts with '##AGENT-ERROR##'
           if (self.agentErrorRE.match(temp)):
             data = temp
@@ -195,30 +239,28 @@ class DeviceManager:
             if (promptre.match(line)):
               found = True
           data += temp
 
           # If we violently lose the connection to the device, this loop tends to spin,
           # this guard prevents that
           if (temp == ''):
             loopguard += 1
-            
-    # TODO: We had an old sleep here but we don't need it
+
     if (shouldCloseSocket == True):
       try:
         self._sock.close()
         self._sock = None
       except:
-        self._redo = True
         self._sock = None
         return None
 
     return data
   
-  
+  # internal function
   # take a data blob and strip instances of the prompt '$>\x00'
   def stripPrompt(self, data):
     promptre = re.compile(self.prompt_regex + '.*')
     retVal = []
     lines = data.split('\n')
     for line in lines:
       try:
         while (promptre.match(line)):
@@ -228,279 +270,361 @@ class DeviceManager:
           line = self.prompt_sep.join(pieces)
       except(ValueError):
         pass
       retVal.append(line)
 
     return '\n'.join(retVal)
   
 
+  # external function
+  # returns:
+  #  success: True
+  #  failure: False
   def pushFile(self, localname, destname):
     if (self.debug >= 3): print "in push file with: " + localname + ", and: " + destname
     if (self.validateFile(destname, localname) == True):
       if (self.debug >= 3): print "files are validated"
-      return ''
+      return True
 
     if self.mkDirs(destname) == None:
       print "unable to make dirs: " + destname
-      return None
+      return False
 
     if (self.debug >= 3): print "sending: push " + destname
     
     filesize = os.path.getsize(localname)
     f = open(localname, 'rb')
     data = f.read()
     f.close()
-    retVal = self.sendCMD(['push ' + destname + ' ' + str(filesize) + '\r\n', data], newline = False)
-    
+
+    try:
+      retVal = self.verifySendCMD(['push ' + destname + ' ' + str(filesize) + '\r\n', data], newline = False)
+    except(DMError):
+      retVal = False
+  
     if (self.debug >= 3): print "push returned: " + str(retVal)
 
     validated = False
     if (retVal):
       retline = self.stripPrompt(retVal).strip() 
-      if (retline == None or self.agentErrorRE.match(retVal)):
+      if (retline == None):
         # Then we failed to get back a hash from agent, try manual validation
         validated = self.validateFile(destname, localname)
       else:
         # Then we obtained a hash from push
         localHash = self.getLocalHash(localname)
         if (str(localHash) == str(retline)):
           validated = True
     else:
       # We got nothing back from sendCMD, try manual validation
       validated = self.validateFile(destname, localname)
 
     if (validated):
       if (self.debug >= 3): print "Push File Validated!"
       return True
     else:
       if (self.debug >= 2): print "Push File Failed to Validate!"
-      return None
+      return False
   
+  # external function
+  # returns:
+  #  success: directory name
+  #  failure: None
   def mkDir(self, name):
     if (self.dirExists(name)):
       return name
     else:
-      return self.sendCMD(['mkdr ' + name])
-  
+      try:
+        retVal = self.verifySendCMD(['mkdr ' + name])
+      except(DMError):
+        retVal = None
+      return retVal
+
   # make directory structure on the device
+  # external function
+  # returns:
+  #  success: directory structure that we created
+  #  failure: None
   def mkDirs(self, filename):
     parts = filename.split('/')
     name = ""
     for part in parts:
       if (part == parts[-1]): break
       if (part != ""):
         name += '/' + part
         if (self.mkDir(name) == None):
           print "failed making directory: " + str(name)
           return None
-    return ''
+    return name
 
   # push localDir from host to remoteDir on the device
+  # external function
+  # returns:
+  #  success: remoteDir
+  #  failure: None
   def pushDir(self, localDir, remoteDir):
-    if (self.debug >= 2): print "pushing directory: " + localDir + " to " + remoteDir
+    if (self.debug >= 2): print "pushing directory: %s to %s" % (localDir, remoteDir)
     for root, dirs, files in os.walk(localDir):
       parts = root.split(localDir)
       for file in files:
         remoteRoot = remoteDir + '/' + parts[1]
         remoteName = remoteRoot + '/' + file
         if (parts[1] == ""): remoteRoot = remoteDir
-        if (self.pushFile(os.path.join(root, file), remoteName) == None):
+        if (self.pushFile(os.path.join(root, file), remoteName) == False):
+          # retry once
           self.removeFile(remoteName)
-          if (self.pushFile(os.path.join(root, file), remoteName) == None):
+          if (self.pushFile(os.path.join(root, file), remoteName) == False):
             return None
-    return True
+    return remoteDir
 
+  # external function
+  # returns:
+  #  success: True
+  #  failure: False
   def dirExists(self, dirname):
     match = ".*" + dirname + "$"
     dirre = re.compile(match)
-    data = self.sendCMD(['cd ' + dirname, 'cwd'])
-    # Because this is a compound command, cd can fail while cwd can succeed, 
-    # we should check for agent error directly
-    if (data == None or self.agentErrorRE.match(data) ):
-      return None
+    try:
+      data = self.verifySendCMD(['cd ' + dirname, 'cwd'])
+    except(DMError):
+      return False
+
     retVal = self.stripPrompt(data)
     data = retVal.split('\n')
     found = False
     for d in data:
       if (dirre.match(d)): 
         found = True
 
     return found
 
   # Because we always have / style paths we make this a lot easier with some
   # assumptions
+  # external function
+  # returns:
+  #  success: True
+  #  failure: False
   def fileExists(self, filepath):
     s = filepath.split('/')
     containingpath = '/'.join(s[:-1])
     listfiles = self.listFiles(containingpath)
     for f in listfiles:
       if (f == s[-1]):
         return True
     return False
 
   # list files on the device, requires cd to directory first
+  # external function
+  # returns:
+  #  success: array of filenames, ['file1', 'file2', ...]
+  #  failure: []
   def listFiles(self, rootdir):
     rootdir = rootdir.rstrip('/')
     if (self.dirExists(rootdir) == False):
-      return []  
-    data = self.sendCMD(['cd ' + rootdir, 'ls'])
-    if (data == None):
-      return None
+      return []
+    try:
+      data = self.verifySendCMD(['cd ' + rootdir, 'ls'])
+    except(DMError):
+      return []
+
     retVal = self.stripPrompt(data)
     files = filter(lambda x: x, retVal.split('\n'))
     if len(files) == 1 and files[0] == '<empty>':
       # special case on the agent: empty directories return just the string "<empty>"
       return []
     return files
 
+  # external function
+  # returns:
+  #  success: output of telnet, i.e. "removing file: /mnt/sdcard/tests/test.txt"
+  #  failure: None
   def removeFile(self, filename):
     if (self.debug>= 2): print "removing file: " + filename
-    return self.sendCMD(['rm ' + filename])
-    
-  # does a recursive delete of directory on the device: rm -Rf remoteDir
-  def removeDir(self, remoteDir):
-    self.sendCMD(['rmdr ' + remoteDir])
+    try:
+      retVal = self.verifySendCMD(['rm ' + filename])
+    except(DMError):
+      return None
 
+    return retVal
+  
+  # does a recursive delete of directory on the device: rm -Rf remoteDir
+  # external function
+  # returns:
+  #  success: output of telnet, i.e. "removing file: /mnt/sdcard/tests/test.txt"
+  #  failure: None
+  def removeDir(self, remoteDir):
+    try:
+      retVal = self.verifySendCMD(['rmdr ' + remoteDir])
+    except(DMError):
+      return None
+
+    return retVal
+
+  # external function
+  # returns:
+  #  success: array of process tuples
+  #  failure: []
   def getProcessList(self):
-    data = self.sendCMD(['ps'])
-    if (data == None):
-      return None
-      
+    try:
+      data = self.verifySendCMD(['ps'])
+    except DMError:
+      return []
+
     retVal = self.stripPrompt(data)
     lines = retVal.split('\n')
     files = []
     for line in lines:
       if (line.strip() != ''):
         pidproc = line.strip().split()
         if (len(pidproc) == 2):
           files += [[pidproc[0], pidproc[1]]]
         elif (len(pidproc) == 3):
           #android returns <userID> <procID> <procName>
           files += [[pidproc[1], pidproc[2], pidproc[0]]]     
     return files
 
-  def getMemInfo(self):
-    data = self.sendCMD(['mems'])
-    if (data == None):
-      return None
-    retVal = self.stripPrompt(data)
-    # TODO: this is hardcoded for now
-    fhandle = open("memlog.txt", 'a')
-    fhandle.write("\n")
-    fhandle.write(retVal)
-    fhandle.close()
-
+  # external function
+  # returns:
+  #  success: pid
+  #  failure: None
   def fireProcess(self, appname):
     if (self.debug >= 2): print "FIRE PROC: '" + appname + "'"
     
-    if (self.processExist(appname) != ''):
+    if (self.processExist(appname) != None):
       print "WARNING: process %s appears to be running already\n" % appname
     
-    self.sendCMD(['exec ' + appname])
+    try:
+      data = self.verifySendCMD(['exec ' + appname])
+    except(DMError):
+      return None
 
-    #NOTE: we sleep for 30 seconds to allow the application to startup
-    time.sleep(30)
+    # wait up to 30 seconds for process to start up
+    timeslept = 0
+    while (timeslept <= 30):
+      process = self.processExist(appname)
+      if (self.process is not None):
+        break
+      time.sleep(3)
+      timeslept += 3
 
-    self.process = self.processExist(appname)
-    if (self.debug >= 4): print "got pid: " + str(self.process) + " for process: " + str(appname)
+    if (self.debug >= 4): print "got pid: %s for process: %s" % (process, appname)
+    return process
 
+  # external function
+  # returns:
+  #  success: output filename
+  #  failure: None
   def launchProcess(self, cmd, outputFile = "process.txt", cwd = '', env = ''):
     cmdline = subprocess.list2cmdline(cmd)
     if (outputFile == "process.txt" or outputFile == None):
-      outputFile = self.getDeviceRoot() + '/' + "process.txt"
+      outputFile = self.getDeviceRoot();
+      if outputFile is None:
+        return None
+      outputFile += "/process.txt"
       cmdline += " > " + outputFile
     
     # Prepend our env to the command 
-    cmdline = ('%s ' % self.formatEnvString(env)) + cmdline
+    cmdline = '%s %s' % (self.formatEnvString(env), cmdline)
 
-    self.fireProcess(cmdline)
+    if self.fireProcess(cmdline) is None:
+      return None
     return outputFile
   
-  #hardcoded: sleep interval of 5 seconds, timeout of 10 minutes
-  def communicate(self, process, timeout = 600):
-    interval = 5
+  # loops until 'process' has exited or 'timeout' seconds is reached
+  # loop sleeps for 'interval' seconds between iterations
+  # external function
+  # returns:
+  #  success: [file contents, None]
+  #  failure: [None, None]
+  def communicate(self, process, timeout = 600, interval = 5):
     timed_out = True
     if (timeout > 0):
       total_time = 0
       while total_time < timeout:
         time.sleep(interval)
-        if (not self.poll(process)):
+        if self.processExist(process) == None:
           timed_out = False
           break
         total_time += interval
 
     if (timed_out == True):
-      return None
+      return [None, None]
 
     return [self.getFile(process, "temp.txt"), None]
 
-
-  def poll(self, process):
-    try:
-      if (self.processExist(process) == ''):
-        return None
-      return 1
-    except:
-      return None
-    return 1
-  
-  # iterates process list and returns pid if exists, otherwise ''
+  # iterates process list and returns pid if exists, otherwise None
+  # external function
+  # returns:
+  #  success: pid
+  #  failure: None
   def processExist(self, appname):
-    pid = ''
+    pid = None
 
     #remove the environment variables in the cli if they exist
-    parts = appname.split(' ')
-    for p in parts:
-      if (p is ''):
-        parts.remove(p)
+    parts = filter(lambda x: x != '', appname.split(' '))
 
     if len(parts[0].strip('"').split('=')) > 1:
-      envvars = parts[0].strip('"').split(',')
-      for e in envvars:
-        env = e.split('=')
-        if (len(env) > 1):
-          os.environ[env[0]] = str(env[1])
       appname = ' '.join(parts[1:])
-
   
     pieces = appname.split(' ')
     parts = pieces[0].split('/')
     app = parts[-1]
     procre = re.compile('.*' + app + '.*')
 
     procList = self.getProcessList()
-    if (procList == None):
+    if (procList == []):
       return None
       
     for proc in procList:
       if (procre.match(proc[1])):
         pid = proc[0]
         break
     return pid
 
+  # external function
+  # returns:
+  #  success: output from testagent
+  #  failure: None
   def killProcess(self, appname):
-    if (self.sendCMD(['kill ' + appname]) == None):
+    try:
+      data = self.verifySendCMD(['kill ' + appname])
+    except(DMError):
       return None
 
-    return True
+    return data
 
+  # external function
+  # returns:
+  #  success: tmpdir, string
+  #  failure: None
   def getTempDir(self):
-    retVal = ''
-    data = self.sendCMD(['tmpd'])
-    if (data == None):
+    try:
+      data = self.verifySendCMD(['tmpd'])
+    except(DMError):
       return None
+
     return self.stripPrompt(data).strip('\n')
 
+  # external function
+  # returns:
+  #  success: filecontents
+  #  failure: None
   def catFile(self, remoteFile):
-    data = self.sendCMD(['cat ' + remoteFile])
-    if data == None:
-        return None
+    try:
+      data = self.verifySendCMD(['cat ' + remoteFile])
+    except(DMError):
+      return None
+
     return self.stripPrompt(data)
   
+  # external function
+  # returns:
+  #  success: output of pullfile, string
+  #  failure: None
   def pullFile(self, remoteFile):
     """Returns contents of remoteFile using the "pull" command.
     The "pull" command is different from other commands in that DeviceManager
     has to read a certain number of bytes instead of just reading to the
     next prompt.  This is more robust than the "cat" command, which will be
     confused if the prompt string exists within the file being catted.
     However it means we can't use the response-handling logic in sendCMD().
     """
@@ -512,21 +636,25 @@ class DeviceManager:
         raise FileError(err_str) 
 
     # FIXME: We could possibly move these socket-reading functions up to
     # the class level if we wanted to refactor sendCMD().  For now they are
     # only used to pull files.
     
     def uread(to_recv, error_msg):
       """ unbuffered read """
-      data = self._sock.recv(to_recv)
-      if not data:
+      try:
+        data = self._sock.recv(to_recv)
+        if not data:
+          err(error_msg)
+          return None
+        return data
+      except:
         err(error_msg)
         return None
-      return data
 
     def read_until_char(c, buffer, error_msg):
       """ read until 'c' is found; buffer rest """
       while not '\n' in buffer:
         data = uread(1024, error_msg)
         if data == None:
           err(error_msg)
           return ('', '', '')
@@ -545,17 +673,21 @@ class DeviceManager:
 
     prompt = self.base_prompt + self.prompt_sep
     buffer = ''
     
     # expected return value:
     # <filename>,<filesize>\n<filedata>
     # or, if error,
     # <filename>,-1\n<error message>
-    self.sendCMD(['pull ' + remoteFile])
+    try:
+      data = self.verifySendCMD(['pull ' + remoteFile])
+    except(DMError):
+      return None
+
     # read metadata; buffer the rest
     metadata, sep, buffer = read_until_char('\n', buffer, 'could not find metadata')
     if not metadata:
       return None
     if self.debug >= 3:
       print 'metadata: %s' % metadata
 
     filename, sep, filesizestr = metadata.partition(',')
@@ -584,41 +716,50 @@ class DeviceManager:
     if buffer == None:
       return None
     if buffer[-len(prompt):] != prompt:
       err('no prompt found after file data--DeviceManager may be out of sync with agent')
       return buffer
     return buffer[:-len(prompt)]
 
   # copy file from device (remoteFile) to host (localFile)
+  # external function
+  # returns:
+  #  success: output of pullfile, string
+  #  failure: None
   def getFile(self, remoteFile, localFile = ''):
     if localFile == '':
       localFile = os.path.join(self.tempRoot, "temp.txt")
   
     retVal = self.pullFile(remoteFile)
-    if retVal == None:
+    if (retVal is None):
       return None
+
     fhandle = open(localFile, 'wb')
     fhandle.write(retVal)
     fhandle.close()
     if not self.validateFile(remoteFile, localFile):
       print 'failed to validate file when downloading %s!' % remoteFile
       return None
     return retVal
-    
+
   # copy directory structure from device (remoteDir) to host (localDir)
+  # external function
+  # returns:
+  #  success: list of files, string
+  #  failure: None
   def getDirectory(self, remoteDir, localDir):
     if (self.debug >= 2): print "getting files in '" + remoteDir + "'"
     filelist = self.listFiles(remoteDir)
-    if (filelist == None):
+    if (filelist == []):
       return None
     if (self.debug >= 3): print filelist
     if not os.path.exists(localDir):
       os.makedirs(localDir)
-   
+
     for f in filelist:
       if f == '.' or f == '..':
         continue
       remotePath = remoteDir + '/' + f
       localPath = os.path.join(localDir, f)
       try:
         is_dir = self.isDir(remotePath)
       except FileError:
@@ -632,45 +773,71 @@ class DeviceManager:
         # It's sometimes acceptable to have getFile() return None, such as
         # when the agent encounters broken symlinks.
         # FIXME: This should be improved so we know when a file transfer really
         # failed.
         if self.getFile(remotePath, localPath) == None:
           print 'failed to get file "%s"; continuing anyway...' % remotePath 
     return filelist
 
+  # external function
+  # returns:
+  #  success: True
+  #  failure: False
+  #  Throws a FileError exception when null (invalid dir/filename)
   def isDir(self, remotePath):
-    data = self.sendCMD(['isdir ' + remotePath])
+    try:
+      data = self.verifySendCMD(['isdir ' + remotePath])
+    except(DMError):
+      data = None
+
     retVal = self.stripPrompt(data).strip()
     if not retVal:
       raise FileError('isdir returned null')
     return retVal == 'TRUE'
 
   # true/false check if the two files have the same md5 sum
+  # external function
+  # returns:
+  #  success: True
+  #  failure: False
   def validateFile(self, remoteFile, localFile):
     remoteHash = self.getRemoteHash(remoteFile)
     localHash = self.getLocalHash(localFile)
 
+    if (remoteHash == None):
+      return False
+
     if (remoteHash == localHash):
       return True
 
     return False
   
   # return the md5 sum of a remote file
+  # internal function
+  # returns:
+  #  success: MD5 hash for given filename
+  #  failure: None
   def getRemoteHash(self, filename):
-    data = self.sendCMD(['hash ' + filename])
-    if (data == None):
-        return ''
+    try:
+      data = self.verifySendCMD(['hash ' + filename])
+    except(DMError):
+      return None
+
     retVal = self.stripPrompt(data)
     if (retVal != None):
       retVal = retVal.strip('\n')
     if (self.debug >= 3): print "remote hash returned: '" + retVal + "'"
     return retVal
     
   # return the md5 sum of a file on the host
+  # internal function
+  # returns:
+  #  success: MD5 hash for given filename
+  #  failure: None
   def getLocalHash(self, filename):
     file = open(filename, 'rb')
     if (file == None):
       return None
 
     try:
       mdsum = hashlib.md5()
     except:
@@ -694,142 +861,188 @@ class DeviceManager:
   # that returned path.
   # Structure on the device is as follows:
   # /tests
   #       /<fennec>|<firefox>  --> approot
   #       /profile
   #       /xpcshell
   #       /reftest
   #       /mochitest
+  #
+  # external function
+  # returns:
+  #  success: path for device root
+  #  failure: None
   def getDeviceRoot(self):
-    # This caching of deviceRoot is causing issues if things fail
-    # if (not self.deviceRoot):
-    data = self.sendCMD(['testroot'])
-    if (data == None):
-      return '/tests'
-    self.deviceRoot = self.stripPrompt(data).strip('\n') + '/tests'
+    try:
+      data = self.verifySendCMD(['testroot'])
+    except:
+      return None
+  
+    deviceRoot = self.stripPrompt(data).strip('\n') + '/tests'
 
-    if (not self.dirExists(self.deviceRoot)):
-      self.mkDir(self.deviceRoot)
+    if (not self.dirExists(deviceRoot)):
+      if (self.mkDir(deviceRoot) == None):
+        return None
 
-    return self.deviceRoot
+    return deviceRoot
 
   # Either we will have /tests/fennec or /tests/firefox but we will never have
   # both.  Return the one that exists
+  # TODO: ensure we can support org.mozilla.firefox
+  # external function
+  # returns:
+  #  success: path for app root
+  #  failure: None
   def getAppRoot(self):
-    if (self.dirExists(self.getDeviceRoot() + '/fennec')):
-      return self.getDeviceRoot() + '/fennec'
-    elif (self.dirExists(self.getDeviceRoot() + '/firefox')):
-      return self.getDeviceRoot() + '/firefox'
-    else:
+    devroot = self.getDeviceRoot()
+    if (devroot == None):
+      return None
+
+    if (self.dirExists(devroot + '/fennec')):
+      return devroot + '/fennec'
+    elif (self.dirExists(devroot + '/firefox')):
+      return devroot + '/firefox'
+    elif (self.dirExsts('/data/data/org.mozilla.fennec')):
       return 'org.mozilla.fennec'
+    elif (self.dirExists('/data/data/org.mozilla.firefox')):
+      return 'org.mozilla.firefox'
+
+    # Failure (either not installed or not a recognized platform)
+    return None
 
   # Gets the directory location on the device for a specific test type
   # Type is one of: xpcshell|reftest|mochitest
+  # external function
+  # returns:
+  #  success: path for test root
+  #  failure: None
   def getTestRoot(self, type):
+    devroot = self.getDeviceRoot()
+    if (devroot == None):
+      return None
+
     if (re.search('xpcshell', type, re.I)):
-      self.testRoot = self.getDeviceRoot() + '/xpcshell'
+      self.testRoot = devroot + '/xpcshell'
     elif (re.search('?(i)reftest', type)):
-      self.testRoot = self.getDeviceRoot() + '/reftest'
+      self.testRoot = devroot + '/reftest'
     elif (re.search('?(i)mochitest', type)):
-      self.testRoot = self.getDeviceRoot() + '/mochitest'
+      self.testRoot = devroot + '/mochitest'
     return self.testRoot
 
   # Sends a specific process ID a signal code and action.
   # For Example: SIGINT and SIGDFL to process x
   def signal(self, processID, signalType, signalAction):
     # currently not implemented in device agent - todo
     pass
 
   # Get a return code from process ending -- needs support on device-agent
-  # this is a todo
   def getReturnCode(self, processID):
-    # todo make this real
+    # TODO: make this real
     return 0
 
+  # external function
+  # returns:
+  #  success: output of unzip command
+  #  failure: None
   def unpackFile(self, filename):
+    devroot = self.getDeviceRoot()
+    if (devroot == None):
+      return None
+
     dir = ''
     parts = filename.split('/')
     if (len(parts) > 1):
       if self.fileExists(filename):
         dir = '/'.join(parts[:-1])
     elif self.fileExists('/' + filename):
       dir = '/' + filename
-    elif self.fileExists(self.getDeviceRoot() + '/' + filename):
-      dir = self.getDeviceRoot() + '/' + filename
+    elif self.fileExists(devroot + '/' + filename):
+      dir = devroot + '/' + filename
     else:
       return None
 
-    return self.sendCMD(['cd ' + dir, 'unzp ' + filename])
+    try:
+      data = self.verifySendCMD(['cd ' + dir, 'unzp ' + filename])
+    except(DMError):
+      return None
 
-  def reboot(self, wait = False):
-    self.sendCMD(['rebt'])
+    return data
 
-    if wait == True:
-      time.sleep(30)
-      timeout = 270
-      done = False
-      while (not done):
-        if self.listFiles('/') != None:
-          return ''
-        print "sleeping another 10 seconds"
-        time.sleep(10)
-        timeout = timeout - 10
-        if (timeout <= 0):
-          return None
-    return ''
+  # external function
+  # returns:
+  #  success: status from test agent
+  #  failure: None
+  def reboot(self):
+    cmd = 'rebt'
+
+    if (self.debug > 3): print "INFO: sending rebt command"
+    
+    try:
+      status = self.verifySendCMD([cmd])
+    except DMError:
+      return None
+
+    if (self.debug > 3): print "INFO: rebt- got status back: " + str(status)
+    return status
 
   # validate localDir from host to remoteDir on the device
+  # external function
+  # returns:
+  #  success: True
+  #  failure: False
   def validateDir(self, localDir, remoteDir):
     if (self.debug >= 2): print "validating directory: " + localDir + " to " + remoteDir
     for root, dirs, files in os.walk(localDir):
       parts = root.split(localDir)
       for file in files:
         remoteRoot = remoteDir + '/' + parts[1]
         remoteRoot = remoteRoot.replace('/', '/')
         if (parts[1] == ""): remoteRoot = remoteDir
         remoteName = remoteRoot + '/' + file
         if (self.validateFile(remoteName, os.path.join(root, file)) <> True):
-            return None
+            return False
     return True
 
   # Returns information about the device:
   # Directive indicates the information you want to get, your choices are:
   # os - name of the os
   # id - unique id of the device
   # uptime - uptime of the device
   # systime - system time of the device
   # screen - screen resolution
   # memory - memory stats
   # process - list of running processes (same as ps)
   # disk - total, free, available bytes on disk
   # power - power status (charge, battery temp)
   # all - all of them - or call it with no parameters to get all the information
+  # returns:
+  #   success: dict of info strings by directive name
+  #   failure: {}
   def getInfo(self, directive=None):
     data = None
     result = {}
     collapseSpaces = re.compile('  +')
 
     directives = ['os', 'id','uptime','systime','screen','memory','process',
                   'disk','power']
     if (directive in directives):
       directives = [directive]
 
     for d in directives:
-      data = self.sendCMD(['info ' + d])
+      data = self.verifySendCMD(['info ' + d])
       if (data is None):
         continue
       data = self.stripPrompt(data)
       data = collapseSpaces.sub(' ', data)
       result[d] = data.split('\n')
 
     # Get rid of any 0 length members of the arrays
-    for v in result.itervalues():
-      while '' in v:
-        v.remove('')
+    for k, v in result.iteritems():
+      result[k] = filter(lambda x: x != '', result[k])
     
     # Format the process output
     if 'process' in result:
       proclist = []
       for l in result['process']:
         if l:
           proclist.append(l.split('\t'))
       result['process'] = proclist
@@ -838,100 +1051,121 @@ class DeviceManager:
     return result
 
   """
   Installs the application onto the device
   Application bundle - path to the application bundle on the device
   Destination - destination directory of where application should be
                 installed to (optional)
   Returns None for success, or output if known failure
-  TODO: we need a better way to know if this works or not
   """
+  # external function
+  # returns:
+  #  success: output from agent for inst command
+  #  failure: None
   def installApp(self, appBundlePath, destPath=None):
     cmd = 'inst ' + appBundlePath
     if destPath:
       cmd += ' ' + destPath
-    data = self.sendCMD([cmd])
-    if (data is None):
+    try:
+      data = self.verifySendCMD([cmd])
+    except(DMError):
       return None
-    
+
     f = re.compile('Failure')
     for line in data.split():
       if (f.match(line)):
         return data
     return None
 
   """
   Uninstalls the named application from device and causes a reboot.
   Takes an optional argument of installation path - the path to where the application
   was installed.
   Returns True, but it doesn't mean anything other than the command was sent,
   the reboot happens and we don't know if this succeeds or not.
   """
+  # external function
+  # returns:
+  #  success: True
+  #  failure: None
   def uninstallAppAndReboot(self, appName, installPath=None):
     cmd = 'uninst ' + appName
     if installPath:
       cmd += ' ' + installPath
-    data = self.sendCMD([cmd])
+    try:
+      data = self.verifySendCMD([cmd])
+    except(DMError):
+      return None
+
     if (self.debug > 3): print "uninstallAppAndReboot: " + str(data)
     return True
 
   """
   Updates the application on the device.
   Application bundle - path to the application bundle on the device
   Process name of application - used to end the process if the applicaiton is
                                 currently running
   Destination - Destination directory to where the application should be
                 installed (optional)
   ipAddr - IP address to await a callback ping to let us know that the device has updated
            properly - defaults to current IP.
   port - port to await a callback ping to let us know that the device has updated properly
          defaults to 30000, and counts up from there if it finds a conflict
   Returns True if succeeds, False if not
-  
-  NOTE: We have no real way to know if the device gets updated or not due to the
-        reboot that the udpate call forces on us.  We can't install our own heartbeat
-        listener here because we run the risk of racing with other heartbeat listeners.
   """
+  # external function
+  # returns:
+  #  success: text status from command or callback server
+  #  failure: None
   def updateApp(self, appBundlePath, processName=None, destPath=None, ipAddr=None, port=30000):
     status = None
     cmd = 'updt '
     if (processName == None):
       # Then we pass '' for processName
       cmd += "'' " + appBundlePath
     else:
       cmd += processName + ' ' + appBundlePath
 
     if (destPath):
       cmd += " " + destPath
 
-    if (self.debug > 3): print "updateApp using command: " + str(cmd)
+    if (self.debug > 3): print "INFO: updateApp using command: " + str(cmd)
 
     if (ipAddr is not None):
       ip, port = self.getCallbackIpAndPort(ipAddr, port)
-
       cmd += " %s %s" % (ip, port)
-
       # Set up our callback server
       callbacksvr = callbackServer(ip, port, self.debug)
-      data = self.sendCMD([cmd])
+
+    try:
+      status = self.verifySendCMD([cmd])
+    except(DMError):
+      return None
+
+    if ipAddr is not None:
       status = callbacksvr.disconnect()
-      if (self.debug > 3): print "got status back: " + str(status)
-    else:
-      status = self.sendCMD([cmd])
+
+    if (self.debug > 3): print "INFO: updateApp: got status back: " + str(status)
 
     return status
 
   """
     return the current time on the device
   """
+  # external function
+  # returns:
+  #  success: time in ms
+  #  failure: None
   def getCurrentTime(self):
-    data = self.sendCMD(['clok'])
-    if (data == None):
+    try:
+      data = self.verifySendCMD(['clok'])
+    except(DMError):
       return None
+
     return self.stripPrompt(data).strip('\n')
 
   """
     Connect the ipaddress and port for a callback ping.  Defaults to current IP address
     And ports starting at 30000.
     NOTE: the detection for current IP address only works on Linux!
   """
   def getCallbackIpAndPort(self, aIp, aPort):
@@ -950,37 +1184,31 @@ class DeviceManager:
     Input - env, which is either None, '', or a dict
     Output - a quoted string of the form: '"envvar1=val1,envvar2=val2..."'
     If env is None or '' return '""' (empty quoted string)
   """
   def formatEnvString(self, env):
     if (env == None or env == ''):
       return '""'
 
-    envstr = '"'
-    # TODO: I believe this is inefficient for large dicts
-    for k, v in env.items():
-      envstr += ('%s=%s,' % (k, v))
-    
-    # kill the trailing comma, add the last quote
-    envstr = envstr.rstrip(',')
-    envstr += '"'
-
-    return envstr
+    return '"%s"' % ','.join(map(lambda x: '%s=%s' % (x[0], x[1]), env.iteritems()))
 
 gCallbackData = ''
 
+class myServer(SocketServer.TCPServer):
+  allow_reuse_address = True
+
 class callbackServer():
   def __init__(self, ip, port, debuglevel):
     self.ip = ip
     self.port = port
     self.connected = False
     self.debug = debuglevel
     if (self.debug > 3) : print "Creating server with " + str(ip) + ":" + str(port)
-    self.server = SocketServer.TCPServer((ip, port), self.myhandler)
+    self.server = myServer((ip, port), self.myhandler)
     self.server_thread = Thread(target=self.server.serve_forever) 
     self.server_thread.setDaemon(True)
     self.server_thread.start()
 
   def disconnect(self, step = 60, timeout = 600):
     t = 0
     if (self.debug > 3): print "Calling disconnect on callback server"
     while t < timeout:
@@ -1044,18 +1272,19 @@ class NetworkTools:
       if isinstance(seed, basestring):
         seed = int(seed)
       maxportnum = seed + 5000 # We will try at most 5000 ports to find an open one
       while not connected:
         try:
           s.bind((ip, seed))
           connected = True
           s.close()
+          break
         except:          
           if seed > maxportnum:
             print "Could not find open port after checking 5000 ports"
           raise
         seed += 1
     except:
       print "Socket error trying to find open port"
         
     return seed
-    
+
--- a/build/mobile/remoteautomation.py
+++ b/build/mobile/remoteautomation.py
@@ -137,17 +137,17 @@ class RemoteAutomation(Automation):
 
             # Setting timeout at 1 hour since on a remote device this takes much longer
             self.timeout = 3600
             time.sleep(15)
 
         @property
         def pid(self):
             hexpid = self.dm.processExist(self.procName)
-            if (hexpid == '' or hexpid == None):
+            if (hexpid == None):
                 hexpid = "0x0"
             return int(hexpid, 0)
     
         @property
         def stdout(self):
             return self.dm.getFile(self.proc)
  
         def wait(self, timeout = None):
--- a/testing/mochitest/runtestsremote.py
+++ b/testing/mochitest/runtestsremote.py
@@ -254,17 +254,17 @@ class MochiRemote(Mochitest):
         return retVal
 
     def installChromeFile(self, filename, options):
         parts = options.app.split('/')
         if (parts[0] == options.app):
           return "NO_CHROME_ON_DROID"
         path = '/'.join(parts[:-1])
         manifest = path + "/chrome/" + os.path.basename(filename)
-        if self._dm.pushFile(filename, manifest) == None:
+        if self._dm.pushFile(filename, manifest) == False:
             raise devicemanager.FileError("Unable to install Chrome files on device.")
         return manifest
 
     def getLogFilePath(self, logFile):             
         return logFile
 
 def main():
     scriptdir = os.path.abspath(os.path.realpath(os.path.dirname(__file__)))