search

ARMmbed / mbed-os-tools

The tools to test and work with Mbed OS
Apache License 2.0
33 stars 67 forks source link

mbed_base: Fix copy_method.check_flash_error #270

Closed rwalton-arm closed 3 years ago

rwalton-arm commented 3 years ago

Description

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.

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Patater commented 3 years ago

Seems there is still an issue with importing mbed_base in Python 2. Rebased on latest master.

rwalton-arm commented 3 years ago

Seems there is still an issue with importing mbed_base in Python 2. Rebased on latest master.

It was because TemporaryDirectory isn't available for Python 2 and test_mbed_base.py imports it. There is a backport of TemporaryDirectory on PyPI, but that caused some other issues with sys.shutdown so I didn't feel comfortable using it. I've just added a thin context manager wrapper over mkdtemp to ensure the test directory is cleaned up in an exception-safe way.