targets: standardise target type name normalisation (#1432)

This change fixes issues using DFP targets with dashes or other
punctuation in the name.

Author: flit

pyocd/board/board.py

@@ -1,6 +1,6 @@
 # pyOCD debugger
 # Copyright (c) 2006-2013,2018 Arm Limited
-# Copyright (c) 2021 Chris Reed
+# Copyright (c) 2021-2022 Chris Reed
 # SPDX-License-Identifier: Apache-2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -19,7 +19,7 @@
 from typing import (Any, Optional, TYPE_CHECKING)
 
 from ..core import exceptions
-from ..target import TARGET
+from ..target import (TARGET, normalise_target_type_name)
 from ..target.pack import pack_target
 from ..utility.graph import GraphNode
 
@@ -84,7 +84,7 @@ def __init__(self,
         assert target is not None
 
         # Convert dashes to underscores in the target type, and convert to lower case.
-        target = target.replace('-', '_').lower()
+        target = normalise_target_type_name(target)
 
         # Write the effective target type back to options if it's different.
         if target != session.options.get('target_override'):

pyocd/target/__init__.py

@@ -1,5 +1,6 @@
 # pyOCD debugger
 # Copyright (c) 2013-2019 Arm Limited
+# Copyright (c) 2022 Chris Reed
 # SPDX-License-Identifier: Apache-2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -14,10 +15,35 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import string
+
 from .builtin import BUILTIN_TARGETS
 
 ## @brief Dictionary of all targets.
 #
 # This table starts off with only the builtin targets. At runtime it may be extended with
 # additional targets from CMSIS DFPs or other sources.
 TARGET = BUILTIN_TARGETS.copy()
+
+## @brief Legal characters in target type names.
+#
+# Basically, C language identifier characters.
+_TARGET_TYPE_NAME_CHARS = string.ascii_letters + string.digits + '_'
+
+def normalise_target_type_name(target_type: str) -> str:
+    """@brief Normalise a target type name.
+
+    The output string has all non-ASCII alphanumeric characters replaced with underscores and is
+    converted to all lowercase. Only one underscore in a row will be inserted in the output. For example,
+    "foo--bar" will be normalised to "foo_bar".
+    """
+    result = ""
+    in_replace = False
+    for c in target_type:
+        if c in _TARGET_TYPE_NAME_CHARS:
+            result += c.lower()
+            in_replace = False
+        elif not in_replace:
+            result += '_'
+            in_replace = True
+    return result

pyocd/target/builtin/__init__.py

@@ -124,8 +124,9 @@
 
 ## @brief Dictionary of all builtin targets.
 #
-# @note Target type names must be all lowercase and use _underscores_ instead of dashes. The code in Board
-#   automatically converts dashes in user-supplied target type names to underscores.
+# @note Target type names must be a valid C identifier, normalised to all lowercase, using _underscores_
+#   instead of dashes punctuation. See pyocd.target.normalise_target_type_name() for the code that
+#   normalises user-provided target type names for comparison with these.
 BUILTIN_TARGETS = {
           'mps3_an522': target_MPS3_AN522.AN522,
           'mps3_an540': target_MPS3_AN540.AN540,

pyocd/target/pack/pack_target.py

@@ -24,6 +24,7 @@
 from .. import TARGET
 from ...coresight.coresight_target import CoreSightTarget
 from ...debug.svd.loader import SVDFile
+from .. import normalise_target_type_name
 
 if TYPE_CHECKING:
     from zipfile import ZipFile
@@ -96,10 +97,10 @@ def populate_target(device_name: str) -> None:
         device part number is used to find the target to populate. If multiple packs are installed
         that provide the same part numbers, all matching targets will be populated.
         """
-        device_name = device_name.lower()
+        device_name = normalise_target_type_name(device_name)
         targets = ManagedPacks.get_installed_targets()
         for dev in targets:
-            if device_name == dev.part_number.lower():
+            if device_name == normalise_target_type_name(dev.part_number):
                 PackTargets.populate_device(dev)
 
 if CPM_AVAILABLE:
@@ -171,7 +172,7 @@ def _generate_pack_target_class(dev: CmsisPackDevice) -> Optional[type]:
             superklass = PackTargets._find_family_class(dev)
 
             # Replace spaces and dashes with underscores on the new target subclass name.
-            subclassName = dev.part_number.replace(' ', '_').replace('-', '_')
+            subclassName = normalise_target_type_name(dev.part_number).capitalize()
 
             # Create a new subclass for this target.
             targetClass = type(subclassName, (superklass,), {
@@ -197,7 +198,7 @@ def populate_device(dev: CmsisPackDevice) -> None:
             tgt = PackTargets._generate_pack_target_class(dev)
             if tgt is None:
                 return
-            part = dev.part_number.lower().replace("-", "_")
+            part = normalise_target_type_name(dev.part_number)
 
             # Make sure there isn't a duplicate target name.
             if part not in TARGET:
@@ -249,7 +250,7 @@ def is_pack_target_available(target_name: str, session: "Session") -> bool:
     if session.options['pack'] is not None:
         target_types = []
         def collect_target_type(dev: CmsisPackDevice) -> None:
-            part = dev.part_number.lower().replace("-", "_")
+            part = normalise_target_type_name(dev.part_number)
             target_types.append(part)
         PackTargets.process_targets_from_pack(session.options['pack'], collect_target_type)
         return target_name.lower() in target_types

test/unit/test_cmdline.py

@@ -1,5 +1,6 @@
 # pyOCD debugger
 # Copyright (c) 2015,2018-2019 Arm Limited
+# Copyright (c) 2022 Chris Reed
 # SPDX-License-Identifier: Apache-2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -24,6 +25,7 @@
     convert_session_options,
     )
 from pyocd.core.target import Target
+from pyocd.target import normalise_target_type_name
 
 class TestSplitCommandLine(object):
     def test_split(self):
@@ -141,4 +143,19 @@ def test_str(self):
         # Valid
         assert convert_session_options(['test_binary=abc']) == {'test_binary': 'abc'}
 
+class TestTargetTypeNormalisation:
+    def test_passthrough(self):
+        assert normalise_target_type_name("foobar") == "foobar"
+        assert normalise_target_type_name("foo123bar") == "foo123bar"
+        assert normalise_target_type_name("foo_bar") == "foo_bar"
+
+    def test_lower(self):
+        assert normalise_target_type_name("TARGET") == "target"
+        assert normalise_target_type_name("BIGtarget") == "bigtarget"
+        assert normalise_target_type_name("someTARGET123") == "sometarget123"
+
+    def test_trans(self):
+        assert normalise_target_type_name("foo-bar") == "foo_bar"
+        assert normalise_target_type_name("foo--bar") == "foo_bar"
+        assert normalise_target_type_name("foo!@#$%^&*()x") == "foo_x"