Jlink and other fw detection #299

bridadan · 2018-01-30T21:24:58Z

This is a proposal to address #283.

One thing that needs more investigation is how to properly identify JLink devices. Currently I'm assuming that the target id is unique and corresponds with the platform_name, just like DAPLink. I need to check other platforms running JLink to verify this.

jupe · 2018-01-30T21:31:02Z

mbed_lstools/platform_database.py

@@ -47,7 +47,13 @@
    u'0003': u'LPC2368',
    u'0004': u'LPC2368',
    u'0005': u'LPC2368',
-    u'0006': u'LPC2368',
+    u'0006': {
+        u'default': 'daplink',


when this default is needed?

If someone invokes the platform_db.get api without specifying a device_type, then we need to know which platform mapping we need to use. I'm open to restructuring that bit if needed, it was just a quick pass at it.

bridadan · 2018-01-30T22:26:11Z

Bad news: I've tried flashing a KL25Z, a KL27Z, and a KL43Z using the JLink binaries found here: https://www.segger.com/downloads/jlink/#JLinkOpenSDABoardSpecificFirmwares

All of them have a usb target ID of 000000123456. So it doesn't like this is reliable for detection. It also means that they won't be unique in CI if multiple are plugged into the same machine. This may not be an issue for the NRF52_DK Jlink build as it seems to be more unique.

But the point is my modifications to the platform database won't work. Instead the DB needs to have some significant changes to it most likely.

bridadan · 2018-01-30T23:40:58Z

This actually works! But it needs some polish, some comments, and most importantly tests.

@theotherjimmy How do you feel about the platform database changes? I don't love them but they work.

coveralls · 2018-02-01T19:52:21Z

Coverage increased (+1.4%) to 77.668% when pulling ba99fc8 on bridadan:jlink_and_other_fw_detection into 2efd5a8 on ARMmbed:master.

bridadan · 2018-02-01T20:09:58Z

I got more tests to write

bridadan · 2018-02-01T21:35:43Z

I'm actually starting to become reasonably happy with this!

A few things to note:

I have renamed the parameter read_details_txt to include_extra_info in the top level API, which is a breaking change
- This rename was because not all firmware store the extra info in a file called DETAILS.TXT
- Instead of renaming the parameter I could alias the parameter so both of them did the same thing. A deprecation warning could be issued for the read_details_txt parameter when used.
  - If that sounds ok to you @theotherjimmy then I'll go ahead and make that change
If it can't determine that the device is using Jlink (segger.html file is present), then it will assume it is a daplink device. This is fairly safe but could be misleading.
- Ex. ST-Link is typically completely compatible with daplink, but it maybe useful to try and detect if it is an st-link device for management purposes.

theotherjimmy · 2018-02-01T21:48:29Z

@bridadan

Instead of renaming the parameter I could alias the parameter so both of them did the same thing. A deprecation warning could be issued for the read_details_txt parameter when used.

👍

If it can't determine that the device is using Jlink (segger.html file is present), then it will assume it is a daplink device. This is fairly safe but could be misleading.

As usual, it should respect the FsInteraction.whatever options.

bridadan · 2018-02-01T21:53:11Z

@theotherjimmy

If it can't determine that the device is using Jlink (segger.html file is present), then it will assume it is a daplink device. This is fairly safe but could be misleading.

As usual, it should respect the FsInteraction. options.

It does do this currently. But in the case where the files don't match a JLink device, it will default to daplink.

I should also mention this changes the format of the platform database. Previously it was this:

{
    "0000": "name"
}

Now it's this:

{
    "daplink": {
        "0000": "name"
    },
    "jlink": ...
}

I believe this should work on systems that already have a platform db on the filesystem, but it does change how you interact with it when not using daplink devices.

theotherjimmy · 2018-02-01T21:59:28Z

@bridadan The changes to the platform DB should be fine. I forgot that it already gracefully upgrades.

bridadan · 2018-02-01T22:51:52Z

@theotherjimmy at this point I'm pretty much done with the work I had in mind, let me know if you see any issues or want anything changed

theotherjimmy · 2018-02-01T22:59:52Z

Given that you don't use the include_extra_info flag in the JLink implementation, it has exactly the semantics as read_details_text. I see no point it adding the rename yet. Until it means something for JLink, I think the name change is unneeded churn.

bridadan · 2018-02-01T23:11:55Z

Agreed, I've reverted the change to the parameter.

theotherjimmy · 2018-02-02T17:02:18Z

Awesome. Thanks!

juhhov · 2018-02-05T11:34:56Z

This seems to work 👍

One thing to consider. We would like to use the mbedls platform_name for JLinkExe device parameter.
For nrf52 current platform_name NRF52_DK is not recognized. Here is the complete list of JLinkExe supported boards https://www.segger.com/downloads/supported-devices.php , for nrf52 boards select Nordic Semi from the list at the top.
Would it be possible to use platform_name from that list? Thoughts @bridadan , @jupe ?

bridadan · 2018-02-05T16:37:12Z

@juhhov, that should definitely be possible, although we'll have to return it as a separate data value instead of the platform_name for compatibility reasons with the rest of the Mbed OS tools. Probably something like jlink_platform_name.

In the case of the NRF52_DK, what is the correct device name from that list? There seems to be four options: nRF52810_xxAA, nRF52832_xxAA, nRF52832_xxAB, and nRF52840_xxAA.

juhhov · 2018-02-06T08:38:25Z

Thanks @bridadan , separate value is fine.
I guess there is currently no way to know the exact version of the board. The ones that we have are nRF52832. No idea about the xxAA/B.
Plain NRF52 has worked for us so far. Maybe that can be used for now?

simtind · 2018-02-06T13:01:33Z

It's an nRF52832_xxaa, ab is a device with smaller memory that has no separate dev kit.

jupe · 2018-02-06T13:44:10Z

@simtind do you know if there is way to detect board/chip model from dev kit using usb port ? Which can be used for JLinkExe --device <here>.

simtind · 2018-02-06T16:55:55Z

Nothing for the general case. What you can do is connect as some generic cpu (Cortex-m0 fex.) and check the cpuid register. That should tell you at least what core you're working on. Then you can reconnect as the correct core, which should give you access to memory.

For nRF devices we base the Jlink snr on the dev kit as you've guessed in the issue.
The two first numbers (in decimal) are always 68, the third number is 0 for nRF51-Dongle, 1 for nRF51-DK, 2 for nRF52-DK and 3 for nrF52840-PDK.

bridadan · 2018-02-08T19:17:23Z

Ok, rebased and added jlink information to the platform db. Probably need to add a few more tests for some missing coverage.

bridadan · 2018-02-08T20:58:51Z

Coveralls is taking its sweet time but I added the coverage back 👍

juhhov · 2018-02-20T12:55:21Z

@bridadan this works for us. Could this move forward?

theotherjimmy · 2018-02-20T14:29:37Z

@bridadan Could you rebase? I'll review it right now, assuming that little has to change when rebasing.

theotherjimmy · 2018-02-20T14:31:11Z

mbed_lstools/lstools_base.py

@@ -110,7 +110,7 @@ def list_mbeds_ext(self):
    def list_mbeds(
            self, fs_interaction=FSInteraction.BeforeFilter,
            filter_function=None, unique_names=False,
-            read_details_txt=False):
+            read_details_txt=None):


Could you revert this change? It seems like a "style-like" thing to me. I don't think this code explicitly looks for None instead of any falsey value

Oops good catch. That was from when I had initially renamed the parameter.

theotherjimmy · 2018-02-20T14:39:55Z

mbed_lstools/lstools_base.py

@@ -151,6 +151,7 @@ def list_mbeds(
                        platform_count[name] += 1
                        device['platform_name_unique'] = (
                            "%s[%d]" % (name, platform_count[name]))
+


Could you remove this formatting change?

theotherjimmy · 2018-02-20T14:40:36Z

mbed_lstools/lstools_base.py

        if not filter_function or filter_function(device):
            return device
        else:
            return None

    def _fs_after_id_check(self, device, filter_function, read_details_txt):
+


Could you remove this formatting change?

theotherjimmy · 2018-02-20T14:43:09Z

mbed_lstools/lstools_base.py

+        else:
+            device['platform_name'] = None
+
+    def _update_device_details_jlink(self, device, read_details_txt):


Could you change read_details_txt to _ to indicate that you are not using that parameter?

theotherjimmy · 2018-02-20T14:45:47Z

mbed_lstools/lstools_base.py

+            @param read_details_txt A boolean controlling the presense of the
+              output dict attributes read from other files present on the 'mount_point'
+        """
+        files = os.listdir(device['mount_point'])


Could we list the directory only once some how? We could do the listdir outside this function and _detect_device_type and pass it into both. I think that this may have an impact on performance (we're somewhat performance critical to RAAS, IIRC) and we want to avoid making things any slower than they have to be.

theotherjimmy · 2018-02-20T14:48:23Z

mbed_lstools/platform_database.py

+    u'jlink': {
+        u'X349858SLYN': {
+            u'platform_name': u'NRF52_DK',
+            u'jlink_device_name': u'nRF52832_xxaa'


Why do we need this data? Is it important to keep?

This is needed for the reason specified in this comment: #299 (comment)

bridadan · 2018-02-21T17:39:59Z

Rebased!

jupe · 2018-02-24T06:51:59Z

so how json result looks like now for jlink based board?

bridadan · 2018-02-26T15:07:58Z

@jupe Like so:

$ mbedls -j
[
    {
        "device_type": "jlink",
        "jlink_device_name": "nRF52832_xxaa",
        "mount_point": "D:",
        "platform_name": "NRF52_DK",
        "platform_name_unique": "NRF52_DK[0]",
        "serial_port": "COM32",
        "target_id": "000682961642",
        "target_id_usb_id": "000682961642",
        "url": "http://www.nordicsemi.com/X349858SLYN"
    }
]

jupe · 2018-02-26T15:11:25Z

looks good. Is device_type property exists allways, and daplink for daplink based boards?

bridadan · 2018-02-26T15:14:31Z

Yup!

$ mbedls -j
[
    {
        "daplink_auto_reset": "0",
        "daplink_automation_allowed": "1",
        "daplink_bootloader_crc": "0xa65218eb",
        "daplink_bootloader_version": "0242",
        "daplink_daplink_mode": "Interface",
        "daplink_git_sha": "67f8727a030bcc585e982d899fb6382db56d673b",
        "daplink_hic_id": "97969900",
        "daplink_interface_crc": "0xe4422294",
        "daplink_interface_version": "0244",
        "daplink_local_mods": "0",
        "daplink_overflow_detection": "1",
        "daplink_remount_count": "0",
        "daplink_unique_id": "0240000032044e4500257009997b00386781000097969900",
        "daplink_usb_interfaces": "MSD, CDC, HID",
        "daplink_version": "0244",
        "device_type": "daplink",
        "mount_point": "D:",
        "platform_name": "K64F",
        "platform_name_unique": "K64F[0]",
        "serial_port": "COM18",
        "target_id": "0240000032044e4500257009997b00386781000097969900",
        "target_id_mbed_htm": "0240000032044e4500257009997b00386781000097969900",
        "target_id_usb_id": "0240000032044e4500257009997b00386781000097969900"
    }
]

theotherjimmy

Looks great!

theotherjimmy · 2018-02-26T19:38:25Z

@bridadan Are you happy with this implementation?

bridadan · 2018-02-26T20:00:10Z

Yup!

jupe reviewed Jan 30, 2018

View reviewed changes

bridadan force-pushed the jlink_and_other_fw_detection branch from e6d3713 to b5b91ec Compare January 31, 2018 17:49

bridadan changed the title ~~[Proposal] Jlink and other fw detection~~ Jlink and other fw detection Feb 2, 2018

bridadan force-pushed the jlink_and_other_fw_detection branch from f44a467 to 52a60d7 Compare February 8, 2018 19:11

theotherjimmy suggested changes Feb 20, 2018

View reviewed changes

bridadan force-pushed the jlink_and_other_fw_detection branch from a02440d to ee98c12 Compare February 20, 2018 18:58

bridadan added 2 commits February 21, 2018 11:36

Adds support for device type in Platform DB

3161e51

WIP

85bf9b4

bridadan added 16 commits February 21, 2018 11:37

Platform database broken into daplink and jlink

71bd6fe

Detecting jlink and daplink devices

53d2c5a

Merging two functions for simplicity

9e79451

Getting tests passing again

f70453d

Adding tests for new device type detection

62517f2

Adding deprectation warning for 'read_details_txt' parameter

f8d8b6b

Logging warning of potential edge case of JLink devices

b412cf0

Renaming 'include_extra_info' back to 'read_details_txt'

ac629d3

Adding missing docstrings for functions

6d57fab

plat_db now provides jlink_device_name

3aa1962

Tests fixeds for new plat_db data

e8377cc

Adding back missing platforms

03c11dd

Adding test for missing case in plat_db

ca7c6c6

Fixing style changes

944901c

Consolidating calls to os.listdir

1a035a0

Updating tests with consolidated listdir behavior

ba99fc8

bridadan force-pushed the jlink_and_other_fw_detection branch from ee98c12 to ba99fc8 Compare February 21, 2018 17:38

theotherjimmy approved these changes Feb 26, 2018

View reviewed changes

theotherjimmy merged commit 44d819c into ARMmbed:master Feb 26, 2018

bridadan mentioned this pull request Feb 27, 2018

How to run tests over SWD? ARMmbed/mbed-cli#475

Closed

theotherjimmy mentioned this pull request Mar 1, 2018

Detect boards with JLink firmware #283

Closed

bridadan mentioned this pull request Jul 17, 2018

VID/PID based device type detection #370

Merged

Jlink and other fw detection #299

Jlink and other fw detection #299

Uh oh!

Conversation

bridadan commented Jan 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bridadan commented Jan 30, 2018

Uh oh!

bridadan commented Jan 30, 2018

Uh oh!

coveralls commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bridadan commented Feb 1, 2018

Uh oh!

bridadan commented Feb 1, 2018

Uh oh!

theotherjimmy commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bridadan commented Feb 1, 2018

Uh oh!

theotherjimmy commented Feb 1, 2018

Uh oh!

bridadan commented Feb 1, 2018

Uh oh!

theotherjimmy commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bridadan commented Feb 1, 2018

Uh oh!

theotherjimmy commented Feb 2, 2018

Uh oh!

juhhov commented Feb 5, 2018

Uh oh!

bridadan commented Feb 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juhhov commented Feb 6, 2018

Uh oh!

simtind commented Feb 6, 2018

Uh oh!

jupe commented Feb 6, 2018

Uh oh!

simtind commented Feb 6, 2018

Uh oh!

bridadan commented Feb 8, 2018

Uh oh!

bridadan commented Feb 8, 2018

Uh oh!

juhhov commented Feb 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy commented Feb 20, 2018

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bridadan commented Feb 21, 2018

Uh oh!

jupe commented Feb 24, 2018

Uh oh!

bridadan commented Jan 30, 2018 •

edited

Loading

coveralls commented Feb 1, 2018 •

edited

Loading

theotherjimmy commented Feb 1, 2018 •

edited

Loading

theotherjimmy commented Feb 1, 2018 •

edited

Loading

bridadan commented Feb 5, 2018 •

edited

Loading

juhhov commented Feb 20, 2018 •

edited

Loading