Make state explicit in logcat service update config. (#609)


diff --git a/mobly/controllers/android_device_lib/services/logcat.py b/mobly/controllers/android_device_lib/services/logcat.py
index 0a94ea2..6671fbb 100644
--- a/mobly/controllers/android_device_lib/services/logcat.py
+++ b/mobly/controllers/android_device_lib/services/logcat.py
@@ -198,17 +198,19 @@
     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.
+        The service needs to be stopped before updating, and explicitly started
+        after the update.
+
+        This will 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._assert_not_running()
         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.
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 f7452a8..1350a24 100755
--- a/tests/mobly/controllers/android_device_lib/services/logcat_test.py
+++ b/tests/mobly/controllers/android_device_lib/services/logcat_test.py
@@ -145,11 +145,13 @@
         ad = android_device.AndroidDevice(serial=mock_serial)
         logcat_service = logcat.Logcat(ad)
         logcat_service.start()
+        logcat_service.stop()
         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)
+        logcat_service.start()
         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 >> '
@@ -166,6 +168,29 @@
     @mock.patch('mobly.utils.start_standing_subprocess',
                 return_value='process')
     @mock.patch('mobly.utils.stop_standing_subprocess')
+    def test_update_config_while_running(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_config = logcat.Config(logcat_params='-blah',
+                                   output_file_path='some/path/file.txt')
+        with self.assertRaisesRegex(
+                logcat.Error,
+                'Logcat thread is already running, cannot start another one'):
+            logcat_service.update_config(new_config)
+        self.assertTrue(logcat_service.is_alive)
+
+    @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',
         return_value=mock_android_device.MockAdbProxy('1'))