Workaround for buggy 'return' in older OpenOCD

noxfile.py

@@ -73,7 +73,7 @@ def code_quality(session):
 def tests_unit(session):
     session.install(".")
     session.install("pytest")
-    session.run("python3", "-m", "pytest", "tests_unit/", "-vv")
+    session.run("python3", "-m", "pytest", "tests_unit/", "-vv", *session.posargs)
 
 
 @nox.session
@@ -84,7 +84,15 @@ def tests_unit_coverage(session):
 
     session.install("pytest", "coverage")
     session.run(
-        "python3", "-m", "coverage", "run", "-m", "pytest", "tests_unit/", "-vv"
+        "python3",
+        "-m",
+        "coverage",
+        "run",
+        "-m",
+        "pytest",
+        "tests_unit/",
+        "-vv",
+        *session.posargs,
     )
     session.run("python3", "-m", "coverage", "xml")
     session.run("python3", "-m", "coverage", "html")

src/py_openocd_client/client.py

@@ -211,25 +211,34 @@ def cmd(
             raw_cmd = cmd
 
         raw_cmd = "set CMD_RETCODE [ catch { " + raw_cmd + " } CMD_OUTPUT ] ; "
-        raw_cmd += 'return "$CMD_RETCODE $CMD_OUTPUT" ; '
+
+        # Older OpenOCD versions - prior to 93f16eed4(*) - incorrectly trimmed trailing
+        # whitespace from the string passed to the return command. Work around this bug
+        # by wrapping the string by non-whitespace characters.
+        # (*): https://review.openocd.org/c/openocd/+/9084
+        raw_cmd += 'return "<$CMD_RETCODE,$CMD_OUTPUT>" ; '
 
         raw_result = self.raw_cmd(raw_cmd, timeout=timeout)
 
-        # Verify the raw output from OpenOCD the has the expected format. It can be:
-        #
-        # - Command return code (positive or negative decimal number) and that's it.
-        #
-        # - Or, command return code (positive or negative decimal number) followed by
-        #   a space character and optionally followed by the command's textual output.
-        if re.match(r"^-?\d+($| )", raw_result) is None:
+        def is_expected_raw_result(s: str) -> bool:
+            return (
+                s.startswith("<")
+                and s.endswith(">")
+                and re.match(r"^<-?\d+,", s) is not None
+            )
+
+        if not is_expected_raw_result(raw_result):
             msg = (
                 "Received unexpected response from OpenOCD. "
                 "It looks like OpenOCD misbehaves. "
             )
             raise OcdInvalidResponseError(msg, raw_cmd, raw_result)
 
-        raw_result_parts = raw_result.split(" ", maxsplit=1)
-        assert len(raw_result_parts) in [1, 2]
+        # Remove leading "<" and trailing ">"
+        raw_result = raw_result[1:-1]
+        raw_result_parts = raw_result.split(",", maxsplit=1)
+        assert len(raw_result_parts) == 2
+
         retcode = int(raw_result_parts[0], 10)
         out = raw_result_parts[1] if len(raw_result_parts) == 2 else ""
 

tests_integration/conftest.py

@@ -33,7 +33,7 @@ def _is_tcp_port_open(port):
 
 
 def _wait_until(predicate, timeout):
-    """Wait until given predicate becomes true"""
+    """Wait until the given predicate becomes true"""
     time_start = time.time()
     while True:
         if predicate():
@@ -81,9 +81,10 @@ def openocd_version(pytestconfig):
 
 
 @pytest.fixture
-def has_buggy_whitespace_trim(openocd_version):
+def has_return_whitespace_bug(openocd_version):
     """
-    Detect if the OpenOCD version being tested has a known buggy whitespace handling.
+    Return if the OpenOCD version being tested has a known buggy whitespace handling
+    in the "return" command.
 
     In OpenOCD prior to version 0.13.0, the "return" command performed extra
     whitespace trimming on the command output. This was fixed in commit "93f16eed4,
@@ -97,6 +98,14 @@ def has_buggy_whitespace_trim(openocd_version):
     ]
 
 
+def pytest_sessionstart(session):
+    if _is_tcp_port_open(TCL_PORT_NUM):
+        raise RuntimeError(
+            f"TCP port {TCL_PORT_NUM} is already occupied. Please terminate any "
+            "OpenOCD processes before starting the integration tests."
+        )
+
+
 @pytest.fixture
 def openocd_process(openocd_path):
 

tests_integration/test_integration_cmd.py

@@ -18,24 +18,56 @@ def test_expr(openocd_process):
         assert result.out == "6"
 
 
+def test_string_concat(openocd_process):
+    with PyOpenocdClient() as ocd:
+        result = ocd.cmd('string repeat "abc" 3')
+        assert result.out == "abcabcabc"
+
+
+def test_string_concat_whitespace(openocd_process):
+    with PyOpenocdClient() as ocd:
+        result = ocd.cmd('string repeat " abc " 3')
+        assert result.out == " abc  abc  abc "
+
+
+def test_string_concat_multiline(openocd_process):
+    with PyOpenocdClient() as ocd:
+        result = ocd.cmd('string repeat " abc\\n " 3')
+        assert result.out == " abc\n  abc\n  abc\n "
+
+
 def test_echo(openocd_process):
     with PyOpenocdClient() as ocd:
         result = ocd.cmd("echo {some text}")
         assert result.retcode == 0
         assert result.out == ""  # because capture was not set
 
 
-def test_echo_with_capture(openocd_process, has_buggy_whitespace_trim):
+def test_echo_with_capture(openocd_process):
     with PyOpenocdClient() as ocd:
         result = ocd.cmd("echo {some text}", capture=True)
         assert result.retcode == 0
 
         # "echo" appends \n at the end
-        if has_buggy_whitespace_trim:
-            assert result.out == "some text"
-            pytest.xfail("known OpenOCD whitespace bug")
-        else:
-            assert result.out == "some text\n"
+        assert result.out == "some text\n"
+
+
+def test_echo_with_capture_whitespace(openocd_process):
+    with PyOpenocdClient() as ocd:
+        result = ocd.cmd('echo "  some text "', capture=True)
+        assert result.retcode == 0
+
+        # "echo" appends \n at the end
+        assert result.out == "  some text \n"
+
+
+def test_echo_with_capture_multiline(openocd_process):
+    with PyOpenocdClient() as ocd:
+        result = ocd.cmd('echo " \\nabc \\n d ef \\n  ghi  \\n "', capture=True)
+        assert result.retcode == 0
+
+        # "echo" appends \n at the end
+        assert result.out == " \nabc \n d ef \n  ghi  \n \n"
 
 
 def test_failed_cmd(openocd_process):
@@ -81,16 +113,12 @@ def test_failed_cmd_dont_throw(openocd_process):
         assert "invalid command" in result.out
 
 
-def test_assign_variable_and_read_by_echo(openocd_process, has_buggy_whitespace_trim):
+def test_assign_variable_and_read_by_echo(openocd_process):
     with PyOpenocdClient() as ocd:
         ocd.cmd("set MY_VARIALBLE 123456")
         result = ocd.cmd("echo $MY_VARIALBLE", capture=True)
-
-        if has_buggy_whitespace_trim:
-            assert result.out == "123456"
-            pytest.xfail("known OpenOCD whitespace bug")
-        else:
-            assert result.out == "123456\n"
+        # "echo" appends \n at the end
+        assert result.out == "123456\n"
 
 
 def test_assign_variable_and_read_by_set(openocd_process):
@@ -141,12 +169,13 @@ def test_timeout_exceeded(openocd_process):
 
         expected_raw_cmd = (
             "set CMD_RETCODE [ catch { sleep 2000 } CMD_OUTPUT ] ; "
-            'return "$CMD_RETCODE $CMD_OUTPUT" ; '
+            'return "<$CMD_RETCODE,$CMD_OUTPUT>" ; '
         )
         assert e.value.raw_cmd == expected_raw_cmd
         assert e.value.timeout == 1.0
 
-        # Timeout causes re-connection, we must remain connected.
+        # Timeout causes re-connection internally.
+        # From the user perspective, we must remain connected.
         assert ocd.is_connected()
 
         # Commands must still work

tests_integration/test_integration_raw_cmd.py

@@ -26,17 +26,52 @@ def test_return(openocd_process):
         assert out == "abcdef"
 
 
-def test_return_with_whitespace(openocd_process, has_buggy_whitespace_trim):
+def test_return_with_whitespace(openocd_process, has_return_whitespace_bug):
     with PyOpenocdClient() as ocd:
         out = ocd.raw_cmd("return {  abc 4567 }")
 
-        if has_buggy_whitespace_trim:
+        if has_return_whitespace_bug:
             assert out == "abc 4567"
             pytest.xfail("known OpenOCD whitespace bug")
         else:
             assert out == "  abc 4567 "
 
 
+def test_return_with_newline(openocd_process, has_return_whitespace_bug):
+    with PyOpenocdClient() as ocd:
+        out = ocd.raw_cmd('return " \\n abc \\n4567 "')
+
+        if has_return_whitespace_bug:
+            assert out == "abc \n4567"
+            pytest.xfail("known OpenOCD whitespace bug")
+        else:
+            assert out == " \n abc \n4567 "
+
+
+def test_string_concat_with_whitespace(openocd_process):
+    with PyOpenocdClient() as ocd:
+        out = ocd.raw_cmd("string repeat { a } 4")
+        assert out == " a  a  a  a "
+
+
+def test_string_concat_with_newline(openocd_process):
+    with PyOpenocdClient() as ocd:
+        out = ocd.raw_cmd('string repeat " abc\\n " 4')
+        assert out == " abc\n  abc\n  abc\n  abc\n "
+
+
+def test_string_concat_with_newline_and_return(
+    openocd_process, has_return_whitespace_bug
+):
+    with PyOpenocdClient() as ocd:
+        out = ocd.raw_cmd('return [string repeat " abc\\n " 4]')
+        if has_return_whitespace_bug:
+            assert out == "abc\n  abc\n  abc\n  abc"
+            pytest.xfail("known OpenOCD whitespace bug")
+        else:
+            assert out == " abc\n  abc\n  abc\n  abc\n "
+
+
 def test_nonexistent_cmd(openocd_process):
     with PyOpenocdClient() as ocd:
         out = ocd.raw_cmd("nonexistent_cmd")
@@ -53,6 +88,7 @@ def test_echo_capture(openocd_process):
 def test_echo_capture_whitespace(openocd_process):
     with PyOpenocdClient() as ocd:
         out = ocd.raw_cmd("capture { echo { 123 456 } }")
+        # echo appends \n at the end
         assert out == " 123 456 \n"
 
 
@@ -74,45 +110,44 @@ def test_catch_throw(openocd_process):
         assert int(out) == 22  # error code
 
 
-def _parse_out(out):
-    # "5 some text" -> (5, "some text")
-    parts = out.split(" ", maxsplit=1)
+def _parse_out(out: str) -> tuple[int, str]:
+    # "<5,some text>" -> (5, "some text")
+    assert out.startswith("<")
+    assert out.endswith(">")
+    out = out[1:-1]
+    parts = out.split(",", maxsplit=1)
+    assert len(parts) == 2
     return int(parts[0]), parts[1]
 
 
 def test_catch_output_and_success(openocd_process):
     with PyOpenocdClient() as ocd:
-        cmd = 'set RETCODE [ catch { version } OUT ]; return "$RETCODE $OUT" '
+        cmd = 'set RETCODE [ catch { version } OUT ]; return "<$RETCODE,$OUT>" '
         out = ocd.raw_cmd(cmd)
         retcode, text = _parse_out(out)
 
         assert retcode == 0  # success code
         assert "Open On-Chip Debugger" in text
 
 
-def test_catch_output_and_success_whitespace(
-    openocd_process, has_buggy_whitespace_trim
-):
+def test_catch_output_and_success_whitespace(openocd_process):
     with PyOpenocdClient() as ocd:
         cmd = (
             "set RETCODE [catch { string repeat { a } 4 } OUT]; "
-            'return "$RETCODE $OUT"'
+            'return "<$RETCODE,$OUT>"'
         )
         out = ocd.raw_cmd(cmd)
         retcode, text = _parse_out(out)
 
         assert retcode == 0  # success code
-
-        if has_buggy_whitespace_trim:
-            assert text == " a  a  a  a"
-            pytest.xfail("known OpenOCD whitespace bug")
-        else:
-            assert text == " a  a  a  a "
+        assert text == " a  a  a  a "
 
 
 def test_catch_output_and_error(openocd_process):
     with PyOpenocdClient() as ocd:
-        cmd = 'set RETCODE [ catch { nonexistent_cmd } OUT; ]; return "$RETCODE $OUT" '
+        cmd = (
+            'set RETCODE [ catch { nonexistent_cmd } OUT; ]; return "<$RETCODE,$OUT>" '
+        )
         out = ocd.raw_cmd(cmd)
         retcode, text = _parse_out(out)
 
@@ -122,28 +157,25 @@ def test_catch_output_and_error(openocd_process):
 
 def test_catch_output_and_throw(openocd_process):
     with PyOpenocdClient() as ocd:
-        cmd = 'set RETCODE [catch { throw 25 {my msg} } OUT;]; return "$RETCODE $OUT"'
+        cmd = 'set RETCODE [catch { throw 25 {my msg} } OUT;]; return "<$RETCODE,$OUT>"'
         out = ocd.raw_cmd(cmd)
         retcode, text = _parse_out(out)
 
         assert retcode == 25  # error code
         assert text == "my msg"
 
 
-def test_catch_output_and_throw_whitespace(openocd_process, has_buggy_whitespace_trim):
+def test_catch_output_and_throw_whitespace(openocd_process):
     with PyOpenocdClient() as ocd:
         cmd = (
-            'set RETCODE [catch { throw 25 { my msg  } } OUT;]; return "$RETCODE $OUT"'
+            "set RETCODE [catch { throw 25 { my msg  } } OUT;]; "
+            'return "<$RETCODE,$OUT>"'
         )
         out = ocd.raw_cmd(cmd)
         retcode, text = _parse_out(out)
 
         assert retcode == 25  # error code
-        if has_buggy_whitespace_trim:
-            assert text == " my msg"
-            pytest.xfail("known OpenOCD whitespace bug")
-        else:
-            assert text == " my msg  "
+        assert text == " my msg  "
 
 
 def test_raw_cmd_timeout_ok(openocd_process):