mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
fix(web): accept every panel size and row address type the rgbmatrix library does (#586)
* fix(web): accept every panel size and row address type the rgbmatrix library does The Display form capped columns at 128 and chain length at 24, and its submit handler (fixInvalidNumberInputs) rewrote anything larger to the cap, so wide panels and long chains silently saved as the wrong size. The config API checked none of the hardware numbers, so values the library rejects (odd rows, parallel 4, PWM dither bits 3) saved and the matrix then refused to start. - Form limits now match the pinned library: rows even 8-64, cols >= 16 and chain_length >= 1 with no upper bound, parallel 1-3, PWM dither bits 0-2, PWM LSB nanoseconds 50-3000. - save_main_config rejects out-of-range rows, cols, chain_length, parallel, brightness, scan_mode, pwm_bits, pwm_dither_bits, pwm_lsb_nanoseconds and gpio_slowdown with a 400. - A stored gpio_slowdown or pwm_dither_bits of 0 renders as 0 instead of the default, so saving the tab no longer overwrites it. - Row Address Type offers 5 (SM5368 / B707 row shift register). Verified on a Waveshare 96x48 V2 (24S-A1) on a Pi 4 with the Adafruit Triple LED Matrix Bonnet: rows 48, cols 96, row address type 5, BGR, GPIO slowdown 8. - Help text and docs: FM6124-family panels use Panel Type Standard; on a Pi 5 the library supports only row address types 0 and 2. No change to the rpi-rgb-led-matrix submodule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web): drop the rows cap and document every display setting accurately Rows: no upper limit in the form or the API. Still even and at least 8. The current rgbmatrix library rejects more than 64 per panel, so a larger value saves but the matrix won't start; the help tip, README, config reference and troubleshooting section all say so, and nothing here needs changing if the library lifts the limit. limit_refresh_rate_hz: the form accepts 0 (the library's "no cap"), a stored 0 no longer renders and re-saves as 120, and the API rejects negatives. pwm_dither_bits stays 0-2: the library rejects 3 and 4, so the old form's 0-4 only ever let users save a config the display couldn't start with. Docs and help tips, checked against the pinned library and its README: - panel_type and rp1_rio get README entries - show_refresh_rate prints to stdout; it never drew on the panel - dither bits raise the refresh rate; the tip said they lowered it - scan_mode is about interlacing at low refresh, not wrong colours - disable_hardware_pulsing: hardware pulsing needs OE on GPIO 18 and the onboard sound driver off; software timing makes rows flash brighter - gpio_slowdown guidance agrees between the README and the UI - all 22 multiplexing values listed; every numeric setting states its range - troubleshooting for a blank panel after a settings change, jumping rows and brightness flashes Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(web): reject true and 5.5 for row_address_type and multiplexing Both still went straight through int(), so a JSON true saved as 1 and 5.5 as 5. They now use the shared hardware range check like the other panel fields. Review feedback on #586. Also: the RP1 Backend tooltip said it is ignored on Pi 3/4 (it is ignored on every model but the Pi 5), and the README gave the dynamic-duration default cap as 90s; the code default is 180s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat: refuse matrix settings a Raspberry Pi 5 can't drive On a Pi 5 the pinned rgbmatrix library drives the panel through the RP1 chip, and that path supports only row address types 0 and 2, parallel 1-3 and the regular / regular-pi1 / classic / adafruit-hat(-pwm) mappings (Rp1PioConfigSupported in lib/rp1/rp1_pio_backend.cc). For anything else CreateFromOptions returns NULL; the Python binding doesn't check, so the display process crashed on its first call into the matrix and systemd restarted it into the same crash every 10 seconds. - src/pi5_matrix_support.py: the rule and Pi 5 detection, matching the library's /proc/device-tree/model check - DisplayManager raises before creating the matrix, so it is a logged init failure (reported by /api/v3/hardware/status) and fallback mode - the config API rejects those settings on a Pi 5 when a request sets row_address_type, parallel or hardware_mapping - the Display form offers only row address types 0 and 2 on a Pi 5, and warns when a stored value can't be used - CLAUDE.md: re-check the rule whenever the submodule is bumped Review feedback on #586. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Chuck
@@ -0,0 +1,308 @@
|
||||
"""Display hardware settings accept what the rgbmatrix library accepts.
|
||||
|
||||
Held to the ranges in the pinned library (RGBMatrix::Options::Validate in
|
||||
lib/options-initialize.cc, the gpio_slowdown check in lib/led-matrix.cc), with
|
||||
one deliberate exception: rows has no upper bound here, although the library
|
||||
currently rejects more than 64 per panel. Two ways this used to go wrong:
|
||||
|
||||
- The Display form capped cols at 128, chain_length at 24 and
|
||||
pwm_lsb_nanoseconds at 500, and its submit handler (fixInvalidNumberInputs)
|
||||
rewrites anything past an input's min/max to that bound -- so a wide panel or
|
||||
a long chain silently saved as the wrong size.
|
||||
- The API checked none of these, so a value the library rejects (odd rows,
|
||||
parallel 4, pwm_dither_bits 3) saved, and the matrix then refused to start.
|
||||
|
||||
Row address type 5 is the SM5368 / B707 row shift register the Waveshare 96x48
|
||||
V2 needs (Waveshare's own "96X48_1_24_SM5368" panel type in their library fork
|
||||
just sets rows/cols, row_address_type=5 and BGR); the API used to stop at 4.
|
||||
"""
|
||||
import copy
|
||||
import json
|
||||
import re
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
PROJECT_ROOT = Path(__file__).parent.parent
|
||||
sys.path.insert(0, str(PROJECT_ROOT))
|
||||
|
||||
from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402
|
||||
from test.test_web_settings_ui import REALISTIC_CONFIG # noqa: E402
|
||||
from src import pi5_matrix_support # noqa: E402
|
||||
|
||||
#: What a Waveshare RGB-Matrix-P2.5-96x48 V2 (back silkscreen 24S-A1) needed on
|
||||
#: a Pi 4 with an Adafruit Triple LED Matrix Bonnet, checked on the panel.
|
||||
WAVESHARE_96X48_V2 = {
|
||||
'rows': 48, 'cols': 96, 'chain_length': 1, 'parallel': 1,
|
||||
'hardware_mapping': 'regular', 'panel_type': '', 'row_address_type': 5,
|
||||
'led_rgb_sequence': 'BGR', 'gpio_slowdown': 8,
|
||||
}
|
||||
|
||||
#: Fields stored under display.runtime; the rest go under display.hardware.
|
||||
RUNTIME_FIELDS = {'gpio_slowdown'}
|
||||
|
||||
PI5_MODEL = 'Raspberry Pi 5 Model B Rev 1.0'
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def board(tmp_path, monkeypatch):
|
||||
"""Not a Pi 5 unless a test says so, whatever machine runs the suite.
|
||||
|
||||
Returns a setter: board(PI5_MODEL) makes the API and the form see a Pi 5.
|
||||
"""
|
||||
path = tmp_path / 'device-tree-model'
|
||||
|
||||
def set_model(model):
|
||||
path.write_bytes(model.encode() + b'\x00')
|
||||
|
||||
set_model('Raspberry Pi 4 Model B Rev 1.5')
|
||||
monkeypatch.setattr(pi5_matrix_support, 'MODEL_PATH', str(path))
|
||||
return set_model
|
||||
|
||||
|
||||
def _stored(config, field):
|
||||
section = 'runtime' if field in RUNTIME_FIELDS else 'hardware'
|
||||
return config['display'][section][field]
|
||||
|
||||
|
||||
def _post(client, body):
|
||||
return client.post('/api/v3/config/main', data=json.dumps(body),
|
||||
content_type='application/json')
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def saved(api_v3_module, monkeypatch):
|
||||
"""Capture what save_main_config would write.
|
||||
|
||||
Asserting on the stored value, not just the status code, is what shows the
|
||||
value passed validation and landed where DisplayManager reads it.
|
||||
"""
|
||||
captured = {}
|
||||
api_v3_module.api_v3.config_manager.load_config.return_value = {}
|
||||
|
||||
def fake_save(_manager, config, **_kwargs):
|
||||
captured['config'] = config
|
||||
return True, ''
|
||||
|
||||
monkeypatch.setattr(api_v3_module, '_save_config_atomic', fake_save)
|
||||
return captured
|
||||
|
||||
|
||||
@pytest.mark.parametrize('as_strings', [False, True], ids=['json-numbers', 'form-strings'])
|
||||
def test_waveshare_96x48_v2_settings_all_save(api_v3_client, saved, as_strings):
|
||||
"""The Display form posts every value as a string (json-enc); API clients send numbers."""
|
||||
body = {k: str(v) if as_strings else v for k, v in WAVESHARE_96X48_V2.items()}
|
||||
response = _post(api_v3_client, body)
|
||||
assert response.status_code == 200, response.get_data(as_text=True)[:200]
|
||||
for field, value in WAVESHARE_96X48_V2.items():
|
||||
assert _stored(saved['config'], field) == value, field
|
||||
|
||||
|
||||
@pytest.mark.parametrize('field,value', [
|
||||
('rows', 8), ('rows', 64), ('rows', 96), ('rows', 128),
|
||||
('cols', 16), ('cols', 192), ('cols', 512),
|
||||
('chain_length', 1), ('chain_length', 32),
|
||||
('parallel', 3),
|
||||
('row_address_type', 0), ('row_address_type', 5), ('row_address_type', 5.0),
|
||||
('multiplexing', 0), ('multiplexing', 22),
|
||||
('gpio_slowdown', 0), ('gpio_slowdown', 10),
|
||||
('pwm_bits', 1), ('pwm_bits', 11),
|
||||
('pwm_dither_bits', 0), ('pwm_dither_bits', 2),
|
||||
('pwm_lsb_nanoseconds', 50), ('pwm_lsb_nanoseconds', 3000),
|
||||
('scan_mode', 1),
|
||||
('brightness', 1), ('brightness', 100),
|
||||
('limit_refresh_rate_hz', 0), ('limit_refresh_rate_hz', 1000),
|
||||
])
|
||||
def test_values_in_range_are_saved(api_v3_client, saved, field, value):
|
||||
response = _post(api_v3_client, {field: value})
|
||||
assert response.status_code == 200, response.get_data(as_text=True)[:200]
|
||||
assert _stored(saved['config'], field) == value
|
||||
|
||||
|
||||
@pytest.mark.parametrize('field,value', [
|
||||
('rows', 6), ('rows', 47), ('rows', 97), ('rows', '48.5'),
|
||||
('cols', 15), ('cols', 96.5), ('cols', True), ('cols', 'wide'),
|
||||
('chain_length', 0),
|
||||
('parallel', 0), ('parallel', 4),
|
||||
('row_address_type', -1), ('row_address_type', 6),
|
||||
('row_address_type', True), ('row_address_type', 5.5),
|
||||
('multiplexing', -1), ('multiplexing', 23), ('multiplexing', True),
|
||||
('gpio_slowdown', -1), ('gpio_slowdown', 11),
|
||||
('pwm_bits', 0), ('pwm_bits', 12),
|
||||
('pwm_dither_bits', 3),
|
||||
('pwm_lsb_nanoseconds', 49), ('pwm_lsb_nanoseconds', 3001),
|
||||
('scan_mode', 2),
|
||||
('brightness', 0), ('brightness', 101),
|
||||
('limit_refresh_rate_hz', -1),
|
||||
])
|
||||
def test_values_out_of_range_are_refused(api_v3_client, saved, field, value):
|
||||
"""Refused with a message naming the field, and nothing written."""
|
||||
response = _post(api_v3_client, {field: value})
|
||||
assert response.status_code == 400
|
||||
assert field in response.get_json()['message']
|
||||
assert 'config' not in saved
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def display_page(monkeypatch):
|
||||
"""Render the Display settings partial for a given config."""
|
||||
from web_interface.blueprints import pages_v3 as pv
|
||||
|
||||
def render(config):
|
||||
base = PROJECT_ROOT / 'web_interface'
|
||||
app = Flask(__name__, template_folder=str(base / 'templates'),
|
||||
static_folder=str(base / 'static'))
|
||||
app.config['TESTING'] = True
|
||||
config_manager = MagicMock()
|
||||
config_manager.load_config.return_value = config
|
||||
config_manager.get_raw_file_content.return_value = config
|
||||
config_manager.get_config_path.return_value = 'config/config.json'
|
||||
config_manager.get_secrets_path.return_value = 'config/config_secrets.json'
|
||||
monkeypatch.setattr(pv.pages_v3, 'config_manager', config_manager, raising=False)
|
||||
monkeypatch.setattr(pv.pages_v3, 'plugin_manager', MagicMock(plugins={}), raising=False)
|
||||
app.register_blueprint(pv.pages_v3, url_prefix='/v3')
|
||||
response = app.test_client().get('/v3/partials/display')
|
||||
assert response.status_code == 200
|
||||
return response.get_data(as_text=True)
|
||||
|
||||
return render
|
||||
|
||||
|
||||
def _config_with(hardware=None, runtime=None):
|
||||
config = copy.deepcopy(REALISTIC_CONFIG)
|
||||
config['display']['hardware'].update(hardware or {})
|
||||
config['display']['runtime'].update(runtime or {})
|
||||
return config
|
||||
|
||||
|
||||
def _input_tag(body, input_id):
|
||||
match = re.search(r'<input[^>]*\bid="%s"[^>]*>' % re.escape(input_id), body)
|
||||
assert match, f'no <input id="{input_id}">'
|
||||
return match.group(0)
|
||||
|
||||
|
||||
def _attr(tag, name):
|
||||
match = re.search(r'\s%s="([^"]*)"' % name, tag)
|
||||
return match.group(1) if match else None
|
||||
|
||||
|
||||
def _selected_option(body, select_id):
|
||||
select = re.search(r'<select id="%s".*?</select>' % select_id, body, re.S)
|
||||
assert select, f'no <select id="{select_id}">'
|
||||
return re.findall(r'<option value="([^"]*)"\s+selected\s*>', select.group(0))
|
||||
|
||||
|
||||
@pytest.mark.parametrize('input_id,expected', [
|
||||
('rows', {'min': '8', 'max': None, 'step': '2'}),
|
||||
('cols', {'min': '16', 'max': None}),
|
||||
('chain_length', {'min': '1', 'max': None}),
|
||||
('parallel', {'min': '1', 'max': '3'}),
|
||||
('gpio_slowdown', {'min': '0', 'max': '10'}),
|
||||
('pwm_bits', {'min': '1', 'max': '11'}),
|
||||
('pwm_dither_bits', {'min': '0', 'max': '2'}),
|
||||
('pwm_lsb_nanoseconds', {'min': '50', 'max': '3000'}),
|
||||
('limit_refresh_rate_hz', {'min': '0', 'max': '1000'}),
|
||||
])
|
||||
def test_form_limits_match_the_library(display_page, input_id, expected):
|
||||
"""fixInvalidNumberInputs rewrites a value past min/max on submit, so these
|
||||
attributes are the real limits: a max below the library's clamps panels
|
||||
that would work, and one above it saves a value the matrix rejects."""
|
||||
tag = _input_tag(display_page(_config_with()), input_id)
|
||||
for name, value in expected.items():
|
||||
assert _attr(tag, name) == value, f'{input_id} {name}'
|
||||
|
||||
|
||||
def test_waveshare_96x48_v2_config_renders_back_unchanged(display_page):
|
||||
"""Saving the Display tab posts what it rendered, so each value must render as stored.
|
||||
|
||||
Before row address type 5 was in the dropdown no option was selected, the
|
||||
browser posted the first one (0), and one save scrambled the panel again.
|
||||
"""
|
||||
hardware = {k: v for k, v in WAVESHARE_96X48_V2.items() if k not in RUNTIME_FIELDS}
|
||||
body = display_page(_config_with(hardware=hardware, runtime={'gpio_slowdown': 8}))
|
||||
|
||||
assert _selected_option(body, 'row_address_type') == ['5']
|
||||
assert _selected_option(body, 'led_rgb_sequence') == ['BGR']
|
||||
assert _selected_option(body, 'hardware_mapping') == ['regular']
|
||||
for input_id in ('rows', 'cols', 'chain_length', 'parallel', 'gpio_slowdown'):
|
||||
assert _attr(_input_tag(body, input_id), 'value') == str(WAVESHARE_96X48_V2[input_id]), input_id
|
||||
|
||||
|
||||
@pytest.mark.parametrize('field,section', [
|
||||
('gpio_slowdown', 'runtime'), ('pwm_dither_bits', 'hardware'),
|
||||
('limit_refresh_rate_hz', 'hardware'),
|
||||
])
|
||||
def test_a_stored_zero_renders_as_zero(display_page, field, section):
|
||||
"""`value or default` showed a stored 0 as the default, and the next save wrote it back."""
|
||||
body = display_page(_config_with(**{section: {field: 0}}))
|
||||
assert _attr(_input_tag(body, field), 'value') == '0'
|
||||
|
||||
|
||||
# --- Raspberry Pi 5 -------------------------------------------------------
|
||||
# The pinned library's Pi 5 path drives only row address types 0 and 2,
|
||||
# parallel 1-3 and the standard mappings; anything else crashes the display
|
||||
# service (src/pi5_matrix_support.py), so the API and the form refuse it.
|
||||
|
||||
@pytest.mark.parametrize('body', [
|
||||
{'row_address_type': 5}, {'row_address_type': '1'},
|
||||
{'hardware_mapping': 'compute-module'},
|
||||
])
|
||||
def test_pi5_refuses_what_its_library_cannot_drive(api_v3_client, saved, board, body):
|
||||
board(PI5_MODEL)
|
||||
response = _post(api_v3_client, body)
|
||||
assert response.status_code == 400
|
||||
assert 'Raspberry Pi 5' in response.get_json()['message']
|
||||
assert 'config' not in saved
|
||||
|
||||
|
||||
def test_pi5_saves_what_it_can_drive(api_v3_client, saved, board):
|
||||
board(PI5_MODEL)
|
||||
response = _post(api_v3_client, dict(WAVESHARE_96X48_V2, row_address_type=2))
|
||||
assert response.status_code == 200, response.get_data(as_text=True)[:200]
|
||||
assert saved['config']['display']['hardware']['row_address_type'] == 2
|
||||
|
||||
|
||||
def test_pi5_check_uses_the_stored_value_for_fields_not_sent(api_v3_client, api_v3_module, saved, board):
|
||||
"""Changing only the mapping is still checked against the stored row address type."""
|
||||
board(PI5_MODEL)
|
||||
api_v3_module.api_v3.config_manager.load_config.return_value = {
|
||||
'display': {'hardware': {'row_address_type': 5}}}
|
||||
response = _post(api_v3_client, {'hardware_mapping': 'regular'})
|
||||
assert response.status_code == 400
|
||||
assert 'config' not in saved
|
||||
|
||||
|
||||
def test_pi5_stored_combination_does_not_block_unrelated_saves(api_v3_client, api_v3_module, saved, board):
|
||||
board(PI5_MODEL)
|
||||
api_v3_module.api_v3.config_manager.load_config.return_value = {
|
||||
'display': {'hardware': {'row_address_type': 5}}}
|
||||
response = _post(api_v3_client, {'brightness': 70})
|
||||
assert response.status_code == 200, response.get_data(as_text=True)[:200]
|
||||
|
||||
|
||||
def _option_values(body, select_id):
|
||||
select = re.search(r'<select id="%s".*?</select>' % select_id, body, re.S)
|
||||
assert select, f'no <select id="{select_id}">'
|
||||
return re.findall(r'<option value="([^"]*)"', select.group(0))
|
||||
|
||||
|
||||
def test_pi5_form_offers_only_supported_row_address_types(display_page, board):
|
||||
board(PI5_MODEL)
|
||||
body = display_page(_config_with())
|
||||
assert _option_values(body, 'row_address_type') == ['0', '2']
|
||||
assert "can't be used on this Raspberry Pi 5" not in body
|
||||
|
||||
|
||||
def test_other_boards_offer_every_row_address_type(display_page):
|
||||
body = display_page(_config_with())
|
||||
assert _option_values(body, 'row_address_type') == ['0', '1', '2', '3', '4', '5']
|
||||
|
||||
|
||||
def test_pi5_form_warns_about_a_stored_unsupported_row_address_type(display_page, board):
|
||||
"""The unsupported option isn't offered, so the browser posts 0 -- say so."""
|
||||
board(PI5_MODEL)
|
||||
body = display_page(_config_with(hardware={'row_address_type': 5}))
|
||||
assert "Your saved row address type (5) can't be used on this Raspberry Pi 5" in body
|
||||
Reference in New Issue
Block a user