Support updating config in an existing logcat service instance (#608)

diff --git a/mobly/controllers/android_device_lib/services/logcat.py b/mobly/controllers/android_device_lib/services/logcat.py
index f3d4238..0a94ea2 100644
--- a/mobly/controllers/android_device_lib/services/logcat.py
+++ b/mobly/controllers/android_device_lib/services/logcat.py
@@ -36,7 +36,8 @@
         clear_log: bool, clears the logcat before collection if True.
         logcat_params: string, extra params to be added to logcat command.
         output_file_path: string, the path on the host to write the log file
-            to. The service will automatically generate one if not specified.
+            to, including the actual filename. The service will automatically
+            generate one if not specified.
     """
 
     def __init__(self,
@@ -61,7 +62,9 @@
         self._ad = android_device
         self._adb_logcat_process = None
         self.adb_logcat_file_path = None
-        self._configs = configs if configs else Config()
+        # Logcat service uses a single config obj, using singular internal
+        # name: `_config`.
+        self._config = configs if configs else Config()
 
     def _enable_logpersist(self):
         """Attempts to enable logpersist daemon to persist logs."""
@@ -192,27 +195,42 @@
                 self._ad,
                 'Logcat thread is already running, cannot start another one.')
 
+    def update_config(self, new_config):
+        """Updates the configuration for the service.
+
+        This will completely reset the service. Previous output files may be
+        orphaned if output path is changed.
+
+        Args:
+            new_config: Config, the new config to use.
+        """
+        self.stop()
+        self._ad.log.info('[LogcatService] Changing config from %s to %s',
+                          self._config, new_config)
+        self._config = new_config
+        self.start()
+
     def start(self):
         """Starts a standing adb logcat collection.
 
         The collection runs in a separate subprocess and saves logs in a file.
         """
         self._assert_not_running()
-        if self._configs.clear_log:
+        if self._config.clear_log:
             self.clear_adb_log()
         self._start()
 
     def _start(self):
         """The actual logic of starting logcat."""
         self._enable_logpersist()
-        logcat_file_path = self._configs.output_file_path
+        logcat_file_path = self._config.output_file_path
         if not logcat_file_path:
             f_name = 'adblog,%s,%s.txt' % (self._ad.model,
                                            self._ad._normalized_serial)
             logcat_file_path = os.path.join(self._ad.log_path, f_name)
         utils.create_dir(os.path.dirname(logcat_file_path))
         cmd = '"%s" -s %s logcat -v threadtime %s >> "%s"' % (
-            adb.ADB, self._ad.serial, self._configs.logcat_params,
+            adb.ADB, self._ad.serial, self._config.logcat_params,
             logcat_file_path)
         process = utils.start_standing_subprocess(cmd, shell=True)
         self._adb_logcat_process = process
diff --git a/tests/mobly/controllers/android_device_lib/services/logcat_test.py b/tests/mobly/controllers/android_device_lib/services/logcat_test.py
index b7467fd..f7452a8 100755
--- a/tests/mobly/controllers/android_device_lib/services/logcat_test.py
+++ b/tests/mobly/controllers/android_device_lib/services/logcat_test.py
@@ -89,15 +89,13 @@
             output = f.read()
         self.assertNotIn(content, output)
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     @mock.patch('mobly.utils.create_dir')
-    @mock.patch(
-        'mobly.utils.start_standing_subprocess', return_value='process')
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
     def test_start_and_stop(self, stop_proc_mock, start_proc_mock,
                             create_dir_mock, FastbootProxy, MockAdbProxy):
@@ -133,15 +131,40 @@
         self.assertEqual(logcat_service.adb_logcat_file_path,
                          expected_log_path)
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     @mock.patch('mobly.utils.create_dir')
-    @mock.patch(
-        'mobly.utils.start_standing_subprocess', return_value='process')
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
+    @mock.patch('mobly.utils.stop_standing_subprocess')
+    def test_update_config(self, stop_proc_mock, start_proc_mock,
+                           create_dir_mock, FastbootProxy, MockAdbProxy):
+        mock_serial = '1'
+        ad = android_device.AndroidDevice(serial=mock_serial)
+        logcat_service = logcat.Logcat(ad)
+        logcat_service.start()
+        new_log_params = '-a -b -c'
+        new_file_path = 'some/path/log.txt'
+        new_config = logcat.Config(logcat_params=new_log_params,
+                                   output_file_path=new_file_path)
+        logcat_service.update_config(new_config)
+        self.assertTrue(logcat_service._adb_logcat_process)
+        create_dir_mock.assert_has_calls([mock.call('some/path')])
+        expected_adb_cmd = ('"adb" -s 1 logcat -v threadtime -a -b -c >> '
+                            '"some/path/log.txt"')
+        start_proc_mock.assert_called_with(expected_adb_cmd, shell=True)
+        self.assertEqual(logcat_service.adb_logcat_file_path,
+                         'some/path/log.txt')
+
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.utils.create_dir')
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
     @mock.patch(
         'mobly.controllers.android_device_lib.services.logcat.Logcat.clear_adb_log',
@@ -164,14 +187,12 @@
         self.assertTrue(logcat_service.is_alive)
         clear_adb_mock.assert_not_called()
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
-    @mock.patch(
-        'mobly.utils.start_standing_subprocess', return_value='process')
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
     @mock.patch(
         'mobly.controllers.android_device_lib.services.logcat.Logcat.clear_adb_log',
@@ -214,15 +235,13 @@
         self.AssertFileDoesNotContain(FILE_CONTENT, expected_path1)
         self.assertFalse(os.path.exists(logcat_service.adb_logcat_file_path))
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     @mock.patch('mobly.utils.create_dir')
-    @mock.patch(
-        'mobly.utils.start_standing_subprocess', return_value='process')
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
     def test_take_logcat_with_extra_params(self, stop_proc_mock,
                                            start_proc_mock, create_dir_mock,
@@ -249,12 +268,10 @@
         self.assertEqual(logcat_service.adb_logcat_file_path,
                          expected_log_path)
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     def test_instantiation(self, MockFastboot, MockAdbProxy):
         """Verifies the AndroidDevice object's basic attributes are correctly
         set after instantiation.
@@ -265,18 +282,15 @@
         self.assertIsNone(logcat_service._adb_logcat_process)
         self.assertIsNone(logcat_service.adb_logcat_file_path)
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
-    @mock.patch(
-        'mobly.utils.start_standing_subprocess', return_value='process')
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
-    @mock.patch(
-        'mobly.logger.get_log_line_timestamp',
-        return_value=MOCK_ADB_LOGCAT_END_TIME)
+    @mock.patch('mobly.logger.get_log_line_timestamp',
+                return_value=MOCK_ADB_LOGCAT_END_TIME)
     def test_cat_adb_log(self, mock_timestamp_getter, stop_proc_mock,
                          start_proc_mock, FastbootProxy, MockAdbProxy):
         """Verifies that AndroidDevice.cat_adb_log loads the correct adb log
@@ -296,8 +310,8 @@
             logcat_service.cat_adb_log('some_test', MOCK_ADB_LOGCAT_BEGIN_TIME)
         logcat_service.start()
         utils.create_dir(ad.log_path)
-        mock_adb_log_path = os.path.join(ad.log_path, 'adblog,%s,%s.txt' %
-                                         (ad.model, ad.serial))
+        mock_adb_log_path = os.path.join(
+            ad.log_path, 'adblog,%s,%s.txt' % (ad.model, ad.serial))
         with io.open(mock_adb_log_path, 'w', encoding='utf-8') as f:
             f.write(MOCK_ADB_LOGCAT)
         logcat_service.cat_adb_log('some_test', MOCK_ADB_LOGCAT_BEGIN_TIME)
@@ -310,18 +324,15 @@
         # Stops adb logcat.
         logcat_service.stop()
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock_android_device.MockAdbProxy('1'))
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
-    @mock.patch(
-        'mobly.utils.start_standing_subprocess', return_value='process')
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock_android_device.MockAdbProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.utils.start_standing_subprocess',
+                return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
-    @mock.patch(
-        'mobly.logger.get_log_line_timestamp',
-        return_value=MOCK_ADB_LOGCAT_END_TIME)
+    @mock.patch('mobly.logger.get_log_line_timestamp',
+                return_value=MOCK_ADB_LOGCAT_END_TIME)
     def test_cat_adb_log_with_unicode(self, mock_timestamp_getter,
                                       stop_proc_mock, start_proc_mock,
                                       FastbootProxy, MockAdbProxy):
@@ -342,8 +353,8 @@
             logcat_service.cat_adb_log('some_test', MOCK_ADB_LOGCAT_BEGIN_TIME)
         logcat_service.start()
         utils.create_dir(ad.log_path)
-        mock_adb_log_path = os.path.join(ad.log_path, 'adblog,%s,%s.txt' %
-                                         (ad.model, ad.serial))
+        mock_adb_log_path = os.path.join(
+            ad.log_path, 'adblog,%s,%s.txt' % (ad.model, ad.serial))
         with io.open(mock_adb_log_path, 'w', encoding='utf-8') as f:
             f.write(MOCK_ADB_UNICODE_LOGCAT)
         logcat_service.cat_adb_log('some_test', MOCK_ADB_LOGCAT_BEGIN_TIME)
@@ -357,12 +368,10 @@
         # Stops adb logcat.
         logcat_service.stop()
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock.MagicMock())
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock.MagicMock())
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     def test__enable_logpersist_with_logpersist(self, MockFastboot,
                                                 MockAdbProxy):
         mock_serial = '1'
@@ -374,7 +383,8 @@
         }
         mock_adb_proxy.has_shell_command.side_effect = lambda command: {
             'logpersist.start': True,
-            'logpersist.stop': True, }[command]
+            'logpersist.stop': True,
+        }[command]
         ad = android_device.AndroidDevice(serial=mock_serial)
         logcat_service = logcat.Logcat(ad)
         logcat_service._enable_logpersist()
@@ -383,12 +393,10 @@
             mock.call('logpersist.start'),
         ])
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock.MagicMock())
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock.MagicMock())
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     def test__enable_logpersist_with_user_build_device(self, MockFastboot,
                                                        MockAdbProxy):
         mock_serial = '1'
@@ -400,18 +408,17 @@
         }
         mock_adb_proxy.has_shell_command.side_effect = lambda command: {
             'logpersist.start': True,
-            'logpersist.stop': True, }[command]
+            'logpersist.stop': True,
+        }[command]
         ad = android_device.AndroidDevice(serial=mock_serial)
         logcat_service = logcat.Logcat(ad)
         logcat_service._enable_logpersist()
         mock_adb_proxy.shell.assert_not_called()
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock.MagicMock())
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock.MagicMock())
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     def test__enable_logpersist_with_missing_all_logpersist(
             self, MockFastboot, MockAdbProxy):
         def adb_shell_helper(command):
@@ -431,19 +438,18 @@
         }
         mock_adb_proxy.has_shell_command.side_effect = lambda command: {
             'logpersist.start': False,
-            'logpersist.stop': False, }[command]
+            'logpersist.stop': False,
+        }[command]
         mock_adb_proxy.shell.side_effect = adb_shell_helper
         ad = android_device.AndroidDevice(serial=mock_serial)
         logcat_service = logcat.Logcat(ad)
         logcat_service._enable_logpersist()
         mock_adb_proxy.shell.assert_not_called()
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock.MagicMock())
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock.MagicMock())
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     def test__enable_logpersist_with_missing_logpersist_stop(
             self, MockFastboot, MockAdbProxy):
         def adb_shell_helper(command):
@@ -461,7 +467,8 @@
         }
         mock_adb_proxy.has_shell_command.side_effect = lambda command: {
             'logpersist.start': True,
-            'logpersist.stop': False, }[command]
+            'logpersist.stop': False,
+        }[command]
         mock_adb_proxy.shell.side_effect = adb_shell_helper
         ad = android_device.AndroidDevice(serial=mock_serial)
         logcat_service = logcat.Logcat(ad)
@@ -470,9 +477,8 @@
             mock.call('logpersist.stop --clear'),
         ])
 
-    @mock.patch(
-        'mobly.controllers.android_device_lib.adb.AdbProxy',
-        return_value=mock.MagicMock())
+    @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy',
+                return_value=mock.MagicMock())
     @mock.patch('mobly.utils.stop_standing_subprocess')
     def test__enable_logpersist_with_missing_logpersist_start(
             self, MockFastboot, MockAdbProxy):
@@ -491,7 +497,8 @@
         }
         mock_adb_proxy.has_shell_command.side_effect = lambda command: {
             'logpersist.start': False,
-            'logpersist.stop': True, }[command]
+            'logpersist.stop': True,
+        }[command]
         mock_adb_proxy.shell.side_effect = adb_shell_helper
         ad = android_device.AndroidDevice(serial=mock_serial)
         logcat_service = logcat.Logcat(ad)
@@ -499,9 +506,8 @@
         mock_adb_proxy.shell.assert_not_called()
 
     @mock.patch('mobly.controllers.android_device_lib.adb.AdbProxy')
-    @mock.patch(
-        'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
-        return_value=mock_android_device.MockFastbootProxy('1'))
+    @mock.patch('mobly.controllers.android_device_lib.fastboot.FastbootProxy',
+                return_value=mock_android_device.MockFastbootProxy('1'))
     def test_clear_adb_log(self, MockFastboot, MockAdbProxy):
         mock_serial = '1'
         ad = android_device.AndroidDevice(serial=mock_serial)