mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-09 16:46:37 +00:00
feat(scroll): report a panel that cannot reach its refresh cap, and suggest one it can hold (#759)
* feat(scroll): report a panel that cannot reach its refresh cap, and suggest one it can hold
Scroll speeds are solved against display.hardware.limit_refresh_rate_hz,
which is only a ceiling. A panel that cannot reach it still moves whole
pixels per frame, but every scroll runs slow by the shortfall and the
"smooth" ladder is the cap's, not the panel's. A user rig (Pi 4, 2x128x64,
adafruit-hat-pwm, pwm_bits 9, gpio_slowdown 5) measured 107.6-113.1 Hz under
a 120 Hz cap: 60 px/s ran at 55, and nothing said why.
- scroll_config: refresh_shortfall() (more than 3% under the planned rate),
holdable_cap() (a multiple of 10, 5% under the measurement, since the
measurement is the fast end of an uncapped panel's drift), and
describe_refresh_shortfall().
- FrameTimingRecorder.plan_refresh(): once the measured period has held for
three trusted windows, a shortfall is logged once as a warning naming the
cap to use. DisplayManager calls it only for a real panel, not the
emulator or the fallback canvas. The stats file records
planned_refresh_hz (additive).
- GET /api/v3/config/refresh-rate, plus a hint under the Display tab's
Limit Refresh Rate field (js/pages/display.js) with a button that fills in
the suggested cap.
- _panel_refresh_hz (behind the Vegas slider's advice) ignores a measurement
written under a different cap, so a changed cap stops being advised from
the old rate before the display restarts.
Verified on ledpi with a temporary 200 Hz cap: the warning logged about a
minute after the restart ("about 132 Hz ... Set Limit Refresh Rate to
120 Hz"), the endpoint returned the same shortfall, and the Display tab
showed the hint; its button filled in 120. ledpi was restored afterwards.
Rebased onto main after the Display tab became an ES-module page (#771); the
hint moved from inline script into display.js, with a jsdom test.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
* fix(scroll): count only agreeing windows toward the refresh shortfall; ignore a null planned rate
Review fixes on #759.
- A window the period estimate rejects (more than MAX_REFRESH_DROP faster
than the adopted period) no longer counts toward the shortfall check, and
it restarts the run. After a loaded start fixed a slow period, later
windows at the real, faster rate were rejected yet still counted, so the
warning could name the slow rate against a cap the panel was meeting. It
now needs REFRESH_CHECK_WINDOWS consecutive windows that agree with the
period.
- _panel_refresh_hz treats a stats file whose planned_refresh_hz key is
present but null as no measurement. The emulator and the fallback canvas
write it that way (DisplayManager never plans a refresh for them), and
their frame rate says nothing about the cap. A file with no such key (an
older display) keeps the old behaviour.
Three new tests fail on the previous code and pass now.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
370c8fe273
commit
1bcb524fc3
Vendored
+9
@@ -175,6 +175,15 @@
|
||||
"POST"
|
||||
]
|
||||
],
|
||||
[
|
||||
"/api/v3/config/refresh-rate",
|
||||
"api_v3.get_refresh_rate",
|
||||
[
|
||||
"GET",
|
||||
"HEAD",
|
||||
"OPTIONS"
|
||||
]
|
||||
],
|
||||
[
|
||||
"/api/v3/config/schedule",
|
||||
"api_v3.get_schedule_config",
|
||||
|
||||
+1
-1
@@ -71,7 +71,7 @@ server has none.
|
||||
| `dom/test_raw_json_page.js` | yes | The Config Editor tab (`js/pages/raw-json.js`): one POST per Save after repeated swaps, Format/Validate, invalid JSON never sent, a save survives a swap, the old global entry points |
|
||||
| `dom/test_schedule_page.js` | yes | The Schedule tab (`js/pages/schedule.js`) with the real `schedule-picker` widget: both pickers drawn once per swap from the saved config, one notification per save answer after repeated swaps, the brightness label, a late widget waited for, the old global entry points |
|
||||
| `dom/test_visibility_service.js` | yes (no server) | `js/core/visibility.js` with the real `LEDVisibility` from `app-shell.js` and the real registry: start/stop with the active tab and the browser tab's visibility, no interval while hidden or after a swap-out, registrations independent, the no-`LEDVisibility` fallback |
|
||||
| `dom/test_display_page.js` | yes | The Display tab (`js/pages/display.js`) with the real `plugin-order-list` widget and `LEDVisibility`: one page, one sync interval and one action per control after repeated swaps, the sync poll only while on screen and never after a swap-out, sync states as text, the debounced scroll-speed hint, `updateSyncUI`'s entry point |
|
||||
| `dom/test_display_page.js` | yes | The Display tab (`js/pages/display.js`) with the real `plugin-order-list` widget and `LEDVisibility`: one page, one sync interval and one action per control after repeated swaps, the sync poll only while on screen and never after a swap-out, sync states as text, the debounced scroll-speed hint, the refresh-cap hint and its "Use N Hz" button, `updateSyncUI`'s entry point |
|
||||
| `dom/test_general_page.js` | yes | The General tab (`js/pages/general.js`) with the real `timezone-selector` widget: the picker drawn once per swap, one request per Security action after repeated swaps, hostile token names stay text, refused/network/login answers, a write survives a swap, `webLogin`'s entry points |
|
||||
| `dom/test_backup_restore_page.js` | yes | The Backup & Restore tab (`js/pages/backup-restore.js`): one request per action after repeated swaps, the upload and restore options, reads cancelled and writes not on a swap, hostile names stay text, the old global entry points |
|
||||
| `dom/test_tools_sections.js` | yes | The Tools tab's MQTT bridge and Pixlet editor sections: form prefill, the write-only password (blank means unchanged), the running-session banner and countdown, and that the editor link points at the host you loaded the page from |
|
||||
|
||||
@@ -88,6 +88,8 @@ const ok = (l, c, x) => c ? (pass++, console.log(' ok ' + l))
|
||||
let syncAnswer = { status: 'success', data: { role: 'leader', state: 'no_peer' } };
|
||||
let syncMode = 'ok';
|
||||
let advice = smooth;
|
||||
const shortfall = { measured_hz: 110.4, planned_hz: 120, suggested_cap_hz: 100, slow_percent: 8 };
|
||||
let refreshAnswer = { status: 'success', data: { planned_hz: 120, measured_hz: 110.4, shortfall } };
|
||||
const requests = [];
|
||||
function fakeFetch(url, init = {}) {
|
||||
requests.push(url);
|
||||
@@ -99,6 +101,7 @@ const ok = (l, c, x) => c ? (pass++, console.log(' ok ' + l))
|
||||
});
|
||||
if (url === '/api/v3/plugins/installed') return respond(200, { status: 'success', data: { plugins } });
|
||||
if (url.startsWith('/api/v3/config/scroll-speed-advice?')) return respond(200, advice);
|
||||
if (url === '/api/v3/config/refresh-rate') return respond(200, refreshAnswer);
|
||||
if (url === '/api/v3/sync/status') {
|
||||
if (syncMode === 'network') return Promise.reject(new TypeError('Failed to fetch'));
|
||||
if (syncMode === 'login') return respond(401, { status: 'error' }, { 'X-LEDMatrix-Login': '/login' });
|
||||
@@ -148,6 +151,14 @@ const ok = (l, c, x) => c ? (pass++, console.log(' ok ' + l))
|
||||
// ── first load ──────────────────────────────────────────────────────────
|
||||
ok('one plugin-list request on start', count('/api/v3/plugins/installed') === 1, requests);
|
||||
ok('one scroll-speed hint request on start (after the debounce)', count('/api/v3/config/scroll-speed-advice') === 1, requests);
|
||||
const refreshHint = $('limit_refresh_rate_hz_hint');
|
||||
ok('a panel short of its cap says so, as text, with a button for a cap it can hold',
|
||||
/about 110 Hz, below this 120 Hz cap.*8% slower/.test(refreshHint.textContent)
|
||||
&& refreshHint.querySelector('button').textContent === 'Use 100 Hz', refreshHint.textContent);
|
||||
refreshHint.querySelector('button').click();
|
||||
ok('the button fills the field and says to save and restart',
|
||||
$('limit_refresh_rate_hz').value === '100' && /Save, then restart/.test(refreshHint.textContent),
|
||||
[$('limit_refresh_rate_hz').value, refreshHint.textContent]);
|
||||
ok('the saved role is standalone: no sync request, no interval work',
|
||||
$('sync_role').value === 'standalone' && syncPolls() === 0, [$('sync_role').value, syncPolls()]);
|
||||
ok('the sync poll interval runs while the tab is on screen', intervals.size === 1
|
||||
|
||||
@@ -807,3 +807,80 @@ def test_a_process_with_the_gc_monitor_exits_cleanly():
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
assert "Exception ignored" not in proc.stderr
|
||||
assert "installed at exit: False" in proc.stdout
|
||||
|
||||
|
||||
SLOW = 1 / 110.0 # a panel that cannot reach a 120 Hz cap
|
||||
|
||||
|
||||
def _windows(recorder, n, interval, start=0.0):
|
||||
for i in range(n):
|
||||
_feed(recorder, [interval] * 200, start=start + 50.0 * i)
|
||||
_aggregate(recorder)
|
||||
|
||||
|
||||
def _shortfall_warnings(caplog):
|
||||
return [r for r in caplog.records
|
||||
if r.name == "src.common.frame_timing" and "Limit Refresh Rate" in r.getMessage()]
|
||||
|
||||
|
||||
def test_a_panel_slower_than_its_cap_is_reported_once(tmp_path, caplog):
|
||||
r = _recorder(tmp_path)
|
||||
r.plan_refresh(120.0)
|
||||
caplog.set_level("WARNING")
|
||||
_windows(r, 3, SLOW) # adopted on the 2nd window, checked on the 4th
|
||||
assert _shortfall_warnings(caplog) == []
|
||||
_windows(r, 3, SLOW, start=1000.0)
|
||||
warnings = _shortfall_warnings(caplog)
|
||||
assert len(warnings) == 1
|
||||
assert "about 110 Hz" in warnings[0].getMessage()
|
||||
assert "to 100 Hz" in warnings[0].getMessage()
|
||||
|
||||
|
||||
def test_a_panel_that_reaches_its_cap_is_not_reported(tmp_path, caplog):
|
||||
r = _recorder(tmp_path)
|
||||
r.plan_refresh(100.0)
|
||||
caplog.set_level("WARNING")
|
||||
_windows(r, 6, PERIOD)
|
||||
assert _shortfall_warnings(caplog) == []
|
||||
|
||||
|
||||
def test_without_a_planned_rate_nothing_is_checked(tmp_path, caplog):
|
||||
# The emulator and the fallback canvas: DisplayManager never calls
|
||||
# plan_refresh(), since their frames are not paced by a panel.
|
||||
r = _recorder(tmp_path)
|
||||
caplog.set_level("WARNING")
|
||||
_windows(r, 6, SLOW)
|
||||
assert _shortfall_warnings(caplog) == []
|
||||
|
||||
|
||||
def test_the_snapshot_records_the_planned_rate(tmp_path):
|
||||
r = _recorder(tmp_path)
|
||||
assert r.snapshot()["planned_refresh_hz"] is None
|
||||
r.plan_refresh(120.0)
|
||||
assert r.snapshot()["planned_refresh_hz"] == 120.0
|
||||
|
||||
|
||||
def test_windows_the_period_rejected_do_not_count_toward_the_warning(tmp_path, caplog):
|
||||
# A loaded start fixed 60 Hz (two windows agreed); the panel really runs at
|
||||
# 100 Hz, but a window that much faster is ignored by the estimate, so the
|
||||
# period stays 60 Hz. Warning "60 Hz is under your 100 Hz cap" would be wrong.
|
||||
r = _recorder(tmp_path)
|
||||
r.plan_refresh(100.0)
|
||||
caplog.set_level("WARNING")
|
||||
_windows(r, 2, 1 / 60.0)
|
||||
assert abs(1.0 / r.refresh_period - 60.0) < 0.5
|
||||
_windows(r, 6, PERIOD, start=1000.0)
|
||||
assert abs(1.0 / r.refresh_period - 60.0) < 0.5 # still ignored
|
||||
assert _shortfall_warnings(caplog) == []
|
||||
|
||||
|
||||
def test_one_disagreeing_window_restarts_the_run(tmp_path, caplog):
|
||||
r = _recorder(tmp_path)
|
||||
r.plan_refresh(120.0)
|
||||
caplog.set_level("WARNING")
|
||||
_windows(r, 3, SLOW) # two windows toward three
|
||||
_windows(r, 1, 1 / 250.0, start=1000.0) # far faster: rejected, resets
|
||||
_windows(r, 1, SLOW, start=2000.0)
|
||||
assert _shortfall_warnings(caplog) == []
|
||||
_windows(r, 2, SLOW, start=3000.0) # three in a row now
|
||||
assert len(_shortfall_warnings(caplog)) == 1
|
||||
|
||||
@@ -21,6 +21,7 @@ from src.common.scroll_config import ( # noqa: E402
|
||||
refresh_hz_from_config,
|
||||
resolve,
|
||||
)
|
||||
from src.common import scroll_config # noqa: E402
|
||||
|
||||
|
||||
class FakeHelper:
|
||||
@@ -504,3 +505,46 @@ class TestSpeedAdvice:
|
||||
got = solve_crisp(50, 125.74)
|
||||
assert got.steppiness == "smooth"
|
||||
assert got.pixels_per_frame == 1
|
||||
|
||||
|
||||
class TestRefreshShortfall:
|
||||
"""A panel that cannot reach its cap runs every scroll slow."""
|
||||
|
||||
def test_the_ledmatrix_rig_is_told_to_cap_at_100(self):
|
||||
# Pi 4, 2x128x64 on adafruit-hat-pwm under a 120 Hz cap: measured
|
||||
# 107.6-113.1 Hz, and frame_timing reports the fast end.
|
||||
s = scroll_config.refresh_shortfall(113.1, 120)
|
||||
assert s == {"measured_hz": 113.1, "planned_hz": 120.0,
|
||||
"suggested_cap_hz": 100, "slow_percent": 6}
|
||||
|
||||
def test_a_panel_that_holds_its_cap_is_fine(self):
|
||||
assert scroll_config.refresh_shortfall(99.95, 100) is None
|
||||
assert scroll_config.refresh_shortfall(97.5, 100) is None
|
||||
|
||||
def test_a_panel_that_beats_its_cap_is_fine(self):
|
||||
assert scroll_config.refresh_shortfall(125.7, 120) is None
|
||||
|
||||
def test_nothing_measured_says_nothing(self):
|
||||
assert scroll_config.refresh_shortfall(None, 120) is None
|
||||
assert scroll_config.refresh_shortfall(0, 120) is None
|
||||
assert scroll_config.refresh_shortfall("fast", 120) is None
|
||||
|
||||
def test_the_suggestion_leaves_headroom_under_the_measurement(self):
|
||||
assert scroll_config.holdable_cap(113.1) == 100
|
||||
assert scroll_config.holdable_cap(95.0) == 90
|
||||
# 5% under 105 is 99.75: 100 would sit inside the panel's drift.
|
||||
assert scroll_config.holdable_cap(105.0) == 90
|
||||
assert scroll_config.holdable_cap(9.0) is None
|
||||
assert scroll_config.holdable_cap(None) is None
|
||||
|
||||
def test_the_log_line_names_the_cap_to_use(self):
|
||||
text = scroll_config.describe_refresh_shortfall(
|
||||
scroll_config.refresh_shortfall(113.1, 120))
|
||||
assert "about 113 Hz" in text and "120 Hz" in text
|
||||
assert "6% slower" in text
|
||||
assert "Set Limit Refresh Rate to 100 Hz" in text
|
||||
|
||||
def test_no_suggestion_for_a_panel_too_slow_for_any_cap(self):
|
||||
text = scroll_config.describe_refresh_shortfall(
|
||||
scroll_config.refresh_shortfall(9.0, 100))
|
||||
assert "Set Limit Refresh Rate" not in text
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
"""GET /api/v3/config/refresh-rate: the cap, the measured rate, a cap to hold."""
|
||||
import json
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
from web_interface.blueprints.api_v3 import api_v3
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client(monkeypatch, tmp_path):
|
||||
stats = tmp_path / "stats.json"
|
||||
monkeypatch.setattr("src.common.frame_timing.default_stats_path", lambda: str(stats))
|
||||
manager = MagicMock()
|
||||
manager.load_config.return_value = {
|
||||
"display": {"hardware": {"limit_refresh_rate_hz": 120}}}
|
||||
monkeypatch.setattr(api_v3, "config_manager", manager, raising=False)
|
||||
app = Flask(__name__)
|
||||
app.register_blueprint(api_v3, url_prefix="/api/v3")
|
||||
c = app.test_client()
|
||||
c.stats_path = stats
|
||||
return c
|
||||
|
||||
|
||||
def _get(client):
|
||||
body = client.get("/api/v3/config/refresh-rate").get_json()
|
||||
assert body["status"] == "success"
|
||||
return body["data"]
|
||||
|
||||
|
||||
def test_nothing_measured_yet(client):
|
||||
data = _get(client)
|
||||
assert data == {"planned_hz": 120.0, "measured_hz": None, "shortfall": None}
|
||||
|
||||
|
||||
def test_a_panel_short_of_its_cap_gets_a_cap_it_can_hold(client):
|
||||
client.stats_path.write_text(json.dumps(
|
||||
{"measured_refresh_hz": 110.4, "planned_refresh_hz": 120.0}))
|
||||
data = _get(client)
|
||||
assert data["measured_hz"] == 110.4
|
||||
assert data["shortfall"]["suggested_cap_hz"] == 100
|
||||
assert data["shortfall"]["slow_percent"] == 8
|
||||
|
||||
|
||||
def test_a_panel_at_its_cap_has_no_shortfall(client):
|
||||
client.stats_path.write_text(json.dumps(
|
||||
{"measured_refresh_hz": 121.3, "planned_refresh_hz": 120.0}))
|
||||
assert _get(client)["shortfall"] is None
|
||||
|
||||
|
||||
def test_a_file_written_under_another_cap_is_stale(client):
|
||||
# The cap was changed to 120 but the display still runs under 100 Hz.
|
||||
client.stats_path.write_text(json.dumps(
|
||||
{"measured_refresh_hz": 99.9, "planned_refresh_hz": 100.0}))
|
||||
data = _get(client)
|
||||
assert data["measured_hz"] is None
|
||||
assert data["shortfall"] is None
|
||||
|
||||
|
||||
def test_a_file_from_a_display_too_old_to_record_its_cap_still_counts(client):
|
||||
client.stats_path.write_text(json.dumps({"measured_refresh_hz": 110.4}))
|
||||
assert _get(client)["shortfall"]["suggested_cap_hz"] == 100
|
||||
|
||||
|
||||
def test_a_measurement_recorded_without_a_planned_rate_is_not_a_panel(client):
|
||||
# The emulator and the fallback canvas write the key as null: their frames
|
||||
# are not paced by a panel, so a rate under the cap is no shortfall.
|
||||
client.stats_path.write_text(json.dumps(
|
||||
{"measured_refresh_hz": 60.0, "planned_refresh_hz": None}))
|
||||
data = _get(client)
|
||||
assert data["measured_hz"] is None
|
||||
assert data["shortfall"] is None
|
||||
Reference in New Issue
Block a user