mbed_base: Fix copy_method.check_flash_error

After copying an image to a device using a "copy method" plugin, we
attempt to check if any flash errors were encountered. We search for a
FAIL.txt file on a Daplink compatible device's USB mass storage, if the
file is present, we decide an error has occured and return False.

mbedls.detect is used to enumerate all daplink/Mbed devices if a
target_id is passed. The target_id often comes from the USB serial ID,
which bears no relation to whether the device is Mbed enabled/Daplink
compatible or not. If no target_id is given the function returns True
without checking for FAIL.TXT at all. This check is not strong enough to
cover all cases where a user would potentially not be using an Mbed
device.

One example where the above check is not fit for purpose: The user has
specified the pyocd copy method. pyocd supports devices which are not
"Mbed Enabled" or Daplink compatible, but the user must pass a target_id
because pyocd requires it. In this case we would waste 2.5s repeatedly
checking with mbedls.detect if an Mbed/Daplink device was connected. We
could also emit a warning message stating their device was detected but
not mounted, advising them to "pass -u to show the device".
Unfortunately this information is incorrect, as the "-u" option is an
option in mbedls's CLI and not mbedhtrun's CLI. This causes confusion
and irritation for the user.

It seems the only "Mbed specific" copy methods are actually named
"shell" and "default" (however "shell" appears to be the actual default
used if no copy method is specified on the command line). So the least
intrusive fix is to check if we're using one of the mbedls-compatible
copy_methods, and if not, just return True, just like we do when no
target_id is given. This fixes the delay and unecessary warning when
specifying the pyocd copy method with a non-Mbed device.

Author: rwalton-arm

src/mbed_os_tools/test/host_tests_runner/mbed_base.py

@@ -123,6 +123,19 @@ def check_flash_error(target_id, disk, initial_remount_count):
                 self.logger.prn_wrn("Target ID not found: Skipping flash check and retry")
                 return True
 
+            if not copy_method in ["shell", "default"]:
+                # We're using a "copy method" that may not necessarily require
+                # an "Mbed Enabled" device. In this case we shouldn't use
+                # mbedls.detect to attempt to rediscover the mount point, as
+                # mbedls.detect is only compatible with Mbed Enabled devices.
+                # It's best just to return `True` and continue here. This will
+                # avoid the inevitable 2.5s delay caused by us repeatedly
+                # attempting to enumerate Mbed Enabled devices in the code
+                # below when none are connected. The user has specified a
+                # non-Mbed plugin copy method, so we shouldn't delay them by
+                # trying to check for Mbed Enabled devices.
+                return True
+
             bad_files = set(['FAIL.TXT'])
             # Re-try at max 5 times with 0.5 sec in delay
             for i in range(5):

test/test/test_mbed_base.py

@@ -0,0 +1,112 @@
+# Copyright (c) 2021, Arm Limited and affiliates.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+import shutil
+import mock
+import os
+import unittest
+from tempfile import mkdtemp
+
+from mbed_os_tools.test.host_tests_runner.mbed_base import Mbed
+
+class TemporaryDirectory(object):
+    def __init__(self):
+        self.fname = "tempdir"
+
+    def __enter__(self):
+        self.fname = mkdtemp()
+        return self.fname
+
+    def __exit__(self, *args, **kwargs):
+        shutil.rmtree(self.fname)
+
+@mock.patch("mbed_os_tools.test.host_tests_runner.mbed_base.ht_plugins")
+@mock.patch("mbed_os_tools.test.host_tests_runner.mbed_base.detect")
+class TestMbed(unittest.TestCase):
+    def test_skips_discover_mbed_if_non_mbed_copy_method_used(
+        self, mock_detect, mock_ht_plugins
+    ):
+        with TemporaryDirectory() as tmpdir:
+            image_path = os.path.join(tmpdir, "test.elf")
+            with open(image_path, "w") as f:
+                f.write("1234")
+
+            options = mock.Mock(
+                copy_method="pyocd",
+                image_path=image_path,
+                disk=None,
+                port="port",
+                micro="mcu",
+                target_id="BK99",
+                polling_timeout=5,
+                program_cycle_s=None,
+                json_test_configuration=None,
+                format="blah",
+            )
+
+            mbed = Mbed(options)
+            mbed.copy_image()
+
+            mock_detect.create().list_mbeds.assert_not_called()
+            mock_ht_plugins.call_plugin.assert_called_once_with(
+                "CopyMethod",
+                options.copy_method,
+                image_path=options.image_path,
+                mcu=options.micro,
+                serial=options.port,
+                destination_disk=options.disk,
+                target_id=options.target_id,
+                pooling_timeout=options.polling_timeout,
+                format=options.format,
+            )
+
+    def test_discovers_mbed_if_mbed_copy_method_used(
+        self, mock_detect, mock_ht_plugins
+    ):
+        with TemporaryDirectory() as tmpdir:
+            image_path = os.path.join(tmpdir, "test.elf")
+            with open(image_path, "w") as f:
+                f.write("1234")
+            options = mock.Mock(
+                copy_method="shell",
+                image_path=image_path,
+                disk=None,
+                port="port",
+                micro="mcu",
+                target_id="BK99",
+                polling_timeout=5,
+                program_cycle_s=None,
+                json_test_configuration=None,
+                format="blah",
+            )
+
+            mbed = Mbed(options)
+            mbed.copy_image()
+
+            mock_detect.create().list_mbeds.assert_called()
+            mock_ht_plugins.call_plugin.assert_called_once_with(
+                "CopyMethod",
+                options.copy_method,
+                image_path=options.image_path,
+                mcu=options.micro,
+                serial=options.port,
+                destination_disk=options.disk,
+                target_id=options.target_id,
+                pooling_timeout=options.polling_timeout,
+                format=options.format,
+            )
+
+
+if __name__ == "__main__":
+    unittest.main()