From 3fd42087223f35eff0c2f8bcbe6bbc49b4ea6bc6 Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Thu, 27 Aug 2026 10:13:43 +0100 Subject: [PATCH] 20260827 - Ship no location by default default.yml shipped a complete, plausible geometry: Greenwich Observatory as the receiver, Crystal Palace as the illuminator. The merger writes it on first boot, so from power-on every consumer saw a populated location and none could tell an unconfigured node from a configured one. Nodes registered with the server claiming to sit in south-east London. Keys stay, with null values, rather than being removed. Consumers need to tell "unset" from "absent because this config is malformed", and retina-gui's config form renders from these keys. The merger was substituting 0 for a missing coordinate, which is not a neutral value: it is Null Island, a real place the node then claimed to be. Worse, ADSBLOL_ENABLED stayed true, so the adsb.lol query asked for aircraft within ADSBLOL_RADIUS of 0,0 and fed them to blah2 as ground truth. The receiver block is now omitted when unset, and adsb.lol is forced off whatever adsblol_fallback says. Omitting the vars does not fully undo the zero on its own, because tar1090-node's compose uses ${RECEIVER_LAT:-0} and its proxy does parseFloat(... || '0'). Disabling adsb.lol is what actually prevents false truth; the omission stops us asserting a position we do not have. Coordinates are tested against None rather than for truthiness, so an owner who genuinely sets 0,0 keeps it. DEPLOY LAST. Everything that reads this config has to tolerate a null first: retina-telemetry's _require raised, blah2-api dereferenced it unguarded, and retina-gui's schema rejected it. All three are fixed on their own branches; this is the change that makes the null real. test_actual_config_files pinned default.yml's real coordinates and caught this, which is what it is for. It now asserts the new intent. Co-Authored-By: Claude Opus 5 --- config-merger/script/merge_config.py | 29 +++++--- config-merger/test/test_merge_config.py | 89 +++++++++++++++++++++++-- config/default.yml | 27 +++++--- 3 files changed, 123 insertions(+), 22 deletions(-) diff --git a/config-merger/script/merge_config.py b/config-merger/script/merge_config.py index 387611b..1372d36 100755 --- a/config-merger/script/merge_config.py +++ b/config-merger/script/merge_config.py @@ -57,29 +57,38 @@ def generate_env_file(config, output_dir): tar1090_config = config['tar1090'] # Use tar1090.location if set, otherwise fall back to location.rx (receiver location) - location = tar1090_config.get('location') or config.get('location', {}).get('rx', {}) + location = tar1090_config.get('location') or (config.get('location') or {}).get('rx') or {} env_path = os.path.join(output_dir, 'tar1090.env') print(f"Writing tar1090 .env to {env_path}") # Write to temp file first, then atomic rename temp_path = env_path + '.tmp.' + str(os.getpid()) - lat = location.get('latitude', 0) - lon = location.get('longitude', 0) - alt = location.get('altitude', 0) + lat = location.get('latitude') + lon = location.get('longitude') + alt = location.get('altitude') + sited = lat is not None and lon is not None with open(temp_path, 'w') as f: f.write("# Auto-generated by config-merger - do not edit manually\n\n") - # Receiver location (matches tar1090-node .env.example format) + # Receiver location (matches tar1090-node .env.example format). + # Omitted when unset. This used to default to 0, which is Null Island: + # a real place the node then claimed to be. f.write("# Receiver location\n") - f.write(f"RECEIVER_LAT={lat}\n") - f.write(f"RECEIVER_LON={lon}\n") - f.write(f"RECEIVER_ALT={alt}\n\n") + if sited: + f.write(f"RECEIVER_LAT={lat}\n") + f.write(f"RECEIVER_LON={lon}\n") + f.write(f"RECEIVER_ALT={alt if alt is not None else 0}\n\n") + else: + f.write("# unset: no receiver location has been configured\n\n") - # adsb.lol integration + # adsb.lol integration. Forced off without a location whatever the + # config says: the query is a radius around the receiver, so with no + # receiver there is no query to make, only a wrong one. + adsblol = bool(tar1090_config.get('adsblol_fallback', False)) and sited f.write("# adsb.lol integration\n") - f.write(f"ADSBLOL_ENABLED={'true' if tar1090_config.get('adsblol_fallback', False) else 'false'}\n") + f.write(f"ADSBLOL_ENABLED={'true' if adsblol else 'false'}\n") f.write(f"ADSBLOL_RADIUS={tar1090_config.get('adsblol_radius', 40)}\n\n") # External ADS-B feed for readsb diff --git a/config-merger/test/test_merge_config.py b/config-merger/test/test_merge_config.py index d710d8e..e507ae1 100755 --- a/config-merger/test/test_merge_config.py +++ b/config-merger/test/test_merge_config.py @@ -275,6 +275,78 @@ def test_tar1090_env_adsblol_disabled(self): self.assertIn('ADSBLOL_ENABLED=false', env_content) + def test_tar1090_env_omits_an_unset_location(self): + """This used to write RECEIVER_LAT=0, which is Null Island: a real + place the node then claimed to be.""" + self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), { + 'tar1090': {'adsblol_fallback': True, 'adsblol_radius': 40}, + 'location': {'rx': {'latitude': None, 'longitude': None, + 'altitude': None, 'name': None}}, + }) + self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) + + self.run_merge() + + with open(os.path.join(self.config_dir, 'tar1090.env')) as f: + env_content = f.read() + + self.assertNotIn('RECEIVER_LAT=', env_content) + # The adsb.lol query is a radius around the receiver. With no receiver + # there is no query to make, only a wrong one feeding blah2 false truth. + self.assertIn('ADSBLOL_ENABLED=false', env_content) + + def test_tar1090_env_partial_location_is_not_sited(self): + """A latitude with no longitude is not a position.""" + self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), { + 'tar1090': {'adsblol_fallback': True, 'adsblol_radius': 40}, + 'location': {'rx': {'latitude': 42.241528, 'longitude': None}}, + }) + self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) + + self.run_merge() + + with open(os.path.join(self.config_dir, 'tar1090.env')) as f: + env_content = f.read() + + self.assertNotIn('RECEIVER_LAT=', env_content) + self.assertIn('ADSBLOL_ENABLED=false', env_content) + + def test_tar1090_env_returns_once_the_owner_sets_a_location(self): + """The unset case must not be sticky.""" + self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), { + 'tar1090': {'adsblol_fallback': True, 'adsblol_radius': 40}, + 'location': {'rx': {'latitude': None, 'longitude': None, 'altitude': None}}, + }) + self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) + self.write_yaml(os.path.join(self.config_dir, 'user.yml'), { + 'location': {'rx': {'latitude': 42.241528, 'longitude': -72.648361, + 'altitude': 619.2, 'name': 'ret4c844c20'}} + }) + + self.run_merge() + + with open(os.path.join(self.config_dir, 'tar1090.env')) as f: + env_content = f.read() + + self.assertIn('RECEIVER_LAT=42.241528', env_content) + self.assertIn('ADSBLOL_ENABLED=true', env_content) + + def test_tar1090_env_zero_is_a_real_location(self): + """0,0 set by an owner is a choice, not an absence.""" + self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), { + 'tar1090': {'adsblol_fallback': True, 'adsblol_radius': 40}, + 'location': {'rx': {'latitude': 0, 'longitude': 0, 'altitude': 0}}, + }) + self.write_yaml(os.path.join(self.defaults_dir, 'forced.yml'), {}) + + self.run_merge() + + with open(os.path.join(self.config_dir, 'tar1090.env')) as f: + env_content = f.read() + + self.assertIn('RECEIVER_LAT=0', env_content) + self.assertIn('ADSBLOL_ENABLED=true', env_content) + def test_tar1090_env_user_override(self): """Test that user config overrides tar1090 settings in .env""" default_config = { @@ -391,11 +463,20 @@ def test_actual_config_files(self): rx = defaults['location']['rx'] tar1090 = defaults.get('tar1090', {}) - self.assertIn(f"RECEIVER_LAT={rx['latitude']}", env_content) - self.assertIn(f"RECEIVER_LON={rx['longitude']}", env_content) - self.assertIn(f"RECEIVER_ALT={rx['altitude']}", env_content) + sited = rx.get('latitude') is not None and rx.get('longitude') is not None + + if sited: + self.assertIn(f"RECEIVER_LAT={rx['latitude']}", env_content) + self.assertIn(f"RECEIVER_LON={rx['longitude']}", env_content) + self.assertIn(f"RECEIVER_ALT={rx['altitude']}", env_content) + else: + # The shipped default carries no location on purpose, so a node + # nobody has configured must not name a position at all. + self.assertNotIn("RECEIVER_LAT=", env_content) + self.assertNotIn("RECEIVER_LON=", env_content) + self.assertIn( - f"ADSBLOL_ENABLED={'true' if tar1090.get('adsblol_fallback') else 'false'}", + f"ADSBLOL_ENABLED={'true' if tar1090.get('adsblol_fallback') and sited else 'false'}", env_content, ) self.assertIn(f"ADSBLOL_RADIUS={tar1090['adsblol_radius']}", env_content) diff --git a/config/default.yml b/config/default.yml index 86816db..50c10b9 100644 --- a/config/default.yml +++ b/config/default.yml @@ -81,17 +81,28 @@ truth: ip: 0.0.0.0 port: 30001 +# Null until an owner picks a tower. This shipped as Greenwich Observatory / +# Crystal Palace, which is indistinguishable downstream from a real choice, so +# every consumer treated an unconfigured node as a configured one and nodes +# registered with the server claiming to be in south-east London. +# +# Keys kept, with null values, rather than removed: consumers need to tell +# "unset" from "absent because this config is malformed", and retina-gui's +# config form renders from these keys. +# +# Set by retina-gui's tower step (/towers/select), which writes rx, tx and +# capture.fc together into user.yml. location: rx: - latitude: 51.4769 - longitude: -0.0005 - altitude: 48 - name: "Greenwich Observatory" + latitude: null + longitude: null + altitude: null + name: null tx: - latitude: 51.4244 - longitude: -0.0753 - altitude: 219 - name: "Crystal Palace" + latitude: null + longitude: null + altitude: null + name: null save: iq: false