Reject a machine model name declared by two bundles

Preset::get_printer_type matches a preset's printer_model against every vendor's model names and returns the first hit, so two bundles declaring one name make the lookup order-dependent, and the Add Printer list shows the printer twice. The existing name check is per bundle, because base profiles share names across vendors by design; this one covers the global machine_model namespace and runs over the whole tree, like the setting_id and filament_id checks.
This commit is contained in:
SoftFever
2026-09-22 22:15:36 +08:00
parent 592a5ba777
commit 301f2ecba3
2 changed files with 81 additions and 2 deletions
+53 -2
View File
@@ -818,6 +818,55 @@ def check_setting_id_uniqueness(profiles_dir):
return errors
def check_machine_model_name_uniqueness(profiles_dir):
"""No two bundles may declare a machine_model with the same name.
A machine_model name is the key the whole tree resolves a printer type by:
Preset::get_printer_type (and get_current_printer_type) walk every vendor's
models and return the model_id of the first whose name equals the preset's
printer_model, so two models sharing a name make that lookup depend on vendor
order. The name is also what the Add Printer list shows, so a duplicate
renders the same printer twice.
Unlike preset names, which are per bundle - base profiles share one name
across dozens of bundles by design - a machine_model name is global. A vendor
copying another vendor's model (the Custom "Generic Klipper Printer" being the
usual source) is the common way this happens.
Cross-vendor by nature, so it always runs over the whole tree, never narrowed
by --vendor. Returns the error count.
"""
errors = 0
owners = defaultdict(list) # model name -> [relative path]
for vendor in list_profile_dirs(profiles_dir):
for path, _sub in iter_profile_files(os.path.join(profiles_dir, vendor)):
if os.path.basename(path) in NON_PROFILE_FILES:
continue
try:
data = load_json(path)
except (ValueError, OSError):
# Parse failures are reported by the checks that walk the same
# files; reporting them here too would double-count.
continue
if not isinstance(data, dict) or data.get("type") != "machine_model":
continue
name = data.get("name")
if name:
owners[name].append(
os.path.relpath(path, profiles_dir).replace(os.sep, "/"))
for name, paths in sorted(owners.items()):
if len(paths) < 2:
continue
errors += 1
print_error(
f'machine_model name "{name}" is declared by {len(paths)} bundles '
f'({", ".join(sorted(paths))}); a machine model name is global, so the '
f"printer type resolves to whichever bundle is seen first and the Add "
f"Printer list shows it twice - rename or delete the duplicate")
return errors
# ---------------------------------------------------------------------------
# Per-vendor validation
# ---------------------------------------------------------------------------
@@ -1353,9 +1402,11 @@ def check_profiles(profiles_dir=PROFILES_DIR, vendors=None):
if remedies[category]:
print_warning(f"{remedies[category]} {hint}")
# Cross-vendor checks: setting_id uniqueness and the whole filament_id state,
# both validated over the entire tree regardless of --vendor.
# Cross-vendor checks: setting_id and machine_model name uniqueness and the
# whole filament_id state, all validated over the entire tree regardless of
# --vendor.
errors_found += check_setting_id_uniqueness(profiles_dir)
errors_found += check_machine_model_name_uniqueness(profiles_dir)
errors_found += check_filament_ids(profiles_dir)
print("\n==================== SUMMARY ====================")
+28
View File
@@ -678,6 +678,34 @@ class TestCheck(TreeCase):
errors, out = self.names(vendor)
self.assertEqual(errors, 0, out)
def machine_models(self):
"""The cross-vendor machine_model name check, whole tree by design."""
buf = io.StringIO()
with contextlib.redirect_stdout(buf):
errors = apt.check_machine_model_name_uniqueness(self.t.profiles)
return errors, buf.getvalue()
def test_two_bundles_may_not_declare_one_machine_model_name(self):
# The name keys the global printer-type lookup: Preset::get_printer_type
# matches a preset's printer_model against every vendor's model names, so a
# copy of another vendor's model is ambiguous, not merely duplicated.
for vendor in ("V", "W"):
self.t.write(vendor, "machine/MyKlipper.json",
{"type": "machine_model", "name": "Generic Klipper Printer",
"model_id": "my_klipper_01"})
errors, out = self.machine_models()
self.assertEqual(errors, 1, out)
self.assertIn('machine_model name "Generic Klipper Printer"', out)
self.assertIn("V/machine/MyKlipper.json", out)
self.assertIn("W/machine/MyKlipper.json", out)
def test_distinct_machine_model_names_are_left_alone(self):
for vendor in ("V", "W"):
self.t.write(vendor, "machine/model.json",
{"type": "machine_model", "name": f"{vendor} Model"})
errors, out = self.machine_models()
self.assertEqual(errors, 0, out)
def coverage(self, vendor="V"):
"""The index-coverage check for one bundle: (errors, gaps, output)."""
buf = io.StringIO()