ci(usb-ids): check board USB IDs on PRs that change them

The workflow triggers on pull requests that touch a board defconfig or
the USB ID tooling, and checks only the defconfigs the PR adds or
modifies. The file list comes from diffing the checked-out PR merge
commit against its base parent, so no API call or token is needed. A PR that changes the tooling itself is checked against every
board, so a checker change is proven on the real tree and not only by
its unit tests. A test keeps the tooling list and the workflow paths
filter in sync.

A manual run checks every board. There is no push or scheduled run: the
registry repo checks its own changes against PX4 main, and board
changes reach main only through PRs.

The mypy and flake8 step for the checker and runner lives in
python_checks.yml with the other Python tooling checks.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Ramon Roche <mrpollo@gmail.com>

.github/workflows/python_checks.yml

@@ -27,7 +27,7 @@ jobs:
         python-version: "3.10"
 
     - name: Install tools
-      run: pip install mypy types-requests flake8 pyyaml
+      run: pip install mypy types-requests types-PyYAML flake8 pyyaml
 
     - name: Check MAVSDK test scripts with mypy
       run: mypy --strict test/mavsdk_tests/*.py
@@ -35,6 +35,12 @@ jobs:
     - name: Check MAVSDK test scripts with flake8
       run: flake8 test/mavsdk_tests/*.py
 
+    - name: Check USB ID tooling with mypy and flake8
+      working-directory: Tools/ci
+      run: |
+        mypy --strict check_usb_ids.py usb_ids_runner.py test_check_usb_ids.py test_usb_ids_runner.py
+        flake8 check_usb_ids.py usb_ids_runner.py test_check_usb_ids.py test_usb_ids_runner.py
+
     - name: Check ROS workspace tooling
       run: |
         flake8 Tools/ros2/*.py

.github/workflows/usb_ids.yml

@@ -0,0 +1,42 @@
+name: Dronecode USB ID Registry
+
+on:
+  pull_request:
+    branches:
+      - '**'
+    # Keep in sync with TOOLING in Tools/ci/usb_ids_runner.py
+    paths:
+      - 'boards/*/*/nuttx-config/*/defconfig'
+      - '.github/workflows/usb_ids.yml'
+      - 'Tools/ci/check_usb_ids.py'
+      - 'Tools/ci/test_check_usb_ids.py'
+      - 'Tools/ci/test_usb_ids_runner.py'
+      - 'Tools/ci/usb_ids_runner.py'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  check:
+    name: Board USB VID/PID must be registered
+    runs-on: ubuntu-24.04
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          # the PR merge commit and its base parent, to diff the PR locally
+          fetch-depth: 2
+
+      - name: Install PyYAML
+        run: pip install pyyaml --break-system-packages
+
+      - name: Test the USB ID checker and runner
+        working-directory: Tools/ci
+        run: python3 -m unittest -v test_check_usb_ids test_usb_ids_runner
+
+      - name: Check USB IDs against the Dronecode registry
+        run: python3 Tools/ci/usb_ids_runner.py

Tools/ci/test_usb_ids_runner.py

@@ -0,0 +1,140 @@
+"""Check which defconfigs usb_ids_runner.py hands to the checker.
+
+A selection bug fails open: if a PR resolves to the wrong file list, a
+changed board is never checked and the job still goes green. PR diffs
+run against a real throwaway git repo shaped like the checkout
+actions/checkout makes for a pull request: a merge commit whose first
+parent is the base branch.
+"""
+
+import contextlib
+import io
+import os
+from pathlib import Path
+import subprocess
+import tempfile
+from typing import List, Tuple
+import unittest
+from unittest import mock
+
+import yaml
+
+import check_usb_ids
+import usb_ids_runner
+from usb_ids_runner import changed_files
+
+WORKFLOW = (Path(__file__).resolve().parents[2]
+            / '.github/workflows/usb_ids.yml')
+ALL = ['boards/px4/fmu-v6xrt/nuttx-config/nsh/defconfig']
+PR_ENV = {'GITHUB_EVENT_NAME': 'pull_request'}
+KEPT = 'boards/siyi/n7/nuttx-config/nsh/defconfig'
+GONE = 'boards/old/fc/nuttx-config/nsh/defconfig'
+
+
+class ChangedFilesTest(unittest.TestCase):
+    def setUp(self) -> None:
+        patcher = mock.patch.object(usb_ids_runner, 'all_defconfigs',
+                                    return_value=ALL)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(tmp.cleanup)
+        self.repo = Path(tmp.name)
+        cwd = os.getcwd()
+        os.chdir(self.repo)
+        self.addCleanup(os.chdir, cwd)
+        self.git('init', '-q', '-b', 'base')
+        self.write(GONE)
+        self.write('README.md')
+        self.git('add', '.')
+        self.git('commit', '-q', '-m', 'base')
+
+    def git(self, *args: str) -> None:
+        subprocess.run(['git', '-c', 'user.name=t', '-c', 'user.email=t@t',
+                        *args], check=True, capture_output=True)
+
+    def write(self, path: str) -> None:
+        (self.repo / path).parent.mkdir(parents=True, exist_ok=True)
+        (self.repo / path).write_text(path)
+
+    def merge_pr(self, *files: str) -> None:
+        """Commit files on a PR branch that also removes GONE, then merge
+        it into base with a merge commit, as the PR checkout does."""
+        self.git('checkout', '-q', '-b', 'pr')
+        for f in files:
+            self.write(f)
+        self.git('rm', '-q', GONE)
+        self.git('add', '.')
+        self.git('commit', '-q', '-m', 'pr')
+        self.git('checkout', '-q', 'base')
+        self.write('base-moved-on.txt')
+        self.git('add', '.')
+        self.git('commit', '-q', '-m', 'base moves on')
+        self.git('merge', '-q', '--no-ff', '-m', 'merge', 'pr')
+
+    def test_manual_run_checks_the_full_tree(self) -> None:
+        env = {'GITHUB_EVENT_NAME': 'workflow_dispatch'}
+        self.assertEqual(changed_files(env), (ALL, True))
+
+    def test_pull_request_diffs_against_base_without_removed(self) -> None:
+        # base's own later commits and the PR's deletion are left out
+        self.merge_pr(KEPT)
+        self.assertEqual(changed_files(PR_ENV), ([KEPT], False))
+
+    def test_tooling_change_checks_the_full_tree(self) -> None:
+        self.merge_pr(KEPT, 'Tools/ci/check_usb_ids.py')
+        self.assertEqual(changed_files(PR_ENV), (ALL, True))
+
+    def test_shallow_checkout_without_base_fails_loudly(self) -> None:
+        # HEAD has no parent here, as with the default fetch-depth 1
+        with self.assertRaises(SystemExit) as cm:
+            changed_files(PR_ENV)
+        self.assertIn('fetch-depth', str(cm.exception.code))
+
+    def test_tooling_matches_the_workflow_paths_filter(self) -> None:
+        # a file missing from the filter never triggers the workflow, and
+        # one missing from TOOLING never forces the full-tree check
+        on = yaml.safe_load(WORKFLOW.read_text())[True]
+        paths = set(on['pull_request']['paths'])
+        self.assertEqual(paths - {'boards/*/*/nuttx-config/*/defconfig'},
+                         set(usb_ids_runner.TOOLING))
+
+
+class MainTest(unittest.TestCase):
+    def run_main(self, paths: List[str]
+                 ) -> Tuple[int, str, mock.MagicMock, mock.MagicMock]:
+        out = io.StringIO()
+        env = {'GITHUB_EVENT_NAME': 'workflow_dispatch'}
+        with mock.patch.dict(os.environ, env, clear=True), \
+                mock.patch.object(usb_ids_runner, 'changed_files',
+                                  return_value=(paths, False)), \
+                mock.patch.object(check_usb_ids, 'load_registry') as load, \
+                mock.patch.object(check_usb_ids, 'cmd_check',
+                                  return_value=0) as check, \
+                contextlib.redirect_stdout(out):
+            rc = usb_ids_runner.main()
+        return rc, out.getvalue(), load, check
+
+    def test_only_board_defconfigs_reach_the_checker(self) -> None:
+        wanted = ['boards/px4/fmu-v6xrt/nuttx-config/nsh/defconfig',
+                  'boards/siyi/n7/nuttx-config/bootloader/defconfig']
+        ignored = ['boards/px4/fmu-v6xrt/default.px4board',
+                   'boards/px4/fmu-v6xrt/nuttx-config/nsh/defconfig.orig',
+                   'boards/px4/defconfig',
+                   'platforms/nuttx/NuttX/nuttx/boards/arm/stm32/x/configs/'
+                   'nsh/defconfig',
+                   'Tools/ci/usb_ids_runner.py']
+        rc, _, _, check = self.run_main(ignored + wanted)
+        self.assertEqual(rc, 0)
+        self.assertEqual(check.call_args.args[1], wanted)
+
+    def test_no_defconfigs_exits_before_fetching_the_registry(self) -> None:
+        rc, out, load, check = self.run_main(['Tools/ci/usb_ids_runner.py'])
+        self.assertEqual(rc, 0)
+        self.assertIn('No board defconfig changes', out)
+        load.assert_not_called()
+        check.assert_not_called()
+
+
+if __name__ == '__main__':
+    unittest.main()

Tools/ci/usb_ids_runner.py

@@ -0,0 +1,78 @@
+#!/usr/bin/env python3
+"""Check board defconfigs against the Dronecode USB ID registry
+(https://github.com/Dronecode/usb-ids).
+
+Meant for the usb_ids.yml workflow; run from the repository root.
+A pull request checks the defconfigs it adds or modifies, or every board
+defconfig when it changes the USB ID tooling itself. A manual run checks
+every board defconfig.
+
+On a pull request, actions/checkout checks out the test merge commit,
+whose first parent is the base branch; the checkout needs fetch-depth 2
+so the PR's changes can be diffed locally against it.
+"""
+
+import glob
+import os
+import re
+import subprocess
+import sys
+from typing import List, Mapping, Tuple
+
+import check_usb_ids
+
+DEFCONFIG_RE = re.compile(r'^boards/[^/]+/[^/]+/nuttx-config/.+/defconfig$')
+# Keep in sync with the pull_request paths filter in usb_ids.yml
+TOOLING = frozenset((
+    '.github/workflows/usb_ids.yml',
+    'Tools/ci/check_usb_ids.py',
+    'Tools/ci/test_check_usb_ids.py',
+    'Tools/ci/test_usb_ids_runner.py',
+    'Tools/ci/usb_ids_runner.py',
+))
+
+
+def all_defconfigs() -> List[str]:
+    return sorted(glob.glob('boards/*/*/nuttx-config/*/defconfig'))
+
+
+def pr_files() -> List[str]:
+    """Files the checked-out PR merge commit adds or modifies."""
+    diff = subprocess.run(
+        ['git', 'diff', '--name-only', '--diff-filter=d', 'HEAD^1', 'HEAD'],
+        capture_output=True, text=True)
+    if diff.returncode != 0:
+        sys.exit(f'error: cannot diff the PR merge commit against its base '
+                 f'(is fetch-depth 2?): {diff.stderr.strip()}')
+    return diff.stdout.splitlines()
+
+
+def changed_files(env: Mapping[str, str]) -> Tuple[List[str], bool]:
+    """Return (paths, full_tree) for the triggering event."""
+    if env.get('GITHUB_EVENT_NAME') != 'pull_request':
+        return all_defconfigs(), True
+
+    files = pr_files()
+    if TOOLING.intersection(files):
+        # a change to the checker itself is proven against every board
+        return all_defconfigs(), True
+    return files, False
+
+
+def main() -> int:
+    paths, full_tree = changed_files(os.environ)
+    defconfigs = [p for p in paths if DEFCONFIG_RE.match(p)]
+    if not defconfigs:
+        print('No board defconfig changes, nothing to check.')
+        return 0
+
+    if full_tree:
+        print(f'Checking all {len(defconfigs)} board defconfigs.')
+    else:
+        print('Changed defconfigs:')
+        print('\n'.join(defconfigs))
+    return check_usb_ids.cmd_check(check_usb_ids.load_registry(), defconfigs)
+
+
+if __name__ == '__main__':
+    sys.exit(main())