Resource: add Segger J-Link support #1175

PaulVittorino · 2023-05-08T21:41:09Z

Description
Add a resource for the Segger J-Link debug probe.
Export the J-Link resource over IP using the J-Link Remote Server.
Adding support for J-Link over IP combined with pylink provides a Python interface for the J-Link over IP.

Verified by acquiring a place via the Python API and using pylink to establish an IP connection.

resource = target.get_resource(labgrid.resource.remote.NetworkJLinkDevice)
host, port = labgrid.util.proxy.proxymanager.get_host_and_port(resource)
jlink = pylink.JLink()
jlink.open(ip_addr=f"{ip_addr}:{port}")

Supported hardware includes Segger J-Link JTAG probes. Tested with J-Link Ultra+ Version 5.0 hardware.

Checklist

Documentation for the feature
Tests for the feature

The arguments and description in doc/configuration.rst have been updated
Add a section on how to use the feature to doc/usage.rst (N/A)

Add a section on how to use the feature to doc/development.rst (N/A)

PR has been tested

Man pages have been regenerated (N/A)

codecov · 2023-05-16T05:32:18Z

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (2906645) 63.0% compared to head (53cdba7) 63.4%.
Report is 74 commits behind head on master.

Files	Patch %	Lines
labgrid/remote/exporter.py	95.5%	2 Missing ⚠️
labgrid/resource/udev.py	80.0%	2 Missing ⚠️
labgrid/resource/suggest.py	0.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           master   #1175     +/-   ##
========================================
+ Coverage    63.0%   63.4%   +0.4%     
========================================
  Files         160     161      +1     
  Lines       11865   11959     +94     
========================================
+ Hits         7480    7588    +108     
+ Misses       4385    4371     -14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

PaulVittorino · 2023-05-17T02:12:26Z

I added testing of USBJLinkExport to try and meet the code coverage goal.

jluebbe

For new resources, there should also be a corresponding driver. As there is already a Python module which provides access, only a very simple driver is needed to load it and wrap the open method. Take a look at these:

PaulVittorino · 2023-05-25T13:33:33Z

For new resources, there should also be a corresponding driver. As there is already a Python module which provides access, only a very simple driver is needed to load it and wrap the open method. Take a look at these:
* https://github.com/labgrid-project/labgrid/blob/master/labgrid/driver/modbusdriver.py

* https://github.com/labgrid-project/labgrid/blob/master/labgrid/driver/modbusrtudriver.py

@jluebbe driver added with 100% code coverage.

PaulVittorino · 2023-06-07T18:25:05Z

@jluebbe does my driver meet your expectations?

Bastian-Krause · 2023-06-21T11:06:12Z

labgrid/remote/exporter.py

+        child = self.child
+        self.child = None
+        port = self.port
+        self.port = None
+        child.terminate()
+        try:
+            child.wait(2.0)  # JLinkRemoteServer takes about a second to react
+        except subprocess.TimeoutExpired:
+            self.logger.warning("JLinkRemoteServer for %s still running after SIGTERM", self.local.serial)
+            log_subprocess_kernel_stack(self.logger, child)
+            child.kill()
+            child.wait(1.0)
+        self.logger.info("stopped JLinkRemoteServer for %s on port %d", self.local.serial, port)


When operating with pipes (stdout=subprocess.PIPE), they should be closed explicitly when the
subprocess has terminated (see also #1018).

PaulVittorino · 2023-07-13T23:49:24Z

labgrid/resource/udev.py

+    """
+
+    def __attrs_post_init__(self):
+        self.match["ID_VENDOR_ID"] = "1366"


Since USBDebugger now covers the same vendor & model ID as JLinkDevice, do I need to refactor to use USBDebugger instead?

Yes, please do so.

Add a driver and resource for the Segger J-Link debug probe. Export the J-Link resource over IP using the J-Link Remote Server. Signed-off-by: Paul Vittorino <paul.vittorino@garmin.com>

Bastian-Krause · 2023-12-15T11:54:18Z

CHANGES.rst

-  
+


Seems unrelated.

Bastian-Krause · 2023-12-15T11:54:39Z

doc/configuration.rst

+  - `JLinkDriver`_
+
+NetworkJLinkDevice
+~~~~~~~~~~~~~~~~~~~~~


Suggested change

~~~~~~~~~~~~~~~~~~~~~

~~~~~~~~~~~~~~~~~~

Bastian-Krause · 2023-12-15T12:57:10Z

labgrid/driver/jlinkdriver.py

+            # Workaround for Debian's /etc/hosts entry
+            # https://www.debian.org/doc/manuals/debian-reference/ch05.en.html#_the_hostname_resolution
+            if ip_addr == "127.0.1.1":
+                ip_addr = "127.0.0.1"


Why can't we use 127.0.1.1 here?

Bastian-Krause · 2023-12-15T13:04:51Z

labgrid/resource/udev.py

+    """
+
+    def __attrs_post_init__(self):
+        self.match["ID_VENDOR_ID"] = "1366"


Yes, please do so.

Bastian-Krause · 2023-12-15T14:13:25Z

labgrid/remote/exporter.py

+class USBJLinkExport(USBGenericExport):
+    """Export J-Link device using the J-Link Remote Server"""
+
+    def __attrs_post_init__(self):
+        super().__attrs_post_init__()
+        self.child = None
+        self.port = None
+        self.tool = '/opt/SEGGER/JLink/JLinkRemoteServer'
+
+    def _get_params(self):
+        """Helper function to return parameters"""
+        return {
+            'host': self.host,
+            'port': self.port,
+            'busnum': self.local.busnum,
+            'devnum': self.local.devnum,
+            'path': self.local.path,
+            'vendor_id': self.local.vendor_id,
+            'model_id': self.local.model_id,
+        }
+
+    def __del__(self):
+        if self.child is not None:
+            self.stop()
+
+    def _start(self, start_params):
+        """Start ``JLinkRemoteServer`` subprocess"""
+        assert self.local.avail
+        assert self.child is None
+        self.port = get_free_port()
+
+        cmd = [
+            self.tool,
+            "-Port",
+            f"{self.port}",
+            "-select",
+            f"USB={self.local.serial}",
+        ]
+        self.logger.info("Starting JLinkRemoteServer with: %s", " ".join(cmd))
+        self.child = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True)
+
+        # Wait for the server to be ready for incoming connections
+        # Waiting to open the socket with Python does not work
+        timeout = Timeout(10.0)
+        while not timeout.expired:
+            line = self.child.stdout.readline().rstrip()
+            self.logger.debug(line)
+            if "Waiting for client connections..." in line:
+                break
+
+        if timeout.expired:
+            raise TIMEOUT(
+                f"Timeout of {timeout.timeout} seconds exceeded during waiting for J-Link Remote Server startup"
+            )
+        self.logger.info("started JLinkRemoteServer for %s on port %d", self.local.serial, self.port)
+
+    def _stop(self, start_params):
+        """Stop ``JLinkRemoteServer`` subprocess"""
+        assert self.child
+        child = self.child
+        self.child = None
+        port = self.port
+        self.port = None
+        child.terminate()
+        try:
+            child.communicate(2.0)  # JLinkRemoteServer takes about a second to react
+        except subprocess.TimeoutExpired:
+            self.logger.warning("JLinkRemoteServer for %s still running after SIGTERM", self.local.serial)
+            log_subprocess_kernel_stack(self.logger, child)
+            child.kill()
+            child.communicate(1.0)
+        self.logger.info("stopped JLinkRemoteServer for %s on port %d", self.local.serial, port)
+
+


This should not be required. The driver should start the remote server on activate and stop it on deactivate.

Bastian-Krause · 2023-12-15T14:20:48Z

labgrid/resource/udev.py

+
+    def update(self):
+        super().update()
+        self.serial = self.device.properties.get('ID_SERIAL_SHORT')


self.device might be None here. Maybe make it a @property?

Bastian-Krause · 2023-12-15T14:23:33Z

labgrid/driver/jlinkdriver.py

+@target_factory.reg_driver
+@attr.s(eq=False)
+class JLinkDriver(Driver):
+    bindings = {"jlink_device": {"JLinkDevice", "NetworkJLinkDevice"}, }


Maybe s/jlink_device/device/? Since we know that we are in the jlink context anyways.

Bastian-Krause · 2023-12-15T14:25:05Z

doc/configuration.rst

@@ -1789,6 +1809,21 @@ Arguments:
  - board_config (str): optional, board config in the ``openocd/scripts/board/`` directory
  - load_commands (list of str): optional, load commands to use instead of ``init``, ``bootstrap {filename}``, ``shutdown``

+JLinkDriver
+~~~~~~~~~~~~~


Suggested change

~~~~~~~~~~~~~

~~~~~~~~~~~

PaulVittorino force-pushed the jlink branch from 4a7da3c to 791c2c9 Compare May 9, 2023 13:47

PaulVittorino force-pushed the jlink branch from 791c2c9 to 6f050c1 Compare May 17, 2023 02:07

jluebbe requested changes May 17, 2023

View reviewed changes

jluebbe added enhancement work in progress labels May 17, 2023

PaulVittorino force-pushed the jlink branch 4 times, most recently from 8ed0405 to 753a3b3 Compare May 18, 2023 15:03

PaulVittorino requested a review from jluebbe May 19, 2023 16:40

Bastian-Krause reviewed Jun 21, 2023

View reviewed changes

PaulVittorino force-pushed the jlink branch from 684ca88 to ef8e8ed Compare July 13, 2023 23:26

PaulVittorino commented Jul 13, 2023

View reviewed changes

PaulVittorino requested a review from Bastian-Krause August 10, 2023 13:15

Driver/Resource: Add Segger J-Link support

53cdba7

Add a driver and resource for the Segger J-Link debug probe. Export the J-Link resource over IP using the J-Link Remote Server. Signed-off-by: Paul Vittorino <paul.vittorino@garmin.com>

PaulVittorino force-pushed the jlink branch from ef8e8ed to 53cdba7 Compare October 9, 2023 13:49

ep1cman mentioned this pull request Nov 8, 2023

add Trace32 debugger support #1267

Draft

7 tasks

Bastian-Krause self-assigned this Dec 15, 2023

Bastian-Krause requested changes Dec 15, 2023

View reviewed changes

Bastian-Krause assigned PaulVittorino and unassigned Bastian-Krause Dec 15, 2023

Bastian-Krause added needs rebase Needs a rebase onto the master branch, maintainter could probably not push to submitter branch. and removed work in progress labels Dec 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resource: add Segger J-Link support #1175

Resource: add Segger J-Link support #1175

PaulVittorino commented May 8, 2023 •

edited

codecov bot commented May 16, 2023 •

edited

PaulVittorino commented May 17, 2023

jluebbe left a comment

PaulVittorino commented May 25, 2023

PaulVittorino commented Jun 7, 2023

Bastian-Krause Jun 21, 2023 •

edited

PaulVittorino Jul 13, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Bastian-Krause Dec 15, 2023

Resource: add Segger J-Link support #1175

Are you sure you want to change the base?

Resource: add Segger J-Link support #1175

Conversation

PaulVittorino commented May 8, 2023 • edited

codecov bot commented May 16, 2023 • edited

Codecov Report

PaulVittorino commented May 17, 2023

jluebbe left a comment

Choose a reason for hiding this comment

PaulVittorino commented May 25, 2023

PaulVittorino commented Jun 7, 2023

Bastian-Krause Jun 21, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PaulVittorino commented May 8, 2023 •

edited

codecov bot commented May 16, 2023 •

edited

Bastian-Krause Jun 21, 2023 •

edited