Added interface version information to mbed detect command. #5077

dhwalters423 · 2017-09-12T10:03:28Z

Description

This adds the additional information of the interface version (IE DAPlink, OpenSDA, etc) to the command mbed detect

This has proved useful when debugging applications, particularly when encountering interface issues. Not knowing about mbedls, I found it difficult to debug my application, where the USB driver and board was crashing when dragging the .bin to the disk for deployment in Windows 10. This information is a "nice to have" for a quick reference for mbed developers.

Example output:

...Desktop\mbed Workspace\mbed-os>mbed detect

[mbed] Detected K64F, port COM4, mounted D:, interface version 0243
[mbed] Supported toolchains for K64F
+--------+-----------+-----------+-----------+-----------+-----------+
| Target | mbed OS 2 | mbed OS 5 |    ARM    |  GCC_ARM  |    IAR    |
+--------+-----------+-----------+-----------+-----------+-----------+
| K64F   | Supported | Supported | Supported | Supported | Supported |
+--------+-----------+-----------+-----------+-----------+-----------+
Supported targets: 1
Supported toolchains: 3

Status

Built on mbed OS 5.5, will be tested against mbed OS 5.6 when an rc is ready.

Migrations

Added one method to test_api.py def get_mounted_details_txt(mount_point) to pull the details.txt for a specific target given it's mount point. This could prove useful later as additional information may want to be propagated to other tools using the test API.

Related Issues

Submitted issue relating to this here:
ARMmbed/mbed-cli#541

The approach was changed slightly from the original issue request. This was because the mut matrix PrettyTable for the command mbed detect displayed specifically MUT information. In order to not change the format of the MUT JSON, I decided to create the additional function def get_mounted_details_txt(mount_point) which created another mbed ls tools resource and display the information separately from the mut matrix.

Steps to test or reproduce

Run the mbed detect command.

Expected output should be similar to:

[mbed] Detected K64F, port COM4, mounted D:, interface version 0243
[mbed] Supported toolchains for K64F
+--------+-----------+-----------+-----------+-----------+-----------+
| Target | mbed OS 2 | mbed OS 5 |    ARM    |  GCC_ARM  |    IAR    |
+--------+-----------+-----------+-----------+-----------+-----------+
| K64F   | Supported | Supported | Supported | Supported | Supported |
+--------+-----------+-----------+-----------+-----------+-----------+
Supported targets: 1
Supported toolchains: 3

dhwalters423 · 2017-09-12T11:00:40Z

Additional tests performed on Ubuntu 17.04 from VirtualBox using USB Extensions:

dwalters-VirtualBox:~/workspace/mbed-os$ mbed detect

[mbed] Detected K64F, port /dev/ttyACM0, mounted /media/dwalters/DAPLINK, interface version 0243
[mbed] Supported toolchains for K64F
+--------+-----------+-----------+-----------+-----------+-----------+
| Target | mbed OS 2 | mbed OS 5 |    ARM    |  GCC_ARM  |    IAR    |
+--------+-----------+-----------+-----------+-----------+-----------+
| K64F   | Supported | Supported | Supported | Supported | Supported |
+--------+-----------+-----------+-----------+-----------+-----------+
Supported targets: 1
Supported toolchains: 3

theotherjimmy · 2017-09-14T15:23:08Z

tools/test_api.py

@@ -1647,6 +1647,12 @@ def get_module_avail(module_name):
    """
    return module_name in sys.modules.keys()

+def get_mounted_details_txt(mount_point):


Why is this function in test_api.py? I don't think it logically fits here.

@theotherjimmy Happy to move it- my original thought process was to add it into test_api.get_autodetected_MUTS_list and return it as a mut object. After realising it would break the mut .json format, I decided to put it above. Any suggestions on where it may fit in better?

Since it's only used in detect_targets.py let's put it there.

Fixed minor typos in test_api.py Added get_interface_version to detect_targets

dhwalters423 · 2017-09-15T09:18:00Z

@theotherjimmy I removed get_mounted_details_txt from test_api.py and added get_interface_version to detect_targets, also fixed some minor typos I noticed.

theotherjimmy

Thanks for the typo fixes. The changes look good.

theotherjimmy · 2017-09-15T14:17:11Z

tools/detect_targets.py

+
+def get_interface_version(mount_point):
+    """ Function returns interface version from the target mounted on the specified mount point
+            mount_point = mut['port']


What is this line of the function description supposed to tell me?

Example usage. Though I just found one mistake- let me push an update there. An accurate usage should be mount_point = mut['disk']

theotherjimmy

This looks great!

theotherjimmy · 2017-09-15T14:39:56Z

tools/detect_targets.py

+def get_interface_version(mount_point):
+    """ Function returns interface version from the target mounted on the specified mount point
+            Example of mount_point:
+                mount_point = mut['disk']


I'm still not clear to me why I need an example of what to do with the parameter here. Are you suggesting that a user might be able to acquire a mount_point with the example code? If so, could you state that?

Pushed a new commit with a bit more clarity, apologies for the confusion ;)

theotherjimmy

THAT IS SO CLEAR 👏

theotherjimmy · 2017-09-15T14:51:14Z

/morph test

Let's get this in.

dhwalters423 · 2017-09-15T15:02:14Z

Apologies @theotherjimmy but I had to push one more emergency commit. I found an edge case on a NRF52840_DK where there was no details.txt and thus details_txt was None. This was causing a boom, so I added in a safety check there. Thanks for the quick reviews

theotherjimmy

Given the aforementioned "explosion", I think it might be good to have a unit test for this new function.

theotherjimmy · 2017-09-15T15:06:33Z

tools/detect_targets.py

+        details_txt = mbeds.get_details_txt(mount_point)
+
+        if (details_txt is None):
+            details_txt = {}


Could you just

return "unkown"

here?

Makes sense for a unit test, I hadn't realised how many variations of details.txt there could be until now. I'll work on this in the coming few days.

mbed-bot · 2017-09-15T17:54:19Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Let's get this in.

Output

mbed Build Number: 1317

All builds and test passed!

0xc0170 · 2017-09-20T12:38:41Z

I restarted jenkins CI

@dhwalters423 Is this complete ?

dhwalters423 · 2017-09-20T12:46:47Z

Hi @0xc0170 it's not quite ready as I discovered a breaking issue that will require a bit of tweaking and some test coverage. The issue is when a device does not have an interface version in the details.txt listed, which was causing the function to blow up when it tried to return None.

I'm out of the office this week so I've been a bit slow to work on this but I will make another commit addressing these issues in the next coming days.

dhwalters423 · 2017-09-25T15:57:31Z

@theotherjimmy Added in tests as requested, though I think wrapping the entire thing in a try/except covers all cases, therefore I think there is an argument that the unit tests are redundant. Either way, it's a nice double coverage there

theotherjimmy · 2017-09-25T16:05:54Z

tools/test/detect_targets_test.py

+
+            count += 1
+            self.valid_mount_point = mut['disk']
+            break


This looks very out of place. Count is only ever going to be 1 or 0.

theotherjimmy · 2017-09-25T16:07:30Z

tools/test/detect_targets_test.py

+        # Function should handle failure gracefully regardless of a mount point being present.
+        # Therefore it is safe to assume a valid mount point.
+        if count is 0:
+            self.valid_mount_point = "D:"        


This is needed by the test_interface_version_valid_mount_point function, so it should be assigned unconditionally.

How could we handle the case where there was no mount point (no device connected to the host), and across cases where host OS differs.. IE a valid mount point on Win 10 could be D: but a valid mount point in Linux is something like /media/{username}/DAPLink? There would need to be some condition that has to make an assumption at a certain point.

How could we handle the case where there was no mount point (no device connected to the host), and across cases where host OS differs.

Test 'em all?

There would need to be some condition that has to make an assumption at a certain point.

Yes, but that assumption can be guaranteed with mocking.

theotherjimmy · 2017-09-25T16:11:50Z

tools/test/detect_targets_test.py

+        :return:
+        """
+        # Gather a valid mount point from the host machine
+        muts = get_autodetected_MUTS_list()


This should be mocked so that we don't depend on this test machine having some connected devices.

theotherjimmy · 2017-09-25T16:12:54Z

tools/test/detect_targets_test.py

+    def test_interface_version_valid_mount_point(self):
+
+        interface_version = get_interface_version(self.valid_mount_point)
+        assert len(interface_version) > 0


This is not a strong guarantee. I would like to see an assertion like the others in this commit: asserting that the interface version is a particular string/number.

theotherjimmy

Please use mocks in your testing to avoid hardware/environment requirements during testing.

theotherjimmy

I like the tests. It's very clear what mbed LS should provide and what the mbed OS tools do.

theotherjimmy · 2017-09-29T14:18:07Z

tools/test/detect_targets_test.py

+class MbedLsToolsMock():
+
+    def __init__(self, type):
+        self.interface_test_type = type


Could you change the name of this parameter to not shadow a builtin?

theotherjimmy · 2017-09-29T14:43:44Z

It's been a long road. Let's get to the destination.

/morph test

mbed-bot · 2017-09-30T08:31:53Z

/morph test

mbed-bot · 2017-09-30T11:56:06Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1455

All builds and test passed!

mbed-bot · 2017-09-30T20:02:27Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1455

All builds and test passed!

Output

mbed Build Number: 1461

Build failed!

mbed-bot · 2017-10-01T05:10:34Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1455

All builds and test passed!

Output

mbed Build Number: 1461

Build failed!

Output

mbed Build Number: 1466

All builds and test passed!

mbed-bot · 2017-10-01T14:07:00Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1455

All builds and test passed!

Output

mbed Build Number: 1461

Build failed!

Output

mbed Build Number: 1466

All builds and test passed!

Output

mbed Build Number: 1471

All builds and test passed!

mbed-bot · 2017-10-01T23:46:40Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test
Output

mbed Build Number: 1455

All builds and test passed!

Output

mbed Build Number: 1461

Build failed!

Output

mbed Build Number: 1466

All builds and test passed!

Output

mbed Build Number: 1471

All builds and test passed!

Output

mbed Build Number: 1477

All builds and test passed!

theotherjimmy · 2017-10-02T15:06:35Z

/morph test

mbed-bot · 2017-10-03T07:08:32Z

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1498

Test failed!

theotherjimmy · 2017-10-03T15:43:11Z

/morph test

mbed-bot · 2017-10-03T18:56:19Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1514

All builds and test passed!

dhwalters423 added 2 commits September 11, 2017 16:54

Added additional DAPLink version info for mbed detect command`

d43a847

Changed DAPLink version to interface version.

7ff57c4

dhwalters423 mentioned this pull request Sep 12, 2017

mbed detect should show firmware information ARMmbed/mbed-cli#541

Closed

theotherjimmy suggested changes Sep 14, 2017

View reviewed changes

theotherjimmy added the needs: work label Sep 14, 2017

Removed get_mounted_details_txt from test_api.py

e4b55dd

Fixed minor typos in test_api.py Added get_interface_version to detect_targets

theotherjimmy approved these changes Sep 15, 2017

View reviewed changes

theotherjimmy added needs: CI and removed needs: work labels Sep 15, 2017

Fixed mount_point example usage

256ab61

theotherjimmy approved these changes Sep 15, 2017

View reviewed changes

Edited get_interface_version info for clarity

aafe614

theotherjimmy approved these changes Sep 15, 2017

View reviewed changes

Added in safety check if details_txt is None

7b49591

theotherjimmy reviewed Sep 15, 2017

View reviewed changes

0xc0170 added needs: work and removed needs: CI labels Sep 20, 2017

Added safety check, added test converage for new function

b887bb7

Removing unused imports/update copywright`

4c2d17e

theotherjimmy reviewed Sep 25, 2017

View reviewed changes

theotherjimmy suggested changes Sep 25, 2017

View reviewed changes

dhwalters423 added 2 commits September 29, 2017 10:05

Increased test coverage / mocking of external modules

a047f62

Minor copy edits, added comment

bb79f69

theotherjimmy approved these changes Sep 29, 2017

View reviewed changes

Changed type => test_type

61f46b8

theotherjimmy added ready for merge and removed needs: work labels Oct 3, 2017

theotherjimmy merged commit da78647 into ARMmbed:master Oct 5, 2017

theotherjimmy added release-version: 5.6.3 and removed ready for merge labels Oct 5, 2017

dhwalters423 deleted the dhwalters423-add-detect-info branch October 30, 2017 11:29

dhwalters423 mentioned this pull request Dec 15, 2017

tools/detect_targets.py using deprecated call mbedls.get_details_txt #5712

Closed

Added interface version information to mbed detect command. #5077

Added interface version information to mbed detect command. #5077

Uh oh!

Conversation

dhwalters423 commented Sep 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Status

Migrations

Related Issues

Steps to test or reproduce

Uh oh!

dhwalters423 commented Sep 12, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhwalters423 commented Sep 15, 2017

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Sep 15, 2017

Uh oh!

dhwalters423 commented Sep 15, 2017

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbed-bot commented Sep 15, 2017

Output

Uh oh!

0xc0170 commented Sep 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhwalters423 commented Sep 20, 2017

Uh oh!

dhwalters423 commented Sep 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhwalters423 commented Sep 12, 2017 •

edited

Loading

0xc0170 commented Sep 20, 2017 •

edited

Loading