mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
fix: address CodeRabbit review on #580
- Preview fallbacks (SSE stream and /display/current) use logical_size({})
(128x32, the shared default) instead of a hard-coded 128x64.
- display_geometry treats a non-mapping display/hardware block as missing,
so a malformed config.json falls back to defaults instead of raising
AttributeError (which turned the Starlark render into an HTTP 500).
- Docs: the static update interval falls back manifest -> plugin config
-> 60s, in both the API reference and the architecture spec.
Not taken: validating double_sided copies against chain_length/parallel.
An orientation Rotate: or U-mapper pixel mapper decides which axis panels
lie on, so the counts would reject working setups (the existing
vertical-split test is one).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -38,8 +38,9 @@ self.enabled # Boolean enabled status
|
|||||||
|
|
||||||
Fetch/update data for this plugin. Called on the plugin's update interval:
|
Fetch/update data for this plugin. Called on the plugin's update interval:
|
||||||
the value `get_update_interval()` returns when it returns a number, otherwise
|
the value `get_update_interval()` returns when it returns a number, otherwise
|
||||||
the `update_interval` in the plugin's manifest (see
|
the static interval: the `update_interval` in the plugin's manifest, else
|
||||||
[`get_update_interval()`](#get_update_interval---optionalfloat) below).
|
`update_interval` in the plugin's section of `config.json`, else 60 seconds
|
||||||
|
(see [`get_update_interval()`](#get_update_interval---optionalfloat) below).
|
||||||
|
|
||||||
**Example**:
|
**Example**:
|
||||||
```python
|
```python
|
||||||
|
|||||||
@@ -190,7 +190,8 @@ class BasePlugin(ABC):
|
|||||||
"""
|
"""
|
||||||
Fetch/update data for this plugin.
|
Fetch/update data for this plugin.
|
||||||
Called every get_update_interval() seconds when that returns a
|
Called every get_update_interval() seconds when that returns a
|
||||||
number, otherwise every manifest update_interval seconds.
|
number, otherwise at the static interval: the manifest's
|
||||||
|
update_interval, else the plugin config's update_interval, else 60s.
|
||||||
"""
|
"""
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
|||||||
+15
-4
@@ -23,9 +23,16 @@ DEFAULT_CHAIN_LENGTH = 2
|
|||||||
DEFAULT_PARALLEL = 1
|
DEFAULT_PARALLEL = 1
|
||||||
|
|
||||||
|
|
||||||
|
def _display(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]:
|
||||||
|
# A hand-edited config.json can hold anything here; treat a non-mapping
|
||||||
|
# like a missing block so callers get the defaults, not AttributeError.
|
||||||
|
display = (config or {}).get('display')
|
||||||
|
return display if isinstance(display, Mapping) else {}
|
||||||
|
|
||||||
|
|
||||||
def _hardware(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]:
|
def _hardware(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]:
|
||||||
display = (config or {}).get('display') or {}
|
hw = _display(config).get('hardware')
|
||||||
return display.get('hardware') or {}
|
return hw if isinstance(hw, Mapping) else {}
|
||||||
|
|
||||||
|
|
||||||
def physical_size(config: Optional[Mapping[str, Any]]) -> Tuple[int, int]:
|
def physical_size(config: Optional[Mapping[str, Any]]) -> Tuple[int, int]:
|
||||||
@@ -54,6 +61,10 @@ def resolve_double_sided(physical_width: int, physical_height: int,
|
|||||||
config is logged and disabled rather than raised — a misconfigured panel
|
config is logged and disabled rather than raised — a misconfigured panel
|
||||||
should still light up.
|
should still light up.
|
||||||
|
|
||||||
|
Only pixels are checked, not whole panels: ``chain_length`` and
|
||||||
|
``parallel`` don't say which axis a panel lies on once an orientation
|
||||||
|
``Rotate:`` or U-mapper ``pixel_mapper_config`` rearranges the chain.
|
||||||
|
|
||||||
``quiet`` suppresses the log lines, for callers that run on every web
|
``quiet`` suppresses the log lines, for callers that run on every web
|
||||||
request and would otherwise repeat them on each poll.
|
request and would otherwise repeat them on each poll.
|
||||||
"""
|
"""
|
||||||
@@ -118,8 +129,8 @@ def logical_size(config: Optional[Mapping[str, Any]],
|
|||||||
``DisplayManager.width``/``height`` give.
|
``DisplayManager.width``/``height`` give.
|
||||||
"""
|
"""
|
||||||
width, height = physical_size(config)
|
width, height = physical_size(config)
|
||||||
display = (config or {}).get('display') or {}
|
ds = resolve_double_sided(width, height,
|
||||||
ds = resolve_double_sided(width, height, display.get('double_sided') or {},
|
_display(config).get('double_sided') or {},
|
||||||
quiet=quiet)
|
quiet=quiet)
|
||||||
if ds is not None:
|
if ds is not None:
|
||||||
return ds['logical_width'], ds['logical_height']
|
return ds['logical_width'], ds['logical_height']
|
||||||
|
|||||||
@@ -71,6 +71,13 @@ def test_disabled_double_sided_is_ignored():
|
|||||||
assert logical_size(cfg) == (256, 32)
|
assert logical_size(cfg) == (256, 32)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize('display', ['oops', ['a'], 1, {'hardware': 'oops'},
|
||||||
|
{'hardware': ['a']}])
|
||||||
|
def test_non_mapping_display_config_uses_the_defaults(display):
|
||||||
|
assert physical_size({'display': display}) == (128, 32)
|
||||||
|
assert logical_size({'display': display}) == (128, 32)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def display_client(monkeypatch):
|
def display_client(monkeypatch):
|
||||||
from web_interface.blueprints.api_v3 import api_v3
|
from web_interface.blueprints.api_v3 import api_v3
|
||||||
@@ -103,6 +110,15 @@ def test_display_current_defaults_chain_length_like_display_manager(display_clie
|
|||||||
assert (data['width'], data['height']) == (64 * DEFAULT_CHAIN_LENGTH, 32)
|
assert (data['width'], data['height']) == (64 * DEFAULT_CHAIN_LENGTH, 32)
|
||||||
|
|
||||||
|
|
||||||
|
def test_display_current_falls_back_to_the_shared_default(display_client):
|
||||||
|
client, config_manager = display_client
|
||||||
|
config_manager.load_config.side_effect = ValueError('unreadable')
|
||||||
|
|
||||||
|
data = client.get('/api/v3/display/current').get_json()['data']
|
||||||
|
|
||||||
|
assert (data['width'], data['height']) == logical_size({}) == (128, 32)
|
||||||
|
|
||||||
|
|
||||||
def test_preview_callers_do_not_rederive_the_size():
|
def test_preview_callers_do_not_rederive_the_size():
|
||||||
"""Every preview/size caller goes through display_geometry, so none of
|
"""Every preview/size caller goes through display_geometry, so none of
|
||||||
them can drift back to a private chain_length default."""
|
them can drift back to a private chain_length default."""
|
||||||
|
|||||||
@@ -750,12 +750,11 @@ def display_preview_generator():
|
|||||||
|
|
||||||
# Get display dimensions from config: the logical size DisplayManager
|
# Get display dimensions from config: the logical size DisplayManager
|
||||||
# renders at, so double-sided setups preview one screen
|
# renders at, so double-sided setups preview one screen
|
||||||
|
from src.display_geometry import logical_size
|
||||||
try:
|
try:
|
||||||
from src.display_geometry import logical_size
|
|
||||||
width, height = logical_size(config_manager.load_config())
|
width, height = logical_size(config_manager.load_config())
|
||||||
except (KeyError, TypeError, ValueError, AttributeError, ConfigError):
|
except (KeyError, TypeError, ValueError, AttributeError, ConfigError):
|
||||||
width = 128
|
width, height = logical_size({})
|
||||||
height = 64
|
|
||||||
|
|
||||||
while True:
|
while True:
|
||||||
try:
|
try:
|
||||||
|
|||||||
@@ -27,16 +27,12 @@ def get_display_current():
|
|||||||
|
|
||||||
# Get display dimensions from config: the logical size DisplayManager
|
# Get display dimensions from config: the logical size DisplayManager
|
||||||
# renders at, so double-sided setups preview one screen
|
# renders at, so double-sided setups preview one screen
|
||||||
|
from src.display_geometry import logical_size
|
||||||
try:
|
try:
|
||||||
if api_v3.config_manager:
|
config = api_v3.config_manager.load_config() if api_v3.config_manager else {}
|
||||||
from src.display_geometry import logical_size
|
width, height = logical_size(config)
|
||||||
width, height = logical_size(api_v3.config_manager.load_config())
|
|
||||||
else:
|
|
||||||
width = 128
|
|
||||||
height = 64
|
|
||||||
except Exception:
|
except Exception:
|
||||||
width = 128
|
width, height = logical_size({})
|
||||||
height = 64
|
|
||||||
|
|
||||||
# Try to read snapshot file
|
# Try to read snapshot file
|
||||||
image_data = None
|
image_data = None
|
||||||
|
|||||||
Reference in New Issue
Block a user