Skip to content

Commit ba60ffa

Browse files
committed
Add CI check for duplicate firmware_image_type
On PRs, only entries changed vs the base are checked so the pre-existing duplicates don't block; an --all mode (wired up with the dedup PR) guards main.
1 parent bf1059e commit ba60ffa

3 files changed

Lines changed: 140 additions & 1 deletion

File tree

.github/workflows/test.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,20 @@ jobs:
2323
sudo apt-get install -y make python3 python3-pytest
2424
- name: Run tests
2525
run: make tests
26+
27+
image-types-changed:
28+
# On PRs, check only entries changed vs the target branch, so pre-existing
29+
# duplicates don't turn this permanently red. Re-fetches the base at run
30+
# time, so re-running the PR validates against current main.
31+
if: github.event_name == 'pull_request'
32+
runs-on: ubuntu-latest
33+
steps:
34+
- uses: actions/checkout@v3
35+
with:
36+
fetch-depth: 0
37+
- name: Install dependencies
38+
run: sudo apt-get update && sudo apt-get install -y make python3 && pip install pyyaml
39+
- name: Check changed firmware_image_type entries
40+
run: |
41+
git fetch origin ${{ github.base_ref }}
42+
make tools/check_image_types_changed BASE_REF=origin/${{ github.base_ref }}
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
"""Check device_db.yaml for duplicate firmware_image_type values.
2+
3+
Each device must own a unique firmware_image_type. OTA clients (notably zigpy /
4+
Home Assistant ZHA) match an image by manufacturerCode + imageType and refuse
5+
to choose when two images share both, so a duplicate silently breaks OTA for
6+
the colliding devices.
7+
8+
Modes:
9+
--changed BASE_REF
10+
Only devices whose firmware_image_type is new or changed versus BASE_REF
11+
are checked. Pre-existing duplicates do not fail, so this never turns red
12+
on unrelated PRs; but no PR can claim an image type that is already taken.
13+
Use on pull requests.
14+
15+
--all
16+
Every device is checked; fails if ANY firmware_image_type is shared.
17+
Use on the main branch to assert the whole db stays clean.
18+
"""
19+
import argparse
20+
import subprocess
21+
import sys
22+
23+
import yaml
24+
25+
DB = "device_db.yaml"
26+
27+
28+
def image_types(db):
29+
result = {}
30+
for name, fields in (db or {}).items():
31+
if not isinstance(fields, dict):
32+
continue
33+
image_type = fields.get("firmware_image_type")
34+
if image_type in (None, "null"):
35+
continue
36+
result[name] = image_type
37+
return result
38+
39+
40+
def load_worktree():
41+
with open(DB) as f:
42+
return yaml.safe_load(f)
43+
44+
45+
def load_ref(ref):
46+
blob = subprocess.run(
47+
["git", "show", f"{ref}:{DB}"], capture_output=True, text=True
48+
)
49+
if blob.returncode != 0:
50+
print(f"warning: cannot read {DB} from {ref}, treating base as empty")
51+
return {}
52+
return yaml.safe_load(blob.stdout)
53+
54+
55+
def owners_by_type(head):
56+
owners = {}
57+
for name, image_type in head.items():
58+
owners.setdefault(image_type, []).append(name)
59+
return owners
60+
61+
62+
def fail(collisions):
63+
print("firmware_image_type collisions:\n")
64+
for image_type, devices in sorted(collisions.items()):
65+
print(f" {image_type}: {', '.join(sorted(devices))}")
66+
print("\nEach device needs a unique firmware_image_type.")
67+
print("Get the next free id with: make tools/unused_image_type")
68+
sys.exit(1)
69+
70+
71+
def check_all(head):
72+
owners = owners_by_type(head)
73+
collisions = {it: devs for it, devs in owners.items() if len(devs) > 1}
74+
if collisions:
75+
fail(collisions)
76+
print(f"OK: {len(head)} devices, all firmware_image_type values unique")
77+
78+
79+
def check_changed(head, base_ref):
80+
base = image_types(load_ref(base_ref))
81+
owners = owners_by_type(head)
82+
changed = [name for name, it in head.items() if base.get(name) != it]
83+
collisions = {}
84+
for name in changed:
85+
others = [o for o in owners[head[name]] if o != name]
86+
if others:
87+
collisions.setdefault(head[name], set()).update([name, *others])
88+
if collisions:
89+
fail(collisions)
90+
print(f"OK: {len(changed)} added/changed entries checked against "
91+
f"{base_ref}, no image_type collisions")
92+
93+
94+
def main():
95+
parser = argparse.ArgumentParser(
96+
description="Check firmware_image_type uniqueness in device_db.yaml")
97+
group = parser.add_mutually_exclusive_group(required=True)
98+
group.add_argument("--all", action="store_true",
99+
help="check every device (use on main)")
100+
group.add_argument("--changed", metavar="BASE_REF",
101+
help="check only entries changed vs BASE_REF (use on PRs)")
102+
args = parser.parse_args()
103+
104+
head = image_types(load_worktree())
105+
if args.all:
106+
check_all(head)
107+
else:
108+
check_changed(head, args.changed)
109+
110+
111+
if __name__ == "__main__":
112+
main()

tools.mk

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ help:
1919
@echo ""
2020
@echo " Development Utilities:"
2121
@echo " unused_image_type - Show next available firmware image type ID"
22+
@echo " check_image_types_changed - Fail if a changed device reuses an existing image type (PRs)"
23+
@echo " check_image_types_all - Fail if the whole db has any duplicate image type (main)"
2224
@echo ""
2325

2426

@@ -58,4 +60,12 @@ freeze_ota_links:
5860

5961

6062
unused_image_type:
61-
@yq '[.[] | .firmware_image_type] | max + 1' device_db.yaml
63+
@yq '[.[] | .firmware_image_type] | max + 1' device_db.yaml
64+
65+
66+
check_image_types_changed:
67+
python3 helper_scripts/check_image_types.py --changed $(BASE_REF)
68+
69+
70+
check_image_types_all:
71+
python3 helper_scripts/check_image_types.py --all

0 commit comments

Comments
 (0)