servo_updater: consolidate common code-path

Commonly the users of servo_updater (as a script, or a file) care about
the firmware version string in addition to knowing what the filename is,
and the configuration is. These are used to print informational
messages, or check whether an update is needed among other things.
This change consolidates that logic by making sure that the version
string is always extracted when possible.

BRANCH=None
BUG=b:179310743
TEST=sudo servod -b soraka
// observe logs print the servo firmware version information
TEST=sudo servo_updater --board servo_micro
// observe no errors

Change-Id: Id7cb8f53e5fc3e1f557d74445d49fcafbda851ad
Signed-off-by: Ruben Rodriguez Buchillon <coconutruben@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2661020
Reviewed-by: Brian Nemec <bnemec@chromium.org>

1 files changed, 41 insertions, 31 deletions

diff --git a/extra/usb_updater/servo_updater.py b/extra/usb_updater/servo_updater.py
index a528caf9e6..89f3f651ad 100755
--- a/extra/usb_updater/servo_updater.py
+++ b/extra/usb_updater/servo_updater.py
@@ -227,7 +227,32 @@ def do_updater_version(tinys):
   raise ServoUpdaterException(
       "Can't determine updater target from vers: [%s]" % vers)
 
-def findfiles(cname, fname, channel=DEFAULT_CHANNEL):
+def _extract_version(boardname, binfile):
+  """Find the version string from |binfile|.
+
+  Args:
+    boardname: the name of the board, eg. "servo_micro"
+    binfile: path to the binary to search
+
+  Returns:
+    the version string.
+  """
+  if boardname is None:
+    # cannot extract the version if the name is None
+    return None
+  rawstrings = subprocess.check_output(
+      ['cbfstool', binfile, 'read', '-r', 'RO_FRID', '-f', '/dev/stdout'],
+      **c.get_subprocess_args())
+  m = re.match(r'%s_v\S+' % boardname, rawstrings)
+  if m:
+    newvers = m.group(0).strip(' \t\r\n\0')
+  else:
+    raise ServoUpdaterException("Can't find version from file: %s." % binfile)
+
+  return newvers
+
+
+def get_files_and_version(cname, fname, channel=DEFAULT_CHANNEL):
   """Select config and firmware binary files.
 
   This checks default file names and paths.
@@ -238,8 +263,9 @@ def findfiles(cname, fname, channel=DEFAULT_CHANNEL):
     cname: board name, or config name. eg. "servo_v4" or "servo_v4.json"
     fname: firmware binary name. Can be None to try default.
     channel: the channel requested for servo firmware. See |CHANNELS| above.
+
   Returns:
-    cname, fname: validated filenames selected from the path.
+    cname, fname, version: validated filenames selected from the path.
   """
   for p in (DEFAULT_BASE_PATH, TEST_IMAGE_BASE_PATH):
     updater_path = os.path.join(p, COMMON_PATH)
@@ -266,12 +292,14 @@ def findfiles(cname, fname, channel=DEFAULT_CHANNEL):
     if not os.path.isfile(cname):
       raise ServoUpdaterException("Can't find config file: %s." % cname)
 
-  if not fname:
-    # If None, try to infer board name from config.
-    with open(cname) as data_file:
-      data = json.load(data_file)
-    boardname = data['board']
+  # Always retrieve the boardname
+  with open(cname) as data_file:
+    data = json.load(data_file)
+  boardname = data['board']
 
+  if not fname:
+    # If no |fname| supplied, look for the default locations with the board
+    # and channel requested.
     binary_file = '%s.%s.bin' % (boardname, channel)
     newname = os.path.join(firmware_path, binary_file)
     if os.path.isfile(newname):
@@ -287,28 +315,11 @@ def findfiles(cname, fname, channel=DEFAULT_CHANNEL):
     else:
       raise ServoUpdaterException("Can't find file: %s." % fname)
 
-  return cname, fname
+  # Lastly, retrieve the version as well for decision making, debug, and
+  # informational purposes.
+  binvers = _extract_version(boardname, fname)
 
-def find_available_version(boardname, binfile):
-  """Find the version string from the binary file.
-
-  Args:
-    boardname: the name of the board, eg. "servo_micro"
-    binfile: the binary to search
-
-  Returns:
-    the version string.
-  """
-  rawstrings = subprocess.check_output(
-      ['cbfstool', binfile, 'read', '-r', 'RO_FRID', '-f', '/dev/stdout'],
-      **c.get_subprocess_args())
-  m = re.match(r'%s_v\S+' % boardname, rawstrings)
-  if m:
-    newvers = m.group(0).strip(' \t\r\n\0')
-  else:
-    raise ServoUpdaterException("Can't find version from file: %s." % binfile)
-
-  return newvers
+  return cname, fname, binvers
 
 def main():
   parser = argparse.ArgumentParser(description="Image a servo micro device")
@@ -331,7 +342,8 @@ def main():
 
   args = parser.parse_args()
 
-  brdfile, binfile = findfiles(args.board, args.file, args.channel)
+  brdfile, binfile, newvers = get_files_and_version(args.board, args.file,
+                                                    args.channel)
 
   serialno = args.serialno
 
@@ -351,8 +363,6 @@ def main():
   if not args.force:
     vers = do_version(tinys)
     print("Current %s version is   %s" % (boardname, vers))
-
-    newvers = find_available_version(boardname, binfile)
     print("Available %s version is %s" % (boardname, newvers))
 
     if newvers == vers: