From 1b7a635938de220297871631032a5a849656f298 Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Wed, 21 Oct 2026 11:30:23 +0100 Subject: [PATCH] 20261021 - Stop shipping and passing on the tracker's dead minimum SNR retina-tracker v0.5.0 tracks every detection. Its per-site SNR gate is gone (retina-tracker ed9bbea), and a config that still sets min_snr is loaded and ignored. retina-gui stopped offering the field in #119, but default.yml still shipped min_snr: 7.0 and the merger still wrote it into retina-tracker.yaml, so every node was handed a setting nothing reads. Removing it from default.yml alone would reach no node. The merger seeds user.yml with a whole copy of default.yml on first boot, so every node on the estate still holds retina_tracker: {min_snr: 7.0}, or whatever retina-gui wrote there. migrate_remove_min_snr() drops the key from the user overlay at merge time, unconditionally like migrate_remove_tracker(), and drops the section once it is empty. user.yml on disk is left alone, so the migration stays load-bearing and a Mender rollback regenerates the old config from the old default.yml. Simply deleting the key would have left an empty retina_tracker section, which YAML reads as null and the merger would have written as tracker: null. retina-tracker's load_config() calls .update() on each section it finds, so that crashes it with TypeError and the tracker would crash-loop on every node. An empty or null section is now dropped by the migration, and generate_retina_tracker_config() treats one as nothing to override. It also writes the file on every run, as {} when there is nothing to override, because skipping it left the previous merge's file on disk still saying min_snr: 7.0. Tests: the three generic retina-tracker.yaml tests move off min_snr to gate_threshold, and new ones cover the first-boot copy and a value retina-gui wrote, another tracker key surviving, an empty and a null section, a stale file being overwritten, user.yml left untouched and the shipped default.yml. Every tracker file is loaded through a mirror of the tracker's own load_config(). Full merger suite 83 passed, ruff and the dead-code hook clean. On a test node, the real config-merger and retina-tracker v0.5.0 images were run against a copy of its user.yml: the merged config differed from the live one by retina_tracker.min_snr alone, the tracker loaded the {} file cleanly, and the same tracker crashed on a tracker: null file. Co-Authored-By: Claude Opus 5.5 (1M context) --- config-merger/script/merge_config.py | 58 ++++++++++-- config-merger/test/test_merge_config.py | 115 ++++++++++++++++++++---- config/default.yml | 16 ++-- 3 files changed, 156 insertions(+), 33 deletions(-) diff --git a/config-merger/script/merge_config.py b/config-merger/script/merge_config.py index 19c0a61..eb143dd 100755 --- a/config-merger/script/merge_config.py +++ b/config-merger/script/merge_config.py @@ -6,8 +6,8 @@ Merges default.yml + user.yml + forced.yml into config.yml. Also syncs the node_id field in user.yml from Mender device identity (/data/mender/node_id). Generates tar1090.env for tar1090-node if the tar1090 section exists in config. -Generates retina-tracker.yaml for the retina-tracker sidecar if the -retina_tracker section exists in config. +Generates retina-tracker.yaml for the retina-tracker sidecar on every run, from +the retina_tracker section if config has one and as an empty mapping if not. Takes as inputs: 1. Defaults directory - holds baked-in default.yml and forced.yml @@ -167,16 +167,24 @@ def generate_retina_tracker_config(config, output_dir): here are assumed to belong to retina-tracker's own `tracker` section; revisit this mapping if fields for its other sections (`process_noise`, `tracklet`, `adsb`, ...) are ever added to default.yml. + + Written on every run, as `{}` when there is nothing to override, so the + file always matches this merge. Skipping it left the last merge's file on + disk, which on every node said `min_snr: 7.0` long after that key was gone. + + Never writes `tracker: null`. load_config() would call .update(None) on it + and the tracker would crash-loop; an empty or non-mapping section counts as + nothing to override. """ - if 'retina_tracker' not in config: - print("No retina_tracker config found, skipping retina-tracker.yaml generation") - return + section = config.get('retina_tracker') + tracker = section if isinstance(section, dict) and section else None output_path = os.path.join(output_dir, 'retina-tracker.yaml') - print(f"Writing retina-tracker config to {output_path}") + print(f"Writing retina-tracker config to {output_path}" + + ("" if tracker else " (no overrides; the tracker uses its own defaults)")) write_file_atomic(output_path, yaml.dump( - {'tracker': config['retina_tracker']}, default_flow_style=False, sort_keys=False)) + {'tracker': tracker} if tracker else {}, default_flow_style=False, sort_keys=False)) print("retina-tracker.yaml generated successfully!") @@ -319,6 +327,41 @@ def migrate_remove_tracker(user): del ports['track'] +def migrate_remove_min_snr(user): + """Drop retina_tracker.min_snr, which no tracker reads any more. + + retina-tracker v0.5.0 tracks every detection: its per-site SNR gate is gone + (retina-tracker ed9bbea), and a config that still sets min_snr is loaded and + ignored. retina-gui no longer offers the field, and default.yml no longer + ships it, but that alone reaches no node: the merger seeds user.yml with a + whole copy of default.yml on first boot, so every node on the estate still + holds `retina_tracker: {min_snr: 7.0}`, or whatever retina-gui wrote there. + Same first-boot copy problem as migrate_remove_tracker(). + + Unconditional, like migrate_remove_tracker(): the code the key configured is + gone, so any value is equally dead. The section goes with it once it is empty, + so the merged config carries no `retina_tracker` at all rather than an empty + one. An empty or null section is dropped for the same reason: it would + otherwise reach the tracker as `tracker: null` (see + generate_retina_tracker_config()). + + Dropping the key rather than rewriting it leaves user.yml on disk untouched, + so this stays load-bearing rather than a one-shot fixup, and a Mender rollback + regenerates the old config from the old default.yml. + """ + if not isinstance(user, dict) or 'retina_tracker' not in user: + return + + section = user['retina_tracker'] + if isinstance(section, dict) and 'min_snr' in section: + print(f"Dropping retina_tracker.min_snr ({section['min_snr']}) from user config: " + "retina-tracker no longer has an SNR threshold") + del section['min_snr'] + + if not section: + del user['retina_tracker'] + + def migrate_adsb_truth_server(user): """Drop an ADS-B truth server the user never chose, so default.yml can change it. @@ -439,6 +482,7 @@ def main(): migrate_doppler_span(user) migrate_tracker_forward(user) migrate_remove_tracker(user) + migrate_remove_min_snr(user) migrate_adsb_truth_server(user) merge(config, user) else: diff --git a/config-merger/test/test_merge_config.py b/config-merger/test/test_merge_config.py index cc98402..932ce6f 100755 --- a/config-merger/test/test_merge_config.py +++ b/config-merger/test/test_merge_config.py @@ -574,25 +574,36 @@ def test_tar1090_env_uses_location_rx(self): self.assertIn('RECEIVER_LON=-122.3954', env_content) self.assertIn('RECEIVER_ALT=23', env_content) + def tracker_yaml(self): + """retina-tracker.yaml as the tracker itself would load it. + + Mirrors retina_tracker.config.load_config() (v0.5.0): it updates each of + its own sections from the file's, so a null section or a null file raises + there and the tracker crash-loops. Asserting through this catches that + here instead of on a node. + """ + loaded = self.read_yaml(os.path.join(self.config_dir, 'retina-tracker.yaml')) + tracker_defaults = {'tracker': {'m_threshold': 3, 'gate_threshold': 9.0}} + for key in tracker_defaults: + if key in loaded: + tracker_defaults[key].update(loaded[key]) + return loaded + def test_retina_tracker_yaml_generated(self): - """Test that retina-tracker.yaml is generated when retina_tracker config exists""" + """retina-tracker.yaml carries the retina_tracker section as `tracker`""" default_config = { 'process': {'detection': {'pfa': 0.001}}, - 'retina_tracker': {'min_snr': 7.0}, + 'retina_tracker': {'gate_threshold': 12.0}, } self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), default_config) self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) self.run_merge() - tracker_yaml_path = os.path.join(self.config_dir, 'retina-tracker.yaml') - self.assertTrue(os.path.exists(tracker_yaml_path), "retina-tracker.yaml should be generated") - - output = self.read_yaml(tracker_yaml_path) - self.assertEqual(output, {'tracker': {'min_snr': 7.0}}) + self.assertEqual(self.tracker_yaml(), {'tracker': {'gate_threshold': 12.0}}) - def test_retina_tracker_yaml_not_generated_without_config(self): - """Test that retina-tracker.yaml is not generated when retina_tracker config is missing""" + def test_retina_tracker_yaml_empty_without_config(self): + """With no retina_tracker section the file is still written, as {}""" default_config = { 'process': {'detection': {'pfa': 0.001}}, 'network': {'ip': '0.0.0.0'}, @@ -602,17 +613,15 @@ def test_retina_tracker_yaml_not_generated_without_config(self): self.run_merge() - tracker_yaml_path = os.path.join(self.config_dir, 'retina-tracker.yaml') - self.assertFalse(os.path.exists(tracker_yaml_path), - "retina-tracker.yaml should not be generated without retina_tracker config") + self.assertEqual(self.tracker_yaml(), {}) def test_retina_tracker_yaml_user_override(self): - """Test that user config overrides retina_tracker settings in retina-tracker.yaml""" + """user.yml overrides retina_tracker settings in retina-tracker.yaml""" default_config = { - 'retina_tracker': {'min_snr': 7.0}, + 'retina_tracker': {'gate_threshold': 12.0}, } user_config = { - 'retina_tracker': {'min_snr': 4.5}, + 'retina_tracker': {'gate_threshold': 15.0}, } self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), default_config) self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) @@ -620,9 +629,79 @@ def test_retina_tracker_yaml_user_override(self): self.run_merge() - tracker_yaml_path = os.path.join(self.config_dir, 'retina-tracker.yaml') - output = self.read_yaml(tracker_yaml_path) - self.assertEqual(output['tracker']['min_snr'], 4.5) + self.assertEqual(self.tracker_yaml()['tracker']['gate_threshold'], 15.0) + + # --- min_snr removal ---------------------------------------------------- + # retina-tracker v0.5.0 reads no SNR threshold. Every node's user.yml still + # holds the first-boot copy of it, so the merger drops it there. + + def write_min_snr_configs(self, user_tracker): + """A default.yml without retina_tracker, as shipped, and a user.yml holding one.""" + self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), + {'process': {'detection': {'pfa': 0.001}}}) + self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) + self.write_yaml(os.path.join(self.config_dir, 'user.yml'), + {'process': {'detection': {'pfa': 0.001}}, 'retina_tracker': user_tracker}) + + def test_min_snr_first_boot_copy_dropped(self): + """The first-boot 7.0, or any value retina-gui wrote, is gone from both outputs""" + for value in (7.0, 4.5): + with self.subTest(min_snr=value): + self.write_min_snr_configs({'min_snr': value}) + + output = self.read_yaml(self.run_merge()) + + self.assertNotIn('retina_tracker', output) + self.assertEqual(self.tracker_yaml(), {}) + + def test_min_snr_dropped_other_tracker_keys_kept(self): + """Only min_snr goes; a real tracker override in the same section survives""" + self.write_min_snr_configs({'min_snr': 4.5, 'gate_threshold': 12.0}) + + output = self.read_yaml(self.run_merge()) + + self.assertEqual(output['retina_tracker'], {'gate_threshold': 12.0}) + self.assertEqual(self.tracker_yaml(), {'tracker': {'gate_threshold': 12.0}}) + + def test_empty_or_null_tracker_section_never_reaches_the_tracker_as_null(self): + """An empty or null section is dropped, never written as `tracker: null`""" + for section in ({}, None): + with self.subTest(section=section): + self.write_min_snr_configs(section) + + output = self.read_yaml(self.run_merge()) + + self.assertNotIn('retina_tracker', output) + self.assertEqual(self.tracker_yaml(), {}) + + def test_stale_tracker_yaml_overwritten(self): + """A file left by an earlier merge is replaced, not left saying min_snr""" + self.write_min_snr_configs({'min_snr': 7.0}) + self.write_yaml(os.path.join(self.config_dir, 'retina-tracker.yaml'), + {'tracker': {'min_snr': 7.0}}) + + self.run_merge() + + self.assertEqual(self.tracker_yaml(), {}) + + def test_min_snr_migration_does_not_rewrite_user_yml(self): + """user.yml on disk is untouched, so the migration stays load-bearing""" + self.write_min_snr_configs({'min_snr': 7.0}) + user_path = os.path.join(self.config_dir, 'user.yml') + with open(user_path) as f: + before = f.read() + + self.run_merge() + + with open(user_path) as f: + self.assertEqual(f.read(), before) + + def test_shipped_default_has_no_min_snr(self): + """The repo's own default.yml no longer ships a retina_tracker section""" + repo_root = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + with open(os.path.join(repo_root, 'config', 'default.yml')) as f: + shipped = yaml.safe_load(f) + self.assertNotIn('retina_tracker', shipped) # --- Doppler span migration ------------------------------------------- diff --git a/config/default.yml b/config/default.yml index 62ab3b4..bcb9d23 100644 --- a/config/default.yml +++ b/config/default.yml @@ -128,11 +128,11 @@ tar1090: # service goes first and adsb.lol sits behind it. adsb_upstreams: "https://adsb.retina.fm,https://api.adsb.lol" -# retina-tracker sidecar (github.com/offworldlabs/retina-tracker) config. -# blah2 has no built-in tracker of its own; this sidecar does all tracking. -retina_tracker: - # dB. Detections below this are discarded before tracking even begins - - # too high and the tracker silently never confirms a track no matter how - # good the search geometry is. Tune to this node's real noise floor - # (compare against SNR values from blah2_api's /api/detection). - min_snr: 7.0 +# retina-tracker sidecar (github.com/offworldlabs/retina-tracker): no section, +# so it runs on its own built-in defaults. blah2 has no built-in tracker; this +# sidecar does all tracking. A `retina_tracker:` section added here is written +# to retina-tracker.yaml as the tracker's `tracker` section; see +# generate_retina_tracker_config() in config-merger/script/merge_config.py. +# min_snr is gone for good: retina-tracker v0.5.0 tracks every detection and +# reads no SNR threshold, and the merger drops it from user.yml +# (migrate_remove_min_snr).