From a07ac72cc585fcf5c3805c82834e1f55edab2ca3 Mon Sep 17 00:00:00 2001 From: Jeff Epler Date: Thu, 24 Mar 2022 09:42:11 -0500 Subject: [PATCH] Improve the USB vid:pid duplicate checker To me, it made more sense to track which boards go together in a cluster; when reviewing a request to actually use a duplicate vid/pid, you want to know what board(s) it is aliasing. I also revamped the detection of non-USB boards so that a board .mk file that couldn't be parsed by the code here would raise a problem instead of just being skipped for the purposes of checking. There were some lines with comments on the end, and some variation in capitalization of the IDs. These are all normalized and a (sometimes unfriendly!) error printed when it's incorrect. Before this, here were some ways to trick the duplicate vid/pid checker: ``` USB_PID = 0XABCD USB_PID = 0xAbCd USB_PID = 0xABCD # harmless comment? ``` None of these things were ever done on purpose. --- .../sparkfun_samd51_micromod/mpconfigboard.mk | 3 +- .../mpconfigboard.mk | 3 +- .../boards/particle_xenon/mpconfigboard.mk | 3 +- tools/ci_check_duplicate_usb_vid_pid.py | 127 ++++++++---------- 4 files changed, 61 insertions(+), 75 deletions(-) diff --git a/ports/atmel-samd/boards/sparkfun_samd51_micromod/mpconfigboard.mk b/ports/atmel-samd/boards/sparkfun_samd51_micromod/mpconfigboard.mk index 3ae3d8f5a9..026a1978c3 100644 --- a/ports/atmel-samd/boards/sparkfun_samd51_micromod/mpconfigboard.mk +++ b/ports/atmel-samd/boards/sparkfun_samd51_micromod/mpconfigboard.mk @@ -1,6 +1,7 @@ LD_FILE = boards/samd51x20-bootloader-external-flash.ld USB_VID = 0x1b4f -USB_PID = 0x0020 # Used by uf2 bootloader +# Used by uf2 bootloader +USB_PID = 0x0020 USB_PRODUCT = "SparkFun MicroMod SAMD51 Processor" USB_MANUFACTURER = "SparkFun Electronics" diff --git a/ports/atmel-samd/boards/sparkfun_samd51_thing_plus/mpconfigboard.mk b/ports/atmel-samd/boards/sparkfun_samd51_thing_plus/mpconfigboard.mk index e33035d949..4b66bc7420 100644 --- a/ports/atmel-samd/boards/sparkfun_samd51_thing_plus/mpconfigboard.mk +++ b/ports/atmel-samd/boards/sparkfun_samd51_thing_plus/mpconfigboard.mk @@ -1,6 +1,7 @@ LD_FILE = boards/samd51x20-bootloader-external-flash.ld USB_VID = 0x1b4f -USB_PID = 0x0016 # Used by uf2 bootloader +# Used by uf2 bootloader +USB_PID = 0x0016 USB_PRODUCT = "SparkFun SAMD51 Thing+" USB_MANUFACTURER = "SparkFun Electronics" diff --git a/ports/nrf/boards/particle_xenon/mpconfigboard.mk b/ports/nrf/boards/particle_xenon/mpconfigboard.mk index 0722c4ac65..f6b801c9eb 100644 --- a/ports/nrf/boards/particle_xenon/mpconfigboard.mk +++ b/ports/nrf/boards/particle_xenon/mpconfigboard.mk @@ -1,5 +1,6 @@ USB_VID = 0x2b04 -USB_PID = 0xc00e # argon is 0xc00c +# argon is 0xc00c +USB_PID = 0xc00e USB_PRODUCT = "Xenon" USB_MANUFACTURER = "Particle" diff --git a/tools/ci_check_duplicate_usb_vid_pid.py b/tools/ci_check_duplicate_usb_vid_pid.py index 51eac31f03..6becd5bd7e 100644 --- a/tools/ci_check_duplicate_usb_vid_pid.py +++ b/tools/ci_check_duplicate_usb_vid_pid.py @@ -24,57 +24,40 @@ # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. +from collections import defaultdict import argparse import pathlib import re import sys -DEFAULT_IGNORELIST = [ - "circuitplayground_express", - "circuitplayground_express_crickit", - "circuitplayground_express_displayio", - "pycubed", - "pycubed_mram", - "pycubed_v05", - "pycubed_mram_v05", - "pygamer", - "pygamer_advance", - "trinket_m0", - "trinket_m0_haxpress", - "sparkfun_qwiic_micro_with_flash", - "sparkfun_qwiic_micro_no_flash", - "feather_m0_express", - "feather_m0_supersized", - "cp32-m4", - "metro_m4_express", - "unexpectedmaker_feathers2", - "unexpectedmaker_feathers2_prerelease", - "espressif_kaluga_1", - "espressif_kaluga_1.3", - "espressif_esp32s2_devkitc_1_n4r2", - "espressif_esp32s3_devkitc_1_n8", - "espressif_esp32s3_devkitc_1_n8r2", - "espressif_esp32s3_devkitc_1_n8r8", - "espressif_saola_1_wrover", - "jpconstantineau_pykey18", - "jpconstantineau_pykey44", - "jpconstantineau_pykey60", - "jpconstantineau_pykey87", -] +DEFAULT_CLUSTERLIST = { + "0x04D8:0xEC44": ["pycubed", "pycubed_mram", "pycubed_mram_v05", "pycubed_v05"], + "0x1B4F:0x8D24": ["sparkfun_qwiic_micro_no_flash", "sparkfun_qwiic_micro_with_flash"], + "0x1D50:0x6153": [ + "jpconstantineau_pykey18", + "jpconstantineau_pykey44", + "jpconstantineau_pykey60", + "jpconstantineau_pykey87", + ], + "0x239A:0x8019": [ + "circuitplayground_express", + "circuitplayground_express_crickit", + "circuitplayground_express_displayio", + ], + "0x239A:0x801F": ["trinket_m0_haxpress", "trinket_m0"], + "0x239A:0x8021": ["metro_m4_express", "cp32-m4"], + "0x239A:0x8023": ["feather_m0_express", "feather_m0_supersized"], + "0x239A:0x80A6": ["espressif_esp32s2_devkitc_1_n4r2", "espressif_saola_1_wrover"], + "0x239A:0x80AC": ["unexpectedmaker_feathers2", "unexpectedmaker_feathers2_prerelease"], + "0x239A:0x80C8": ["espressif_kaluga_1", "espressif_kaluga_1.3"], + "0x303A:0x7003": [ + "espressif_esp32s3_devkitc_1_n8", + "espressif_esp32s3_devkitc_1_n8r2", + "espressif_esp32s3_devkitc_1_n8r8", + ], +} cli_parser = argparse.ArgumentParser(description="USB VID/PID Duplicate Checker") -cli_parser.add_argument( - "--ignorelist", - dest="ignorelist", - nargs="?", - action="store", - default=DEFAULT_IGNORELIST, - help=( - "Board names to ignore duplicate VID/PID combinations. Pass an empty " - "string to disable all duplicate ignoring. Defaults are: " - f"{', '.join(DEFAULT_IGNORELIST)}" - ), -) def configboard_files(): @@ -87,48 +70,49 @@ def configboard_files(): return working_dir.glob("ports/**/boards/**/mpconfigboard.mk") -def check_vid_pid(files, ignorelist): +def check_vid_pid(files, clusterlist): """Compiles a list of USB VID & PID values for all boards, and checks for duplicates. Exits with ``sys.exit()`` (non-zero exit code) if duplicates are found, and lists the duplicates. """ - duplicates_found = False - - usb_ids = {} - - vid_pattern = re.compile(r"^USB_VID\s*\=\s*(.*)", flags=re.M) - pid_pattern = re.compile(r"^USB_PID\s*\=\s*(.*)", flags=re.M) + vid_pattern = re.compile(r"^USB_VID\s*=\s*(.*)", flags=re.M) + pid_pattern = re.compile(r"^USB_PID\s*=\s*(.*)", flags=re.M) + usb_pattern = re.compile(r"^CIRCUITPY_USB\s*=\s*0$|^IDF_TARGET = esp32c3$", flags=re.M) + usb_ids = defaultdict(set) for board_config in files: src_text = board_config.read_text() usb_vid = vid_pattern.search(src_text) usb_pid = pid_pattern.search(src_text) - + non_usb = usb_pattern.search(src_text) board_name = board_config.parts[-2] - board_ignorelisted = False - if board_name in ignorelist: - board_ignorelisted = True - board_name += " (ignorelisted)" - if usb_vid and usb_pid: - id_group = f"{usb_vid.group(1)}:{usb_pid.group(1)}" - if id_group not in usb_ids: - usb_ids[id_group] = {"boards": [board_name], "duplicate": False} + id_group = f"0x{int(usb_vid.group(1), 16):04X}:0x{int(usb_pid.group(1), 16):04X}" + elif non_usb: + continue + else: + raise SystemExit(f"Could not parse {board_config}") + + usb_ids[id_group].add(board_name) + + duplicates = [] + for key, boards in usb_ids.items(): + if len(boards) == 1: + continue + + # It is a cluster + cluster = set(clusterlist.get(key, [])) + if cluster != boards: + if key == "": + duplicates.append(f"- Non-USB:\n" f" Boards: {', '.join(sorted(boards))}") else: - usb_ids[id_group]["boards"].append(board_name) - if not board_ignorelisted: - usb_ids[id_group]["duplicate"] = True - duplicates_found = True - - if duplicates_found: - duplicates = "" - for key, value in usb_ids.items(): - if value["duplicate"]: - duplicates += f"- VID/PID: {key}\n" f" Boards: {', '.join(value['boards'])}\n" + duplicates.append(f"- VID/PID: {key}\n" f" Boards: {', '.join(sorted(boards))}") + if duplicates: + duplicates = "\n".join(duplicates) duplicate_message = ( f"Duplicate VID/PID usage found!\n{duplicates}\n" f"If you are open source maker, then you can request a PID from http://pid.codes\n" @@ -143,7 +127,6 @@ if __name__ == "__main__": arguments = cli_parser.parse_args() print("Running USB VID/PID Duplicate Checker...") - print(f"Ignoring the following boards: {', '.join(arguments.ignorelist)}", end="\n\n") board_files = configboard_files() - check_vid_pid(board_files, arguments.ignorelist) + check_vid_pid(board_files, DEFAULT_CLUSTERLIST)