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