Compare commits

...
Author SHA1 Message Date
Claude 32a94e9761 docs(web): name the exact plugin keys the device location seeds
Review follow-up. The General settings help text said the device location
was "the default for every plugin that asks for a city", which overstates
what the code does: only the fully-namespaced `location_city` /
`location_state` / `location_country` keys are substituted. A plugin with
a bare `city` key gets nothing — deliberately, since `ledmatrix-elections`
uses `state` for a two-letter code. The tips now name the exact keys.

Worth noting for anyone editing these: `ui.help_tip(...)` takes a
single-quoted Jinja string, so an apostrophe in the tip text has to be
escaped or written around. The wording here avoids them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLrSZ32FNKpHRaduKEJsg
2026-08-21 17:31:41 +00:00
Claude 500e7af224 fix(config): make the device location the default for plugin location fields
A user in Kansas City reported their radar centred on Dallas, TX with
nothing in config.json to explain it.

The radar is the `ledmatrix-weather` plugin's `radar` mode, and it centres
on the same coordinates as every other weather mode: `forecast_data`
lat/lon, geocoded from the plugin's own `location_city` /
`location_state` / `location_country`. Those ship with schema defaults of
Dallas / Texas / US. A user who never opened the weather plugin's config
form therefore has no `location_city` on disk, and `PluginManager` merges
the schema default in at load time — so the whole plugin (not just the
radar) silently runs on Dallas. Radar is just the only mode that draws a
recognisable map and gives the mismatch away.

Meanwhile the device-wide `location` block that General settings writes
was read by nothing at all, despite its own help text promising it was
"used for weather, sunrise/sunset, and other location-based content".

`SchemaManager.generate_default_config()` now substitutes the device
`location` into the three fully-namespaced `location_*` keys before
handing defaults back, so the promise holds:

- Only `location_city` / `location_state` / `location_country` are
  substituted. A bare `state` key is left alone — `ledmatrix-elections`
  uses it for a two-letter code, and rewriting it would break that plugin.
- A value the user saved on the plugin still wins: this replaces the
  schema default, and `merge_with_defaults` puts user config on top.
- The substitution is applied on the way out of the defaults cache rather
  than into it, so changing the device location takes effect immediately.
- No config manager, no `location` block, or an unreadable config all
  fall back to the plugin's own schema defaults.

Every caller benefits: the plugin loader, the config form (which now
pre-fills the user's real city), config save, and reset-to-defaults.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLrSZ32FNKpHRaduKEJsg
2026-08-21 17:24:00 +00:00
6 changed files with 274 additions and 10 deletions
+1 -1
View File
@@ -18,7 +18,7 @@ tooling against it.
| `web_display_autostart` | bool, `true` | Whether the web interface service starts with the system | `scripts/utils/start_web_conditionally.py` |
| `timezone` | string, `"America/New_York"` | IANA timezone for schedules and displays | `ConfigManager.get_timezone()` |
| `target_fps` | int, `100` | Frame-rate ceiling for plugin rendering | `src/plugin_system/base_plugin.py`, `src/common/sports_scroll.py` |
| `location` | object | `city` / `state` / `country`, offered to plugins that need a location (weather, etc.) | plugins via merged config |
| `location` | object | `city` / `state` / `country`. Supplies the **default** for a plugin's own `location_city` / `location_state` / `location_country` setting, so weather, radar and friends follow this device without being configured twice. A value saved on the plugin itself still overrides it. | `SchemaManager.apply_device_location()`, then plugins via merged config |
## `schedule` — display on/off hours
+2 -1
View File
@@ -71,7 +71,8 @@ class PluginManager:
self.plugin_loader = PluginLoader(logger=self.logger)
self.plugin_executor = PluginExecutor(default_timeout=30.0, logger=self.logger)
self.state_manager = PluginStateManager(logger=self.logger)
self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, logger=self.logger)
self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, logger=self.logger,
config_manager=self.config_manager)
# Lock protecting plugin_manifests and plugin_directories from
# concurrent mutation (background reconciliation) and reads (requests).
+87 -4
View File
@@ -26,7 +26,25 @@ class SchemaManager:
- Cache invalidation on plugin changes
"""
def __init__(self, plugins_dir: Optional[Path] = None, project_root: Optional[Path] = None, logger: Optional[logging.Logger] = None):
# Plugin config keys that mean "where this device is". A plugin declaring
# any of these in its schema gets the device-wide ``location`` block from
# config.json as the *default* for that field, instead of whatever city the
# plugin author happened to ship. A value the user set on the plugin itself
# always wins -- this only ever replaces the schema default, so an explicit
# per-plugin location is still honoured.
#
# Only these fully-namespaced keys are substituted. A bare ``state`` or
# ``city`` key is deliberately left alone: plugins use those for unrelated
# things (ledmatrix-elections' ``state`` is a two-letter code, not a place
# name), and silently rewriting them would break those plugins.
DEVICE_LOCATION_KEYS: Dict[str, str] = {
'location_city': 'city',
'location_state': 'state',
'location_country': 'country',
}
def __init__(self, plugins_dir: Optional[Path] = None, project_root: Optional[Path] = None,
logger: Optional[logging.Logger] = None, config_manager: Optional[Any] = None):
"""
Initialize the Schema Manager.
@@ -34,10 +52,14 @@ class SchemaManager:
plugins_dir: Base plugins directory path
project_root: Project root directory path
logger: Optional logger instance
config_manager: Optional config manager, used to resolve the
device-wide ``location`` that seeds plugin location defaults.
Omitting it simply leaves schema defaults untouched.
"""
self.logger = logger or logging.getLogger(__name__)
self.plugins_dir = plugins_dir
self.project_root = project_root or Path.cwd()
self.config_manager = config_manager
# Schema cache: plugin_id -> schema dict
self._schema_cache: Dict[str, Dict[str, Any]] = {}
@@ -212,10 +234,70 @@ class SchemaManager:
return defaults
def get_device_location(self) -> Optional[Dict[str, Any]]:
"""
Return the device-wide ``location`` block from config.json, or None.
This is the City/State/Country the user sets once under General
settings. Returns None when there is no config manager wired, the
config can't be read, or no location has been configured.
"""
if self.config_manager is None:
return None
try:
config = self.config_manager.load_config()
except Exception as e:
# A config that can't be read must never stop defaults being
# generated -- the plugin's own schema defaults still apply.
self.logger.debug(f"Could not read device location from config: {e}")
return None
if not isinstance(config, dict):
return None
location = config.get('location')
return location if isinstance(location, dict) else None
def apply_device_location(self, defaults: Dict[str, Any]) -> Dict[str, Any]:
"""
Replace location-shaped schema defaults with the device's own location.
Without this, a plugin that ships ``"location_city": "Dallas"`` as its
schema default silently reports Dallas weather (and centres its radar
there) for every user who never opened that plugin's config form --
even though they set their real city under General settings. The
substituted value is still only a *default*: ``merge_with_defaults``
lets any per-plugin value the user saved win over it.
Mutates and returns ``defaults`` for convenience.
"""
if not defaults:
return defaults
if not any(key in defaults for key in self.DEVICE_LOCATION_KEYS):
return defaults
location = self.get_device_location()
if not location:
return defaults
for key, field in self.DEVICE_LOCATION_KEYS.items():
if key not in defaults:
continue
value = location.get(field)
# Only a non-empty string is a real answer; a blank or missing
# field means "not configured", which leaves the schema default.
if isinstance(value, str) and value.strip():
defaults[key] = value.strip()
return defaults
def generate_default_config(self, plugin_id: str, use_cache: bool = True) -> Dict[str, Any]:
"""
Generate default configuration for a plugin from its schema.
Location fields (see ``DEVICE_LOCATION_KEYS``) default to the device's
configured location rather than the plugin author's. That substitution
is applied on the way out rather than being cached, so changing the
device location takes effect without invalidating the defaults cache.
Args:
plugin_id: Plugin identifier
use_cache: If True, return cached defaults if available
@@ -225,7 +307,7 @@ class SchemaManager:
"""
# Check cache first
if use_cache and plugin_id in self._defaults_cache:
return self._defaults_cache[plugin_id].copy()
return self.apply_device_location(self._defaults_cache[plugin_id].copy())
schema = self.load_schema(plugin_id, use_cache=use_cache)
if not schema:
@@ -249,10 +331,11 @@ class SchemaManager:
if 'live_priority' not in defaults:
defaults['live_priority'] = schema.get('properties', {}).get('live_priority', {}).get('default', False)
# Cache the defaults
# Cache the defaults *before* the device location is layered on, so a
# later change to the device location is picked up by the next call.
self._defaults_cache[plugin_id] = defaults.copy()
return defaults
return self.apply_device_location(defaults)
def validate_config_against_schema(self, config: Dict[str, Any], schema: Dict[str, Any],
plugin_id: Optional[str] = None) -> Tuple[bool, List[str]]:
+179
View File
@@ -0,0 +1,179 @@
"""
Tests for the device-location default: a plugin that ships a location field in
its schema must default to the device's configured City/State/Country, not to
whatever place the plugin author hard-coded.
The bug this pins: ledmatrix-weather ships ``"location_city": "Dallas"`` as a
schema default, so a user who set Kansas City under General settings but never
opened the weather plugin's own config form got Dallas weather — and a radar
centred on Dallas with nothing in config.json to explain it.
"""
import json
import pytest
from src.plugin_system.schema_manager import SchemaManager
class FakeConfigManager:
"""Minimal stand-in exposing the load_config() SchemaManager relies on."""
def __init__(self, config):
self.config = config
self.load_count = 0
def load_config(self):
self.load_count += 1
return self.config
class ExplodingConfigManager:
def load_config(self):
raise OSError("config.json is unreadable")
WEATHER_SCHEMA = {
"type": "object",
"properties": {
"location_city": {"type": "string", "default": "Dallas"},
"location_state": {"type": "string", "default": "Texas"},
"location_country": {"type": "string", "default": "US"},
"units": {"type": "string", "default": "imperial"},
},
}
def write_plugin(plugins_dir, plugin_id, schema):
plugin_dir = plugins_dir / plugin_id
plugin_dir.mkdir(parents=True, exist_ok=True)
(plugin_dir / "config_schema.json").write_text(json.dumps(schema))
return plugin_dir
@pytest.fixture
def plugins_dir(tmp_path):
d = tmp_path / "plugin-repos"
d.mkdir()
return d
def make_sm(plugins_dir, tmp_path, location):
config = {} if location is None else {"location": location}
cm = FakeConfigManager(config)
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path,
config_manager=cm)
return sm, cm
class TestDeviceLocationDefaults:
def test_device_location_replaces_plugin_default(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path,
{"city": "Kansas City", "state": "Missouri", "country": "US"})
defaults = sm.generate_default_config("ledmatrix-weather")
assert defaults["location_city"] == "Kansas City"
assert defaults["location_state"] == "Missouri"
assert defaults["location_country"] == "US"
# Non-location defaults are untouched.
assert defaults["units"] == "imperial"
def test_user_set_plugin_value_still_wins(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path,
{"city": "Kansas City", "state": "Missouri", "country": "US"})
defaults = sm.generate_default_config("ledmatrix-weather")
merged = sm.merge_with_defaults({"location_city": "Denver"}, defaults)
assert merged["location_city"] == "Denver"
# Fields the user did not override still follow the device.
assert merged["location_state"] == "Missouri"
def test_blank_and_missing_device_fields_leave_schema_default(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path, {"city": "Kansas City", "state": " "})
defaults = sm.generate_default_config("ledmatrix-weather")
assert defaults["location_city"] == "Kansas City"
assert defaults["location_state"] == "Texas" # blank -> not configured
assert defaults["location_country"] == "US" # absent -> schema default
def test_no_device_location_configured_is_a_no_op(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, _ = make_sm(plugins_dir, tmp_path, None)
defaults = sm.generate_default_config("ledmatrix-weather")
assert defaults["location_city"] == "Dallas"
def test_no_config_manager_is_a_no_op(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path)
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
def test_unreadable_config_falls_back_to_schema_defaults(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path,
config_manager=ExplodingConfigManager())
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
class TestScopedToNamespacedKeys:
def test_bare_state_key_is_not_rewritten(self, plugins_dir, tmp_path):
"""ledmatrix-elections' ``state`` is a two-letter code, not a place name."""
write_plugin(plugins_dir, "ledmatrix-elections", {
"type": "object",
"properties": {
"state": {"type": "string", "default": "CA"},
"city": {"type": "string", "default": "Springfield"},
},
})
sm, _ = make_sm(plugins_dir, tmp_path,
{"city": "Kansas City", "state": "Missouri", "country": "US"})
defaults = sm.generate_default_config("ledmatrix-elections")
assert defaults["state"] == "CA"
assert defaults["city"] == "Springfield"
def test_plugin_without_location_fields_never_reads_config(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "clock-simple", {
"type": "object",
"properties": {"format": {"type": "string", "default": "12h"}},
})
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
defaults = sm.generate_default_config("clock-simple")
assert defaults["format"] == "12h"
assert cm.load_count == 0
class TestCachingStaysFresh:
def test_location_change_is_picked_up_through_the_defaults_cache(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Kansas City"
cm.config["location"]["city"] = "Omaha"
# Second call is served from the defaults cache, but must not serve a
# stale location.
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Omaha"
def test_cached_defaults_are_not_mutated_by_the_overlay(self, plugins_dir, tmp_path):
write_plugin(plugins_dir, "ledmatrix-weather", WEATHER_SCHEMA)
sm, cm = make_sm(plugins_dir, tmp_path, {"city": "Kansas City"})
sm.generate_default_config("ledmatrix-weather")
assert sm._defaults_cache["ledmatrix-weather"]["location_city"] == "Dallas"
cm.config.pop("location")
assert sm.generate_default_config("ledmatrix-weather")["location_city"] == "Dallas"
+2 -1
View File
@@ -118,7 +118,8 @@ saved_repositories_manager = SavedRepositoriesManager()
schema_manager = SchemaManager(
plugins_dir=plugins_dir,
project_root=project_root,
logger=None
logger=None,
config_manager=config_manager
)
# Initialize operation queue for plugin operations
@@ -95,7 +95,7 @@
<!-- Location Information -->
<div class="grid grid-cols-1 md:grid-cols-3 xl:grid-cols-3 2xl:grid-cols-3 gap-4">
<div class="form-group" id="setting-general-city" data-setting-key="location.city">
<label for="city" class="block text-sm font-medium text-gray-700">City{{ ui.help_tip('City used for weather, sunrise/sunset, and other location-based content.\nExample: Dallas.', 'City') }}</label>
<label for="city" class="block text-sm font-medium text-gray-700">City{{ ui.help_tip('City used for weather, sunrise/sunset, radar, and other location-based content.\nExample: Kansas City.\nUsed as the default for the location_city setting on plugins that have one; a value saved on the plugin itself overrides it.', 'City') }}</label>
<input type="text"
id="city"
name="city"
@@ -104,7 +104,7 @@
</div>
<div class="form-group" id="setting-general-state" data-setting-key="location.state">
<label for="state" class="block text-sm font-medium text-gray-700">State{{ ui.help_tip('State or region for your location.\nExample: Texas. Improves location-lookup accuracy.', 'State') }}</label>
<label for="state" class="block text-sm font-medium text-gray-700">State{{ ui.help_tip('State or region for your location.\nExample: Missouri. Improves location-lookup accuracy.\nUsed as the default for the location_state setting on plugins that have one.', 'State') }}</label>
<input type="text"
id="state"
name="state"
@@ -113,7 +113,7 @@
</div>
<div class="form-group" id="setting-general-country" data-setting-key="location.country">
<label for="country" class="block text-sm font-medium text-gray-700">Country{{ ui.help_tip('Country code or name for your location.\nExample: US. Used with City and State for weather and geolocation.', 'Country') }}</label>
<label for="country" class="block text-sm font-medium text-gray-700">Country{{ ui.help_tip('Country code or name for your location.\nExample: US. Used with City and State for weather, radar, and geolocation.\nUsed as the default for the location_country setting on plugins that have one.', 'Country') }}</label>
<input type="text"
id="country"
name="country"