From c999cd676e21d9086b387fefcbc7eccb139a98a1 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:02:41 +0200 Subject: [PATCH 01/14] fix(dashboard): route widget-addressing methods through active-page list (P0-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setWidgetPosition/getWidgetByTitle/markAllDirty/preview read obj.Widgets directly, so they silently broke once addPage() was used — widgets then live under obj.Pages{ActivePage}.Widgets and obj.Widgets is empty {}. getWidgetByTitle returned [] for widgets that exist, setWidgetPosition threw invalidIndex for valid indices, markAllDirty flagged nothing, and preview printed "(empty)". Route all four through the existing activePageWidgets()/allPageWidgets() accessors, mirroring removeWidget. Single-page behaviour is byte-identical. Adds multi-page + single-page regression tests to TestDashboardMultiPage. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 3882536d8ab739770b223815b9d074073b5dc41f) --- libs/Dashboard/DashboardEngine.m | 39 ++++++++++-------- tests/suite/TestDashboardMultiPage.m | 59 ++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 16 deletions(-) diff --git a/libs/Dashboard/DashboardEngine.m b/libs/Dashboard/DashboardEngine.m index bc92e892..186c3fa4 100644 --- a/libs/Dashboard/DashboardEngine.m +++ b/libs/Dashboard/DashboardEngine.m @@ -1180,7 +1180,8 @@ function preview(obj, varargin) width = 48; end - nWidgets = numel(obj.Widgets); + ws = obj.activePageWidgets(); + nWidgets = numel(ws); % Empty dashboard if nWidgets == 0 @@ -1192,7 +1193,7 @@ function preview(obj, varargin) cols = obj.Layout.Columns; maxRow = 1; for i = 1:nWidgets - p = obj.Widgets{i}.Position; + p = ws{i}.Position; bottomRow = p(2) + p(4) - 1; if bottomRow > maxRow maxRow = bottomRow; @@ -1210,7 +1211,7 @@ function preview(obj, varargin) % Render each widget for i = 1:nWidgets - w = obj.Widgets{i}; + w = ws{i}; p = w.Position; % [col, row, wCols, hRows] % Character coordinates (1-based) @@ -1496,10 +1497,11 @@ function removeWidget(obj, idx) function setWidgetPosition(obj, idx, pos) %SETWIDGETPOSITION Set the grid position of a widget by index. % Clamps width to grid columns and resolves overlaps with other - % widgets. - if idx < 1 || idx > numel(obj.Widgets) + % widgets. Operates on the active page in multi-page mode. + ws = obj.activePageWidgets(); + if idx < 1 || idx > numel(ws) error('DashboardEngine:invalidIndex', ... - 'Widget index %d out of range [1, %d].', idx, numel(obj.Widgets)); + 'Widget index %d out of range [1, %d].', idx, numel(ws)); end % Clamp to grid bounds cols = obj.Layout.Columns; @@ -1508,25 +1510,28 @@ function setWidgetPosition(obj, idx, pos) pos(2) = max(1, pos(2)); pos(4) = max(1, pos(4)); % Resolve overlaps against other widgets - existingPositions = cell(1, numel(obj.Widgets) - 1); + existingPositions = cell(1, numel(ws) - 1); k = 0; - for i = 1:numel(obj.Widgets) + for i = 1:numel(ws) if i ~= idx k = k + 1; - existingPositions{k} = obj.Widgets{i}.Position; + existingPositions{k} = ws{i}.Position; end end pos = obj.Layout.resolveOverlap(pos, existingPositions); - obj.Widgets{idx}.Position = pos; + % ws{idx} is a handle, so this mutates the widget stored in the page. + ws{idx}.Position = pos; end function w = getWidgetByTitle(obj, title) %GETWIDGETBYTITLE Find a widget by its Title property. - % Returns the widget object, or empty if not found. + % Searches every page in multi-page mode (active page or single-page + % Widgets otherwise). Returns the widget object, or empty if not found. w = []; - for i = 1:numel(obj.Widgets) - if strcmp(obj.Widgets{i}.Title, title) - w = obj.Widgets{i}; + ws = obj.allPageWidgets(); + for i = 1:numel(ws) + if strcmp(ws{i}.Title, title) + w = ws{i}; return; end end @@ -2257,8 +2262,10 @@ function onLiveTick(obj) function markAllDirty(obj) %MARKALLDIRTY Flag all widgets as needing refresh. % Called on theme change, figure resize, or other global state changes. - for i = 1:numel(obj.Widgets) - obj.Widgets{i}.markDirty(); + % Covers every page in multi-page mode. + ws = obj.allPageWidgets(); + for i = 1:numel(ws) + ws{i}.markDirty(); end end diff --git a/tests/suite/TestDashboardMultiPage.m b/tests/suite/TestDashboardMultiPage.m index 73d76600..90b01463 100644 --- a/tests/suite/TestDashboardMultiPage.m +++ b/tests/suite/TestDashboardMultiPage.m @@ -124,6 +124,65 @@ function testLiveTickScopedToActivePage(testCase) testCase.verifyEqual(d.ActivePage, 1); end + function testMultiPageWidgetAddressing(testCase) + %TESTMULTIPAGEWIDGETADDRESSING P0-1: widget-addressing methods are page-aware. + % getWidgetByTitle/setWidgetPosition/markAllDirty/preview must operate on + % the active page's widgets, not the (empty) obj.Widgets, once addPage is used. + d = DashboardEngine('mp'); + d.addPage('P1'); + w = MockDashboardWidget('Title', 'Alpha', 'Position', [1 1 4 2]); + d.addWidget(w); + % Widget lives under the page; obj.Widgets stays empty in multi-page mode. + testCase.verifyEmpty(d.Widgets); + testCase.verifyEqual(numel(d.Pages{1}.Widgets), 1); + + % getWidgetByTitle finds the page widget (returned [] before P0-1 fix). + g = d.getWidgetByTitle('Alpha'); + testCase.verifyTrue(~isempty(g) && g == w); + + % setWidgetPosition on a valid index does not throw and moves the widget + % (threw DashboardEngine:invalidIndex before P0-1 fix). + threw = false; + try + d.setWidgetPosition(1, [3 3 4 2]); + catch + threw = true; + end + testCase.verifyFalse(threw); + testCase.verifyEqual(w.Position(1), 3); + + % markAllDirty flags the page widget (was a no-op before P0-1 fix). + w.Dirty = false; + d.markAllDirty(); + testCase.verifyTrue(w.Dirty); + + % preview reports the widget instead of "(empty)" (printed empty before fix). + out = evalc('d.preview(''Width'', 120)'); + testCase.verifyFalse(contains(out, 'empty -- no widgets')); + testCase.verifyTrue(contains(out, '1 widgets')); + end + + function testSinglePageWidgetAddressingUnchanged(testCase) + %TESTSINGLEPAGEWIDGETADDRESSINGUNCHANGED P0-1 regression: single-page path intact. + d = DashboardEngine('sp'); + w = MockDashboardWidget('Title', 'Solo', 'Position', [1 1 4 2]); + d.addWidget(w); + testCase.verifyEqual(numel(d.Widgets), 1); + g = d.getWidgetByTitle('Solo'); + testCase.verifyTrue(~isempty(g) && g == w); + threw = false; + try + d.setWidgetPosition(1, [2 2 4 2]); + catch + threw = true; + end + testCase.verifyFalse(threw); + testCase.verifyEqual(w.Position(1), 2); + w.Dirty = false; + d.markAllDirty(); + testCase.verifyTrue(w.Dirty); + end + end end From e37b9ab1d739405609d91353e7e868c564c060eb Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:07:23 +0200 Subject: [PATCH 02/14] fix(dashboard): chart widgets read Tag data via getXY() contract (P0-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BarChart/Histogram/Heatmap/Scatter read obj.Sensor.Y (and SensorX/SensorY.Y) directly. DerivedTag and CompositeTag expose no public .Y, so binding one to these widgets threw MATLAB:noSuchMethodOrField at refresh — even though they are first-class members of the documented Tag hierarchy and getXY() is the contract data accessor (FastSenseWidget already uses it). Route all four widgets' refresh and asciiRender data reads through Tag.getXY(), which works uniformly across SensorTag/StateTag/DerivedTag/CompositeTag. SensorTag and DataFcn paths unchanged. Adds a DerivedTag-refresh regression test to each of the four widget suites. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 557a0315c36d726a1c83703ce50813f8e5316816) --- libs/Dashboard/BarChartWidget.m | 9 ++++++--- libs/Dashboard/HeatmapWidget.m | 8 +++++--- libs/Dashboard/HistogramWidget.m | 20 +++++++++++++------- libs/Dashboard/ScatterWidget.m | 29 ++++++++++++++++++++--------- tests/suite/TestBarChartWidget.m | 20 ++++++++++++++++++++ tests/suite/TestHeatmapWidget.m | 20 ++++++++++++++++++++ tests/suite/TestHistogramWidget.m | 20 ++++++++++++++++++++ tests/suite/TestScatterWidget.m | 22 ++++++++++++++++++++++ 8 files changed, 126 insertions(+), 22 deletions(-) diff --git a/libs/Dashboard/BarChartWidget.m b/libs/Dashboard/BarChartWidget.m index 6a476861..bccead1c 100644 --- a/libs/Dashboard/BarChartWidget.m +++ b/libs/Dashboard/BarChartWidget.m @@ -42,9 +42,12 @@ function refresh(obj) data = []; cats = {}; - if ~isempty(obj.Sensor) - if isempty(obj.Sensor.Y), return; end - data = obj.Sensor.Y; + if ~isempty(obj.Tag) + % Read via the Tag.getXY() contract so Derived/Composite tags + % (which expose no public .Y) work, not just SensorTag/StateTag. + [~, y] = obj.Tag.getXY(); + if isempty(y), return; end + data = y; elseif ~isempty(obj.DataFcn) result = obj.DataFcn(); if isstruct(result) diff --git a/libs/Dashboard/HeatmapWidget.m b/libs/Dashboard/HeatmapWidget.m index 015f2972..e70c0327 100644 --- a/libs/Dashboard/HeatmapWidget.m +++ b/libs/Dashboard/HeatmapWidget.m @@ -48,9 +48,11 @@ function refresh(obj) end data = []; - if ~isempty(obj.Sensor) - if isempty(obj.Sensor.Y), return; end - data = obj.Sensor.Y; + if ~isempty(obj.Tag) + % Read via the Tag.getXY() contract (Derived/Composite tags have no .Y). + [~, y] = obj.Tag.getXY(); + if isempty(y), return; end + data = y; elseif ~isempty(obj.DataFcn) data = obj.DataFcn(); end diff --git a/libs/Dashboard/HistogramWidget.m b/libs/Dashboard/HistogramWidget.m index 1dc0f173..4f024c5e 100644 --- a/libs/Dashboard/HistogramWidget.m +++ b/libs/Dashboard/HistogramWidget.m @@ -44,9 +44,11 @@ function refresh(obj) end data = []; - if ~isempty(obj.Sensor) - if isempty(obj.Sensor.Y), return; end - data = obj.Sensor.Y(:)'; + if ~isempty(obj.Tag) + % Read via the Tag.getXY() contract (Derived/Composite tags have no .Y). + [~, y] = obj.Tag.getXY(); + if isempty(y), return; end + data = y(:)'; elseif ~isempty(obj.DataFcn) data = obj.DataFcn(); data = data(:)'; @@ -100,10 +102,14 @@ function refresh(obj) lines{1} = [ttl, repmat(' ', 1, width - numel(ttl))]; if height >= 2 - hasData = (~isempty(obj.Sensor) && ~isempty(obj.Sensor.Y)) || ... - ~isempty(obj.DataFcn); - if hasData && ~isempty(obj.Sensor) - info = sprintf('%d data points', numel(obj.Sensor.Y)); + if ~isempty(obj.Tag) + [~, yProbe] = obj.Tag.getXY(); + else + yProbe = []; + end + hasData = ~isempty(yProbe) || ~isempty(obj.DataFcn); + if hasData && ~isempty(obj.Tag) + info = sprintf('%d data points', numel(yProbe)); else info = '[-- histogram --]'; end diff --git a/libs/Dashboard/ScatterWidget.m b/libs/Dashboard/ScatterWidget.m index d7fc7ccc..0947999e 100644 --- a/libs/Dashboard/ScatterWidget.m +++ b/libs/Dashboard/ScatterWidget.m @@ -45,10 +45,13 @@ function refresh(obj) xData = []; yData = []; if ~isempty(obj.SensorX) && ~isempty(obj.SensorY) - if isempty(obj.SensorX.Y) || isempty(obj.SensorY.Y), return; end - n = min(numel(obj.SensorX.Y), numel(obj.SensorY.Y)); - xData = obj.SensorX.Y(1:n); - yData = obj.SensorY.Y(1:n); + % Read via the Tag.getXY() contract (Derived/Composite tags have no .Y). + [~, yx] = obj.SensorX.getXY(); + [~, yy] = obj.SensorY.getXY(); + if isempty(yx) || isempty(yy), return; end + n = min(numel(yx), numel(yy)); + xData = yx(1:n); + yData = yy(1:n); end if isempty(xData), return; end @@ -71,8 +74,14 @@ function refresh(obj) end cla(obj.hAxes); - if ~isempty(obj.SensorColor) && ~isempty(obj.SensorColor.Y) - cData = obj.SensorColor.Y(1:min(numel(obj.SensorColor.Y), numel(xData))); + cData = []; + if ~isempty(obj.SensorColor) + [~, yc] = obj.SensorColor.getXY(); + if ~isempty(yc) + cData = yc(1:min(numel(yc), numel(xData))); + end + end + if ~isempty(cData) % Use line with markers for Octave compatibility obj.hScatter = scatter(obj.hAxes, xData, yData, obj.MarkerSize, cData, 'filled'); colormap(obj.hAxes, obj.Colormap); @@ -112,9 +121,11 @@ function refresh(obj) lines{1} = [ttl, repmat(' ', 1, width - numel(ttl))]; if height >= 2 - if ~isempty(obj.SensorX) && ~isempty(obj.SensorY) && ... - ~isempty(obj.SensorX.Y) && ~isempty(obj.SensorY.Y) - n = min(numel(obj.SensorX.Y), numel(obj.SensorY.Y)); + yx = []; yy = []; + if ~isempty(obj.SensorX), [~, yx] = obj.SensorX.getXY(); end + if ~isempty(obj.SensorY), [~, yy] = obj.SensorY.getXY(); end + if ~isempty(yx) && ~isempty(yy) + n = min(numel(yx), numel(yy)); info = sprintf('%d points', n); else info = '[-- scatter --]'; diff --git a/tests/suite/TestBarChartWidget.m b/tests/suite/TestBarChartWidget.m index 62d499a4..bdebfad9 100644 --- a/tests/suite/TestBarChartWidget.m +++ b/tests/suite/TestBarChartWidget.m @@ -35,5 +35,25 @@ function testToStruct(testCase) testCase.verifyEqual(s.orientation, 'horizontal'); testCase.verifyEqual(s.stacked, true); end + + function testDerivedTagRefreshNoCrash(testCase) + %TESTDERIVEDTAGREFRESHNOCRASH P0-2: a DerivedTag (no public .Y) must refresh via getXY(). + a = SensorTag('a', 'X', 1:10, 'Y', 1:10); + b = SensorTag('b', 'X', 1:10, 'Y', 2:11); + d = DerivedTag('d', {a, b}, @(p) deal(p{1}.X, p{1}.Y + p{2}.Y)); + w = BarChartWidget('Title', 'Bar'); + w.Tag = d; + fig = figure('Visible', 'off'); + cleanup = onCleanup(@() close(fig)); %#ok + hp = uipanel(fig, 'Position', [0 0 1 1]); + w.ParentTheme = DashboardTheme('dark'); + threw = false; msg = ''; + try + w.render(hp); % refresh() read obj.Sensor.Y -> threw before P0-2 fix + catch err + threw = true; msg = err.message; + end + testCase.verifyFalse(threw, msg); + end end end diff --git a/tests/suite/TestHeatmapWidget.m b/tests/suite/TestHeatmapWidget.m index bd2649bc..5edd8583 100644 --- a/tests/suite/TestHeatmapWidget.m +++ b/tests/suite/TestHeatmapWidget.m @@ -35,5 +35,25 @@ function testToStructRoundTrip(testCase) testCase.verifyEqual(s.colormap, 'jet'); testCase.verifyEqual(s.showColorbar, false); end + + function testDerivedTagRefreshNoCrash(testCase) + %TESTDERIVEDTAGREFRESHNOCRASH P0-2: a DerivedTag (no public .Y) must refresh via getXY(). + a = SensorTag('a', 'X', 1:10, 'Y', 1:10); + b = SensorTag('b', 'X', 1:10, 'Y', 2:11); + d = DerivedTag('d', {a, b}, @(p) deal(p{1}.X, p{1}.Y + p{2}.Y)); + w = HeatmapWidget('Title', 'Heat'); + w.Tag = d; + fig = figure('Visible', 'off'); + cleanup = onCleanup(@() close(fig)); %#ok + hp = uipanel(fig, 'Position', [0 0 1 1]); + w.ParentTheme = DashboardTheme('dark'); + threw = false; msg = ''; + try + w.render(hp); % refresh() read obj.Sensor.Y -> threw before P0-2 fix + catch err + threw = true; msg = err.message; + end + testCase.verifyFalse(threw, msg); + end end end diff --git a/tests/suite/TestHistogramWidget.m b/tests/suite/TestHistogramWidget.m index bc0f9129..0b2b6555 100644 --- a/tests/suite/TestHistogramWidget.m +++ b/tests/suite/TestHistogramWidget.m @@ -35,5 +35,25 @@ function testToStruct(testCase) testCase.verifyEqual(s.numBins, 20); testCase.verifyEqual(s.showNormalFit, true); end + + function testDerivedTagRefreshNoCrash(testCase) + %TESTDERIVEDTAGREFRESHNOCRASH P0-2: a DerivedTag (no public .Y) must refresh via getXY(). + a = SensorTag('a', 'X', 1:10, 'Y', 1:10); + b = SensorTag('b', 'X', 1:10, 'Y', 2:11); + d = DerivedTag('d', {a, b}, @(p) deal(p{1}.X, p{1}.Y + p{2}.Y)); + w = HistogramWidget('Title', 'Hist'); + w.Tag = d; + fig = figure('Visible', 'off'); + cleanup = onCleanup(@() close(fig)); %#ok + hp = uipanel(fig, 'Position', [0 0 1 1]); + w.ParentTheme = DashboardTheme('dark'); + threw = false; msg = ''; + try + w.render(hp); % refresh() read obj.Sensor.Y -> threw before P0-2 fix + catch err + threw = true; msg = err.message; + end + testCase.verifyFalse(threw, msg); + end end end diff --git a/tests/suite/TestScatterWidget.m b/tests/suite/TestScatterWidget.m index 25e5c589..65d7e51e 100644 --- a/tests/suite/TestScatterWidget.m +++ b/tests/suite/TestScatterWidget.m @@ -21,5 +21,27 @@ function testToStruct(testCase) testCase.verifyEqual(s.type, 'scatter'); testCase.verifyEqual(s.markerSize, 10); end + + function testDerivedTagRefreshNoCrash(testCase) + %TESTDERIVEDTAGREFRESHNOCRASH P0-2: DerivedTag SensorX/SensorY must refresh via getXY(). + a = SensorTag('a', 'X', 1:10, 'Y', 1:10); + b = SensorTag('b', 'X', 1:10, 'Y', 2:11); + dx = DerivedTag('dx', {a, b}, @(p) deal(p{1}.X, p{1}.Y + p{2}.Y)); + dy = DerivedTag('dy', {a, b}, @(p) deal(p{1}.X, p{2}.Y)); + w = ScatterWidget('Title', 'Scatter'); + w.SensorX = dx; + w.SensorY = dy; + fig = figure('Visible', 'off'); + cleanup = onCleanup(@() close(fig)); %#ok + hp = uipanel(fig, 'Position', [0 0 1 1]); + w.ParentTheme = DashboardTheme('dark'); + threw = false; msg = ''; + try + w.render(hp); % refresh() read obj.SensorX.Y -> threw before P0-2 fix + catch err + threw = true; msg = err.message; + end + testCase.verifyFalse(threw, msg); + end end end From 53dcdd0daee5f0ea75fd6192afbf9537506bbe8b Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:14:33 +0200 Subject: [PATCH 03/14] fix(dashboard): restore chart data bindings on load; delegate save() to linesForWidget (P0-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent data-loss bugs on the serialized-dashboard backward-compat path: 1. BarChart/Histogram/Heatmap/Scatter toStruct wrote their data source (callback function / dual-sensor keys) but fromStruct never read it, so save/load returned charts with no binding — they rendered empty with no warning. Each fromStruct now restores its source (str2func for callbacks, TagRegistry for Scatter sensors), warns with a namespaced *:sourceUnresolved id when a key can't be resolved, and stays silent for old structs that have no source field. 2. DashboardSerializer.save() open-coded a per-type switch that was a lossy duplicate of linesForWidget — number/status/gauge .m export dropped the ValueFcn/StaticValue/StatusFcn bindings that exportScript preserves. save() now delegates per-widget emission to linesForWidget (keeping the group case, which needs a captured gN variable for addChild), so all bindings survive. Adds chart-callback, Scatter-sensor, old-struct-quiet, and save-binding tests to TestDashboardSerializerRoundTrip. Group/.m-export round-trips (MSerializer) stay green. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 262a8ca9b1837d2a82fbc98e9abac521f95e4b7a) --- libs/Dashboard/BarChartWidget.m | 12 ++ libs/Dashboard/DashboardSerializer.m | 158 +++++------------- libs/Dashboard/HeatmapWidget.m | 11 ++ libs/Dashboard/HistogramWidget.m | 11 ++ libs/Dashboard/ScatterWidget.m | 23 +++ .../suite/TestDashboardSerializerRoundTrip.m | 55 ++++++ 6 files changed, 151 insertions(+), 119 deletions(-) diff --git a/libs/Dashboard/BarChartWidget.m b/libs/Dashboard/BarChartWidget.m index bccead1c..771587f8 100644 --- a/libs/Dashboard/BarChartWidget.m +++ b/libs/Dashboard/BarChartWidget.m @@ -143,6 +143,18 @@ function refresh(obj) end if isfield(s, 'orientation'), obj.Orientation = s.orientation; end if isfield(s, 'stacked'), obj.Stacked = s.stacked; end + % Restore the data binding (callback) — dropped before P0-3. + % Old structs without s.source load with no binding and no warning. + if isfield(s, 'source') && isfield(s.source, 'type') + switch s.source.type + case 'callback' + obj.DataFcn = str2func(s.source.function); + otherwise + warning('BarChartWidget:sourceUnresolved', ... + 'Unresolved source type ''%s'' for BarChart ''%s''.', ... + s.source.type, obj.Title); + end + end end end end diff --git a/libs/Dashboard/DashboardSerializer.m b/libs/Dashboard/DashboardSerializer.m index 151559f6..8b951ae3 100644 --- a/libs/Dashboard/DashboardSerializer.m +++ b/libs/Dashboard/DashboardSerializer.m @@ -32,129 +32,49 @@ function save(config, filepath) pos = sprintf('[%d %d %d %d]', ws.position.col, ws.position.row, ... ws.position.width, ws.position.height); - switch ws.type - case 'fastsense' - showPl = isfield(ws, 'showPlantLog') && ws.showPlantLog; - if isfield(ws, 'source') - switch ws.source.type - case 'sensor' - lines{end+1} = sprintf(' w = d.addWidget(''fastsense'', ''Title'', ''%s'', ...', ws.title); - lines{end+1} = sprintf(' ''Position'', %s, ...', pos); - if showPl - lines{end+1} = sprintf(' ''Tag'', TagRegistry.get(''%s''), ...', ws.source.name); - lines{end+1} = sprintf(' ''ShowPlantLog'', true);'); - else - lines{end+1} = sprintf(' ''Tag'', TagRegistry.get(''%s''));', ws.source.name); - end - case 'file' - lines{end+1} = sprintf(' w = d.addWidget(''fastsense'', ''Title'', ''%s'', ...', ws.title); - lines{end+1} = sprintf(' ''Position'', %s, ...', pos); - if showPl - lines{end+1} = sprintf(' ''File'', ''%s'', ''XVar'', ''%s'', ''YVar'', ''%s'', ...', ... - ws.source.path, ws.source.xVar, ws.source.yVar); - lines{end+1} = sprintf(' ''ShowPlantLog'', true);'); - else - lines{end+1} = sprintf(' ''File'', ''%s'', ''XVar'', ''%s'', ''YVar'', ''%s'');', ... - ws.source.path, ws.source.xVar, ws.source.yVar); - end - case 'data' - lines{end+1} = sprintf(' w = d.addWidget(''fastsense'', ''Title'', ''%s'', ...', ws.title); - lines{end+1} = sprintf(' ''Position'', %s, ...', pos); - if showPl - lines{end+1} = sprintf(' ''XData'', %s, ''YData'', %s, ...', ... - mat2str(ws.source.x), mat2str(ws.source.y)); - lines{end+1} = sprintf(' ''ShowPlantLog'', true);'); - else - lines{end+1} = sprintf(' ''XData'', %s, ''YData'', %s);', ... - mat2str(ws.source.x), mat2str(ws.source.y)); - end - otherwise - if showPl - lines{end+1} = sprintf( ... - ' d.addWidget(''fastsense'', ''Title'', ''%s'', ''Position'', %s, ''ShowPlantLog'', true);', ... - ws.title, pos); - else - lines{end+1} = sprintf(' d.addWidget(''fastsense'', ''Title'', ''%s'', ''Position'', %s);', ws.title, pos); - end - end - else - if showPl - lines{end+1} = sprintf( ... - ' d.addWidget(''fastsense'', ''Title'', ''%s'', ''Position'', %s, ''ShowPlantLog'', true);', ... - ws.title, pos); - else - lines{end+1} = sprintf(' d.addWidget(''fastsense'', ''Title'', ''%s'', ''Position'', %s);', ws.title, pos); - end - end - case 'number' - line = sprintf(' d.addWidget(''number'', ''Title'', ''%s'', ''Position'', %s', ws.title, pos); - if isfield(ws, 'units') && ~isempty(ws.units) - line = [line, sprintf(', ...\n ''Units'', ''%s''', ws.units)]; - end - lines{end+1} = [line, ');']; - case 'status' - line = sprintf(' d.addWidget(''status'', ''Title'', ''%s'', ''Position'', %s', ws.title, pos); - lines{end+1} = [line, ');']; - case 'text' - line = sprintf(' d.addWidget(''text'', ''Title'', ''%s'', ''Position'', %s', ws.title, pos); - if isfield(ws, 'content') && ~isempty(ws.content) - line = [line, sprintf(', ...\n ''Content'', ''%s''', ws.content)]; - end - lines{end+1} = [line, ');']; - case 'gauge' - line = sprintf(' d.addWidget(''gauge'', ''Title'', ''%s'', ''Position'', %s', ws.title, pos); - if isfield(ws, 'range') - line = [line, sprintf(', ...\n ''Range'', [%g %g]', ws.range(1), ws.range(2))]; - end - if isfield(ws, 'units') && ~isempty(ws.units) - line = [line, sprintf(', ...\n ''Units'', ''%s''', ws.units)]; - end - lines{end+1} = [line, ');']; - case 'group' - groupVarName = sprintf('g%d', groupCount); - groupCount = groupCount + 1; - line = sprintf(' %s = d.addWidget(''group'', ''Label'', ''%s'', ''Position'', %s', ... - groupVarName, ws.label, pos); - if isfield(ws, 'mode') && ~isempty(ws.mode) - line = [line, sprintf(', ...\n ''Mode'', ''%s''', ws.mode)]; - end - lines{end+1} = [line, ');']; - % Emit children - if isfield(ws, 'mode') && strcmp(ws.mode, 'tabbed') && isfield(ws, 'tabs') && ~isempty(ws.tabs) - tabs = normalizeToCell(ws.tabs); - for ti = 1:numel(tabs) - tab = tabs{ti}; - tabWidgets = normalizeToCell(tab.widgets); - for ci = 1:numel(tabWidgets) - [childLines, childVar, groupCount] = ... - DashboardSerializer.emitChildWidget(tabWidgets{ci}, groupCount); - lines = [lines, childLines]; - lines{end+1} = sprintf(' %s.addChild(%s, ''%s'');', ... - groupVarName, childVar, tab.name); - end - end - elseif isfield(ws, 'children') && ~isempty(ws.children) - ch = normalizeToCell(ws.children); - for ci = 1:numel(ch) + if strcmp(ws.type, 'group') + % Groups need a captured variable (gN) so children can be + % attached via gN.addChild(...). linesForWidget emits only the + % group header, so keep the group-aware emission here. + groupVarName = sprintf('g%d', groupCount); + groupCount = groupCount + 1; + line = sprintf(' %s = d.addWidget(''group'', ''Label'', ''%s'', ''Position'', %s', ... + groupVarName, ws.label, pos); + if isfield(ws, 'mode') && ~isempty(ws.mode) + line = [line, sprintf(', ...\n ''Mode'', ''%s''', ws.mode)]; + end + lines{end+1} = [line, ');']; + % Emit children + if isfield(ws, 'mode') && strcmp(ws.mode, 'tabbed') && isfield(ws, 'tabs') && ~isempty(ws.tabs) + tabs = normalizeToCell(ws.tabs); + for ti = 1:numel(tabs) + tab = tabs{ti}; + tabWidgets = normalizeToCell(tab.widgets); + for ci = 1:numel(tabWidgets) [childLines, childVar, groupCount] = ... - DashboardSerializer.emitChildWidget(ch{ci}, groupCount); + DashboardSerializer.emitChildWidget(tabWidgets{ci}, groupCount); lines = [lines, childLines]; - lines{end+1} = sprintf(' %s.addChild(%s);', groupVarName, childVar); + lines{end+1} = sprintf(' %s.addChild(%s, ''%s'');', ... + groupVarName, childVar, tab.name); end end - case 'divider' - lines{end+1} = sprintf(' d.addWidget(''divider'', ''Position'', %s);', pos); - case 'iconcard' - lines{end+1} = sprintf(' d.addWidget(''iconcard'', ''Title'', ''%s'', ...', ws.title); - lines{end+1} = sprintf(' ''Position'', %s);', pos); - case 'chipbar' - lines{end+1} = sprintf(' d.addWidget(''chipbar'', ''Title'', ''%s'', ...', ws.title); - lines{end+1} = sprintf(' ''Position'', %s);', pos); - case 'sparkline' - lines{end+1} = sprintf(' d.addWidget(''sparkline'', ''Title'', ''%s'', ...', ws.title); - lines{end+1} = sprintf(' ''Position'', %s);', pos); - otherwise - lines{end+1} = sprintf(' d.addWidget(''%s'', ''Title'', ''%s'', ''Position'', %s);', ws.type, ws.title, pos); + elseif isfield(ws, 'children') && ~isempty(ws.children) + ch = normalizeToCell(ws.children); + for ci = 1:numel(ch) + [childLines, childVar, groupCount] = ... + DashboardSerializer.emitChildWidget(ch{ci}, groupCount); + lines = [lines, childLines]; + lines{end+1} = sprintf(' %s.addChild(%s);', groupVarName, childVar); + end + end + else + % Delegate all non-group widgets to the complete per-type emitter + % so number/status/gauge ValueFcn/StaticValue/StatusFcn bindings + % (and every other widget type) survive .m export — exactly as + % exportScript already does. The old open-coded switch here was a + % lossy duplicate that dropped those bindings. + wLines = DashboardSerializer.linesForWidget(ws, pos, ' '); + lines = [lines, wLines]; %#ok end lines{end+1} = ''; end diff --git a/libs/Dashboard/HeatmapWidget.m b/libs/Dashboard/HeatmapWidget.m index e70c0327..c6fbb2a6 100644 --- a/libs/Dashboard/HeatmapWidget.m +++ b/libs/Dashboard/HeatmapWidget.m @@ -135,6 +135,17 @@ function refresh(obj) if isfield(s, 'showColorbar'), obj.ShowColorbar = s.showColorbar; end if isfield(s, 'xLabels'), obj.XLabels = s.xLabels; end if isfield(s, 'yLabels'), obj.YLabels = s.yLabels; end + % Restore the data binding (callback) — dropped before P0-3. + if isfield(s, 'source') && isfield(s.source, 'type') + switch s.source.type + case 'callback' + obj.DataFcn = str2func(s.source.function); + otherwise + warning('HeatmapWidget:sourceUnresolved', ... + 'Unresolved source type ''%s'' for Heatmap ''%s''.', ... + s.source.type, obj.Title); + end + end end end end diff --git a/libs/Dashboard/HistogramWidget.m b/libs/Dashboard/HistogramWidget.m index 4f024c5e..47704a33 100644 --- a/libs/Dashboard/HistogramWidget.m +++ b/libs/Dashboard/HistogramWidget.m @@ -142,6 +142,17 @@ function refresh(obj) if isfield(s, 'numBins'), obj.NumBins = s.numBins; end if isfield(s, 'showNormalFit'), obj.ShowNormalFit = s.showNormalFit; end if isfield(s, 'edgeColor'), obj.EdgeColor = s.edgeColor; end + % Restore the data binding (callback) — dropped before P0-3. + if isfield(s, 'source') && isfield(s.source, 'type') + switch s.source.type + case 'callback' + obj.DataFcn = str2func(s.source.function); + otherwise + warning('HistogramWidget:sourceUnresolved', ... + 'Unresolved source type ''%s'' for Histogram ''%s''.', ... + s.source.type, obj.Title); + end + end end end end diff --git a/libs/Dashboard/ScatterWidget.m b/libs/Dashboard/ScatterWidget.m index 0947999e..8fc1e6c3 100644 --- a/libs/Dashboard/ScatterWidget.m +++ b/libs/Dashboard/ScatterWidget.m @@ -187,6 +187,29 @@ function refresh(obj) end if isfield(s, 'markerSize'), obj.MarkerSize = s.markerSize; end if isfield(s, 'colormap'), obj.Colormap = s.colormap; end + % Restore the dual-sensor bindings via TagRegistry — dropped before P0-3. + % Old structs without these keys load with no binding and no warning. + if isfield(s, 'sensorX'), obj.SensorX = ScatterWidget.resolveTag_(s.sensorX, obj.Title); end + if isfield(s, 'sensorY'), obj.SensorY = ScatterWidget.resolveTag_(s.sensorY, obj.Title); end + if isfield(s, 'sensorColor'), obj.SensorColor = ScatterWidget.resolveTag_(s.sensorColor, obj.Title); end + end + + function t = resolveTag_(key, title) + %RESOLVETAG_ Resolve a Tag key via TagRegistry; warn (no throw) if absent. + % Mirrors FastSenseWidget:tagNotFound semantics: a missing key yields [] + % and a namespaced warning rather than an error, so a dashboard saved + % against a different registry still loads. + t = []; + if isempty(key), return; end + if exist('TagRegistry', 'class') + try + t = TagRegistry.get(key); + catch + t = []; + warning('ScatterWidget:sourceUnresolved', ... + 'Unresolved sensor ''%s'' for Scatter ''%s''.', key, title); + end + end end end end diff --git a/tests/suite/TestDashboardSerializerRoundTrip.m b/tests/suite/TestDashboardSerializerRoundTrip.m index c251ed9a..3a5791c1 100644 --- a/tests/suite/TestDashboardSerializerRoundTrip.m +++ b/tests/suite/TestDashboardSerializerRoundTrip.m @@ -189,5 +189,60 @@ function testRoundTripPreservesWidgetSpecificProperties(testCase) testCase.verifyEqual(etw.Events(2).label, 'B', ... 'EventTimelineWidget second event label should be B'); end + + function testChartCallbackRoundTrip(testCase) + %TESTCHARTCALLBACKROUNDTRIP P0-3: a callback chart widget's DataFcn + % must survive toStruct/fromStruct (was silently dropped before). + w = BarChartWidget('Title', 'Bars'); + w.DataFcn = @() [1 2 3]; + s = w.toStruct(); + w2 = BarChartWidget.fromStruct(s); + testCase.verifyNotEmpty(w2.DataFcn, 'DataFcn must be restored from s.source'); + testCase.verifyEqual(func2str(w2.DataFcn), func2str(w.DataFcn)); + end + + function testScatterSensorRoundTrip(testCase) + %TESTSCATTERSENSORROUNDTRIP P0-3: Scatter SensorX/SensorY must be + % restored from TagRegistry on load (was silently dropped before). + TagRegistry.clear(); + testCase.addTeardown(@() TagRegistry.clear()); + tx = SensorTag('sx', 'X', 1:5, 'Y', 1:5); + ty = SensorTag('sy', 'X', 1:5, 'Y', 2:6); + TagRegistry.register('sx', tx); + TagRegistry.register('sy', ty); + w = ScatterWidget('Title', 'Sc'); + w.SensorX = tx; + w.SensorY = ty; + s = w.toStruct(); + w2 = ScatterWidget.fromStruct(s); + testCase.verifyNotEmpty(w2.SensorX, 'SensorX must be restored'); + testCase.verifyNotEmpty(w2.SensorY, 'SensorY must be restored'); + testCase.verifyEqual(w2.SensorX.Key, 'sx'); + end + + function testChartOldStructNoSourceLoadsQuietly(testCase) + %TESTCHARTOLDSTRUCTNOSOURCELOADSQUIETLY P0-3 backward-compat: an old + % chart struct WITHOUT s.source must load with no binding and NO warning. + s = struct('type', 'barchart', 'title', 'B', ... + 'position', struct('col', 1, 'row', 1, 'width', 8, 'height', 4)); + lastwarn(''); + w2 = BarChartWidget.fromStruct(s); + [~, wid] = lastwarn(); + testCase.verifyEmpty(wid, 'Old struct without source must not emit a warning'); + testCase.verifyEmpty(w2.DataFcn); + end + + function testSaveEmitsNumberBinding(testCase) + %TESTSAVEEMITSNUMBERBINDING P0-3: DashboardSerializer.save (.m export) + % must emit the ValueFcn binding (it was dropped before the fix). + d = DashboardEngine('rt'); + d.addWidget('number', 'Title', 'Speed', 'Position', [1 1 4 2], ... + 'ValueFcn', @() 42); + filepath = fullfile(testCase.TempDir, 'rtbind.m'); + d.save(filepath); + txt = fileread(filepath); + testCase.verifyTrue(contains(txt, 'ValueFcn'), ... + 'save() must emit the number ValueFcn binding'); + end end end From 3a98d54999b574b222c55ba8adf005629be66b15 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:23:12 +0200 Subject: [PATCH 04/14] fix(api): strict unknown-option guards on FastSense + DashboardWidget constructors (P0-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The house convention is that unknown name-value keys throw immediately listing valid options, but the two biggest entry points violated it: - FastSense's constructor called parseOpts WITHOUT the verbose flag and discarded the unmatched output, so FastSense('Themee','dark') silently built a fully defaulted object — the hardest class of config bug to diagnose. It now captures the unmatched struct and throws FastSense:unknownOption (the closed ctor option set makes strict rejection safe; addLine/addBand etc. keep their line() pass-through). - DashboardWidget's base constructor (shared by all ~20 widgets) accepted unknown keys, surfacing only MATLAB's generic 'no public property' error. It now throws DashboardWidget:unknownOption listing valid options. The legacy 'Sensor'->'Tag' remap runs first, so old scripts and serialized dashboards still construct. Adds throw + good-option + legacy-alias tests. Verified the base-ctor guard breaks no legitimate widget construction across all widget suites; the 14 unrelated failures in that sweep (companion-uifigure / timer-recovery env tests) are pre-existing and reproduce with this change reverted. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit a85c06b23bc5bad141bcd5adb48eb5192e39b4cf) --- libs/Dashboard/DashboardWidget.m | 11 ++++++++++- libs/FastSense/FastSense.m | 10 +++++++++- tests/suite/TestDashboardWidget.m | 26 ++++++++++++++++++++++++++ tests/suite/TestFastSenseWidget.m | 19 +++++++++++++++++++ 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/libs/Dashboard/DashboardWidget.m b/libs/Dashboard/DashboardWidget.m index 999379a5..89a617de 100644 --- a/libs/Dashboard/DashboardWidget.m +++ b/libs/Dashboard/DashboardWidget.m @@ -57,7 +57,16 @@ end end for k = 1:2:numel(varargin) - obj.(varargin{k}) = varargin{k+1}; + key = varargin{k}; + % Reject unknown/misspelled options with a namespaced error + % (house convention). The 'Sensor'->'Tag' remap above runs first, + % so legacy scripts and old serialized dashboards still work. + if ~isprop(obj, key) + error('DashboardWidget:unknownOption', ... + 'Unknown option ''%s''. Valid options: %s', ... + key, strjoin(properties(obj), ', ')); + end + obj.(key) = varargin{k+1}; end % Title cascade from Tag. if isempty(obj.Title) && ~isempty(obj.Tag) diff --git a/libs/FastSense/FastSense.m b/libs/FastSense/FastSense.m index cf01cf09..e56d82cb 100644 --- a/libs/FastSense/FastSense.m +++ b/libs/FastSense/FastSense.m @@ -236,7 +236,15 @@ defaults.StorageMode = cfg.StorageMode; defaults.MemoryLimit = cfg.MemoryLimit; defaults.HoverCrosshair = true; - [opts, ~] = parseOpts(defaults, varargin); + [opts, unmatched] = parseOpts(defaults, varargin); + % The constructor has a closed option set, so reject unknown/misspelled + % keys immediately rather than silently discarding them (house convention). + extraKeys = fieldnames(unmatched); + if ~isempty(extraKeys) + error('FastSense:unknownOption', ... + 'Unknown option ''%s''. Valid options: %s', ... + extraKeys{1}, strjoin(fieldnames(defaults), ', ')); + end obj.ParentAxes = opts.Parent; obj.LinkGroup = opts.LinkGroup; diff --git a/tests/suite/TestDashboardWidget.m b/tests/suite/TestDashboardWidget.m index 53aeeff0..490166ec 100644 --- a/tests/suite/TestDashboardWidget.m +++ b/tests/suite/TestDashboardWidget.m @@ -129,5 +129,31 @@ function testClearPanelControlsHandlesInvalidHandle(testCase) MockDashboardWidget.invokeClearPanelControls(fakeHandle); testCase.verifyTrue(true, 'no-op path completed without error'); end + + function testWidgetUnknownOptionThrows(testCase) + %TESTWIDGETUNKNOWNOPTIONTHROWS P0-4: the base ctor rejects unknown options + % with a namespaced id (was a generic MATLAB 'no public property' error). + threw = false; id = ''; + try + NumberWidget('Bogus', 1); + catch err + threw = true; id = err.identifier; + end + testCase.verifyTrue(threw); + testCase.verifyEqual(id, 'DashboardWidget:unknownOption'); + end + + function testWidgetSensorAliasStillWorks(testCase) + %TESTWIDGETSENSORALIASSTILLWORKS P0-4 backward-compat: legacy 'Sensor' maps to Tag. + t = SensorTag('s', 'X', 1:5, 'Y', 1:5); + threw = false; + try + w = NumberWidget('Sensor', t); % legacy alias -> Tag, must NOT throw + catch + threw = true; + end + testCase.verifyFalse(threw, 'legacy Sensor alias must not throw'); + testCase.verifyTrue(~isempty(w.Tag) && w.Tag == t); + end end end diff --git a/tests/suite/TestFastSenseWidget.m b/tests/suite/TestFastSenseWidget.m index 7adf3e50..953907cc 100644 --- a/tests/suite/TestFastSenseWidget.m +++ b/tests/suite/TestFastSenseWidget.m @@ -138,5 +138,24 @@ function testYLimitsAppliedAfterRender(testCase) actualYLim = ylim(ax(1)); testCase.verifyEqual(actualYLim, [0 100], 'AbsTol', 1e-10); end + + function testFastSenseUnknownOptionThrows(testCase) + %TESTFASTSENSEUNKNOWNOPTIONTHROWS P0-4: FastSense ctor rejects unknown options + % instead of silently discarding the typo into the unmatched struct. + threw = false; id = ''; + try + FastSense('Themee', 'dark'); + catch err + threw = true; id = err.identifier; + end + testCase.verifyTrue(threw); + testCase.verifyEqual(id, 'FastSense:unknownOption'); + end + + function testFastSenseGoodOptionConstructs(testCase) + %TESTFASTSENSEGOODOPTIONCONSTRUCTS P0-4: a valid option still constructs fine. + f = FastSense('Theme', 'dark'); + testCase.verifyClass(f, 'FastSense'); + end end end From 26add6276fb30b33363c525f979c7f7342dc3db0 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:40:50 +0200 Subject: [PATCH 05/14] feat(dashboard): add DashboardWidgetRegistry as single source of truth for widget types (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The widget type->class mapping was hand-maintained in four out-of-sync places (DashboardEngine.WidgetTypeMap_, widgetTypes(), DashboardSerializer .createWidgetFromStruct, DetachedMirror.cloneWidget) — adding a widget required editing all of them or it silently failed to deserialize/detach/list. Introduces DashboardWidgetRegistry, a static-method class mirroring TagRegistry (persistent containers.Map, namespaced error ids, reset() for test isolation), seeded with the 19 built-in types and the 'kpi'->'number' alias. API: types/isRegistered/resolveAlias/constructorFor/fromStruct/register/registerAlias/reset. The next commits route the four drift sites through it. This commit only adds the registry + its tests; no consumer wired yet, so behaviour is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 383f2e65e7bd6eb48cd24bcebbf61433c734070f) --- libs/Dashboard/DashboardWidgetRegistry.m | 204 ++++++++++++++++++++++ tests/suite/TestDashboardWidgetRegistry.m | 113 ++++++++++++ 2 files changed, 317 insertions(+) create mode 100644 libs/Dashboard/DashboardWidgetRegistry.m create mode 100644 tests/suite/TestDashboardWidgetRegistry.m diff --git a/libs/Dashboard/DashboardWidgetRegistry.m b/libs/Dashboard/DashboardWidgetRegistry.m new file mode 100644 index 00000000..9cb84a4a --- /dev/null +++ b/libs/Dashboard/DashboardWidgetRegistry.m @@ -0,0 +1,204 @@ +classdef DashboardWidgetRegistry + %DASHBOARDWIDGETREGISTRY Single source of truth for dashboard widget types. + % DashboardWidgetRegistry maps a widget type string (e.g. 'number') to the + % class that implements it, so that EVERY consumer — DashboardEngine.addWidget, + % DashboardEngine.widgetTypes, DashboardSerializer.createWidgetFromStruct and + % DetachedMirror.cloneWidget — dispatches through ONE table instead of four + % hand-maintained switch statements that drift out of sync. + % + % It mirrors the TagRegistry static-singleton pattern (a classdef of static + % methods over a persistent containers.Map), with three intentional deltas: + % + % 1. The catalog is seeded NON-empty on first use with the built-in widget + % types (TagRegistry starts empty). + % 2. Type ALIASES (deprecated/renamed type strings) are a separate concern, + % resolved via resolveAlias() — e.g. the deprecated 'kpi' -> 'number'. + % 3. reset() RE-SEEDS the built-ins and built-in aliases rather than wiping + % to empty; it exists for test isolation after register()/registerAlias(). + % + % DashboardWidgetRegistry Methods (Static, public): + % types — sorted cellstr of all registered canonical type strings + % isRegistered — true if a canonical type is registered (aliases excluded) + % resolveAlias — map an alias to its canonical type (passthrough otherwise) + % constructorFor — the @ClassName constructor handle for a type (resolves alias) + % fromStruct — deserialize a widget struct via the type's static fromStruct + % register — add a NEW canonical type (hard error on collision) + % registerAlias — add an alias for an already-registered canonical type + % reset — restore the built-in catalog + aliases (test isolation) + % + % Example: + % % Register a custom widget so it works through addWidget, serialization, + % % and detach with no core edits: + % DashboardWidgetRegistry.register('mywidget', @MyWidget); + % w = DashboardWidgetRegistry.fromStruct('mywidget', s); + % + % See also DashboardWidget, DashboardEngine, DashboardSerializer, + % DetachedMirror, TagRegistry. + + methods (Static) + + function t = types() + %TYPES Sorted cellstr of all registered canonical widget type strings. + % The single source of truth — DashboardEngine.widgetTypes() derives + % its list from this. + t = sort(DashboardWidgetRegistry.registry().keys()); + end + + function tf = isRegistered(type) + %ISREGISTERED True if TYPE is a registered canonical type. + % Aliases (e.g. 'kpi') are NOT counted — use resolveAlias() first if + % you need alias-aware membership. + tf = DashboardWidgetRegistry.registry().isKey(type); + end + + function c = resolveAlias(type) + %RESOLVEALIAS Map an alias to its canonical type. + % Returns TYPE unchanged when it is not an alias. + a = DashboardWidgetRegistry.aliases(); + if a.isKey(type) + c = a(type); + else + c = type; + end + end + + function h = constructorFor(type) + %CONSTRUCTORFOR Constructor handle (@ClassName) for a widget type. + % Resolves aliases first. Throws DashboardWidgetRegistry:unknownType + % when the (resolved) type is not registered. + canonical = DashboardWidgetRegistry.resolveAlias(type); + map = DashboardWidgetRegistry.registry(); + if ~map.isKey(canonical) + error('DashboardWidgetRegistry:unknownType', ... + 'Unknown widget type ''%s''. Use DashboardWidgetRegistry.types() to list registered types.', ... + type); + end + h = map(canonical); + end + + function w = fromStruct(type, s) + %FROMSTRUCT Deserialize a widget struct via its class fromStruct. + % w = DashboardWidgetRegistry.fromStruct(type, s) resolves TYPE to a + % constructor handle, derives the class name, and calls + % .fromStruct(s). Throws DashboardWidgetRegistry:unknownType + % when TYPE (resolved) is not registered. + h = DashboardWidgetRegistry.constructorFor(type); + className = func2str(h); + % Named function handles stringify without a leading '@'; strip one + % defensively in case a user registers an anonymous handle. + if ~isempty(className) && className(1) == '@' + className = className(2:end); + end + w = feval([className '.fromStruct'], s); + end + + function register(type, ctorHandle) + %REGISTER Add a NEW canonical widget type to the catalog. + % DashboardWidgetRegistry.register(type, @ClassName) registers a + % constructor handle under TYPE. Like TagRegistry.register, this + % HARD-ERRORS on collision (DashboardWidgetRegistry:duplicateType) so + % a custom widget cannot silently clobber a built-in. Call reset() + % to drop custom registrations (test isolation). + % + % Errors: + % DashboardWidgetRegistry:invalidType — ctorHandle is not a handle + % DashboardWidgetRegistry:duplicateType — type already registered + if ~isa(ctorHandle, 'function_handle') + error('DashboardWidgetRegistry:invalidType', ... + 'Constructor must be a function_handle, got %s.', class(ctorHandle)); + end + map = DashboardWidgetRegistry.registry(); + if map.isKey(type) + error('DashboardWidgetRegistry:duplicateType', ... + 'Widget type ''%s'' is already registered. Choose a different type string.', type); + end + map(type) = ctorHandle; + end + + function registerAlias(alias, canonical) + %REGISTERALIAS Map an alias type string to a registered canonical type. + % The canonical type must already be registered, else + % DashboardWidgetRegistry:unknownType is thrown. + map = DashboardWidgetRegistry.registry(); + if ~map.isKey(canonical) + error('DashboardWidgetRegistry:unknownType', ... + 'Cannot alias to unregistered type ''%s''.', canonical); + end + a = DashboardWidgetRegistry.aliases(); + a(alias) = canonical; + end + + function reset() + %RESET Restore the built-in catalog and aliases (test isolation). + % Re-seeds the persistent maps in place so register()/registerAlias() + % side effects from a prior test do not leak. Mirrors TagRegistry.clear, + % but re-seeds the built-ins rather than wiping to empty. + map = DashboardWidgetRegistry.registry(); + k = map.keys(); + for i = 1:numel(k) + map.remove(k{i}); + end + DashboardWidgetRegistry.seedBuiltins_(map); + + a = DashboardWidgetRegistry.aliases(); + ka = a.keys(); + for i = 1:numel(ka) + a.remove(ka{i}); + end + DashboardWidgetRegistry.seedAliases_(a); + end + + end + + methods (Static, Access = private) + + function map = registry() + %REGISTRY Persistent type->constructor catalog (seeded on first use). + persistent cache; + if isempty(cache) + cache = containers.Map('KeyType', 'char', 'ValueType', 'any'); + DashboardWidgetRegistry.seedBuiltins_(cache); + end + map = cache; + end + + function map = aliases() + %ALIASES Persistent alias->canonical map (seeded on first use). + persistent cache; + if isempty(cache) + cache = containers.Map('KeyType', 'char', 'ValueType', 'any'); + DashboardWidgetRegistry.seedAliases_(cache); + end + map = cache; + end + + function seedBuiltins_(map) + %SEEDBUILTINS_ Populate MAP with the built-in widget type->constructor entries. + map('fastsense') = @FastSenseWidget; + map('number') = @NumberWidget; + map('status') = @StatusWidget; + map('text') = @TextWidget; + map('gauge') = @GaugeWidget; + map('table') = @TableWidget; + map('rawaxes') = @RawAxesWidget; + map('timeline') = @EventTimelineWidget; + map('group') = @GroupWidget; + map('heatmap') = @HeatmapWidget; + map('barchart') = @BarChartWidget; + map('histogram') = @HistogramWidget; + map('scatter') = @ScatterWidget; + map('image') = @ImageWidget; + map('multistatus') = @MultiStatusWidget; + map('divider') = @DividerWidget; + map('iconcard') = @IconCardWidget; + map('chipbar') = @ChipBarWidget; + map('sparkline') = @SparklineCardWidget; + end + + function seedAliases_(map) + %SEEDALIASES_ Populate MAP with the built-in deprecated-type aliases. + map('kpi') = 'number'; % 'kpi' was renamed to 'number' + end + + end +end diff --git a/tests/suite/TestDashboardWidgetRegistry.m b/tests/suite/TestDashboardWidgetRegistry.m new file mode 100644 index 00000000..54b751ab --- /dev/null +++ b/tests/suite/TestDashboardWidgetRegistry.m @@ -0,0 +1,113 @@ +classdef TestDashboardWidgetRegistry < matlab.unittest.TestCase +%TESTDASHBOARDWIDGETREGISTRY Tests for the single-source-of-truth widget registry. + + methods (TestClassSetup) + function addPaths(testCase) + addpath(fullfile(fileparts(mfilename('fullpath')), '..', '..')); + install(); + end + end + + methods (TestMethodTeardown) + function resetRegistry(~) + % Persistent map must not leak between tests. + DashboardWidgetRegistry.reset(); + end + end + + methods (Test) + function testTypesHas19IncludingDrifted(testCase) + t = DashboardWidgetRegistry.types(); + testCase.verifyClass(t, 'cell'); + testCase.verifyEqual(numel(t), 19); + % The three types that were missing from the old widgetTypes() list. + testCase.verifyTrue(ismember('iconcard', t)); + testCase.verifyTrue(ismember('chipbar', t)); + testCase.verifyTrue(ismember('sparkline', t)); + % Sorted. + testCase.verifyEqual(t, sort(t)); + end + + function testIsRegistered(testCase) + testCase.verifyTrue(DashboardWidgetRegistry.isRegistered('number')); + testCase.verifyTrue(DashboardWidgetRegistry.isRegistered('sparkline')); + testCase.verifyFalse(DashboardWidgetRegistry.isRegistered('nope')); + % An alias is NOT a registered canonical type. + testCase.verifyFalse(DashboardWidgetRegistry.isRegistered('kpi')); + end + + function testResolveAlias(testCase) + testCase.verifyEqual(DashboardWidgetRegistry.resolveAlias('kpi'), 'number'); + testCase.verifyEqual(DashboardWidgetRegistry.resolveAlias('number'), 'number'); + testCase.verifyEqual(DashboardWidgetRegistry.resolveAlias('unknownxyz'), 'unknownxyz'); + end + + function testConstructorFor(testCase) + h = DashboardWidgetRegistry.constructorFor('number'); + testCase.verifyClass(h, 'function_handle'); + testCase.verifyEqual(func2str(h), 'NumberWidget'); + % Alias resolves to the canonical constructor. + hk = DashboardWidgetRegistry.constructorFor('kpi'); + testCase.verifyEqual(func2str(hk), 'NumberWidget'); + end + + function testConstructorForUnknownErrors(testCase) + testCase.verifyError(@() DashboardWidgetRegistry.constructorFor('nope'), ... + 'DashboardWidgetRegistry:unknownType'); + end + + function testRegisterCustomType(testCase) + DashboardWidgetRegistry.register('mytype', @NumberWidget); + testCase.verifyTrue(DashboardWidgetRegistry.isRegistered('mytype')); + testCase.verifyEqual(func2str(DashboardWidgetRegistry.constructorFor('mytype')), 'NumberWidget'); + testCase.verifyTrue(ismember('mytype', DashboardWidgetRegistry.types())); + end + + function testDuplicateRegisterErrors(testCase) + testCase.verifyError(@() DashboardWidgetRegistry.register('number', @NumberWidget), ... + 'DashboardWidgetRegistry:duplicateType'); + end + + function testRegisterNonHandleErrors(testCase) + testCase.verifyError(@() DashboardWidgetRegistry.register('bad', 42), ... + 'DashboardWidgetRegistry:invalidType'); + end + + function testRegisterAlias(testCase) + DashboardWidgetRegistry.registerAlias('kp2', 'number'); + testCase.verifyEqual(DashboardWidgetRegistry.resolveAlias('kp2'), 'number'); + end + + function testRegisterAliasToUnknownErrors(testCase) + testCase.verifyError(@() DashboardWidgetRegistry.registerAlias('x', 'notRegistered'), ... + 'DashboardWidgetRegistry:unknownType'); + end + + function testFromStruct(testCase) + base = NumberWidget('Title', 'N', 'Position', [1 1 4 2], 'StaticValue', 5); + s = base.toStruct(); + w = DashboardWidgetRegistry.fromStruct('number', s); + testCase.verifyClass(w, 'NumberWidget'); + % Alias path. + wk = DashboardWidgetRegistry.fromStruct('kpi', s); + testCase.verifyClass(wk, 'NumberWidget'); + end + + function testFromStructUnknownErrors(testCase) + s = struct('type', 'nope', 'title', 'X', ... + 'position', struct('col', 1, 'row', 1, 'width', 4, 'height', 2)); + testCase.verifyError(@() DashboardWidgetRegistry.fromStruct('nope', s), ... + 'DashboardWidgetRegistry:unknownType'); + end + + function testReset(testCase) + DashboardWidgetRegistry.register('mytype', @NumberWidget); + testCase.verifyTrue(DashboardWidgetRegistry.isRegistered('mytype')); + DashboardWidgetRegistry.reset(); + testCase.verifyEqual(numel(DashboardWidgetRegistry.types()), 19); + testCase.verifyFalse(DashboardWidgetRegistry.isRegistered('mytype')); + % Built-in alias survives the reset. + testCase.verifyEqual(DashboardWidgetRegistry.resolveAlias('kpi'), 'number'); + end + end +end From e2a7ba4aa0e4927b8c39ce0fc0e83a68d5735a0c Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:45:38 +0200 Subject: [PATCH 06/14] refactor(dashboard): route DashboardEngine widget dispatch through the registry (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit addWidget now dispatches via DashboardWidgetRegistry.constructorFor instead of the instance-level WidgetTypeMap_ (removed), and widgetTypes() derives its list from DashboardWidgetRegistry.types() — so it can no longer drift from what addWidget accepts (it now correctly includes iconcard/chipbar/sparkline, which the stale 16-entry static list omitted). Adds the documented extension point DashboardEngine.registerWidgetType(type, ctor). The kpi deprecation warning, timeline no-store warning, GroupWidget ReflowCallback injection, multi-page routing, overlap resolution and pre-constructed-widget passthrough are all preserved unchanged. New TestDashboardEngineWidgetTypes (figure-free): widgetTypes() includes the drifted types, kpi still warns + returns NumberWidget, registerWidgetType makes a custom type work through addWidget, unknown type still errors DashboardEngine:unknownType. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 35b6cda64fda926f4e7d12fa23d49262edb4b8e3) --- libs/Dashboard/DashboardEngine.m | 89 ++++++++++++-------- tests/suite/TestDashboardEngineWidgetTypes.m | 62 ++++++++++++++ 2 files changed, 117 insertions(+), 34 deletions(-) create mode 100644 tests/suite/TestDashboardEngineWidgetTypes.m diff --git a/libs/Dashboard/DashboardEngine.m b/libs/Dashboard/DashboardEngine.m index 186c3fa4..e3cce331 100644 --- a/libs/Dashboard/DashboardEngine.m +++ b/libs/Dashboard/DashboardEngine.m @@ -63,8 +63,6 @@ % Theme caching ThemeCache_ = [] % Cached DashboardTheme struct; lazy-computed by getCachedTheme() ThemeCachePreset_ = '' % Theme preset string that ThemeCache_ was built for - % Widget dispatch table - WidgetTypeMap_ = [] % containers.Map: type string -> constructor function handle % Time control TimePanelHeight = 0.085 % bumped from 0.06 to fit data-range labels below slider (260512-hrn-followup) DataTimeRange = [0 1] % [tMin tMax] across all widget data @@ -162,17 +160,8 @@ end obj.Layout = DashboardLayout(); obj.Layout.EngineRef = obj; % Phase 1032 PLOG-VIZ-05 — used by addPlantLogToggle callback - obj.WidgetTypeMap_ = containers.Map({ ... - 'fastsense', 'number', 'status', 'text', ... - 'gauge', 'table', 'rawaxes', 'timeline', ... - 'group', 'heatmap', 'barchart', 'histogram', ... - 'scatter', 'image', 'multistatus', 'divider', ... - 'iconcard', 'chipbar', 'sparkline'}, ... - {@FastSenseWidget, @NumberWidget, @StatusWidget, @TextWidget, ... - @GaugeWidget, @TableWidget, @RawAxesWidget, @EventTimelineWidget, ... - @GroupWidget, @HeatmapWidget, @BarChartWidget, @HistogramWidget, ... - @ScatterWidget, @ImageWidget, @MultiStatusWidget, @DividerWidget, ... - @IconCardWidget, @ChipBarWidget, @SparklineCardWidget}); + % Widget type->constructor dispatch now lives in DashboardWidgetRegistry + % (the single source of truth shared with the serializer + detach paths). end function pg = addPage(obj, name) @@ -385,8 +374,8 @@ function switchPage(obj, pageIdx) type = 'number'; end - if isKey(obj.WidgetTypeMap_, type) - ctor = obj.WidgetTypeMap_(type); + if DashboardWidgetRegistry.isRegistered(DashboardWidgetRegistry.resolveAlias(type)) + ctor = DashboardWidgetRegistry.constructorFor(type); w = ctor(varargin{:}); else error('DashboardEngine:unknownType', ... @@ -4328,25 +4317,57 @@ function onFigureDestroyed_(obj) methods (Static) function types = widgetTypes() - %WIDGETTYPES List supported widget type strings. - types = { - 'fastsense', 'Time-series plot (FastSenseWidget)' - 'number', 'Single numeric value with trend (NumberWidget)' - 'status', 'Status indicator with dot and label (StatusWidget)' - 'gauge', 'Gauge display in arc/donut/bar/thermometer style (GaugeWidget)' - 'table', 'Data table from sensor (TableWidget)' - 'text', 'Static text block (TextWidget)' - 'timeline', 'Event timeline display (EventTimelineWidget)' - 'rawaxes', 'Raw MATLAB axes for custom plotting (RawAxesWidget)' - 'group', 'Widget container with panel/collapsible/tabbed modes (GroupWidget)' - 'heatmap', 'Heatmap color grid (HeatmapWidget)' - 'barchart', 'Bar chart for categories (BarChartWidget)' - 'histogram', 'Value distribution histogram (HistogramWidget)' - 'scatter', 'X vs Y scatter plot (ScatterWidget)' - 'image', 'Static image display (ImageWidget)' - 'multistatus', 'Multi-sensor status grid (MultiStatusWidget)' - 'divider', 'Horizontal divider line (DividerWidget)' - }; + %WIDGETTYPES Supported widget types + descriptions, as an Nx2 cell. + % Derived from DashboardWidgetRegistry.types() (the single source of + % truth), so it can no longer drift from what addWidget accepts, and + % user-registered types (registerWidgetType) appear automatically with + % a generic description. + descMap = containers.Map( ... + {'fastsense', 'number', 'status', 'gauge', 'table', 'text', ... + 'timeline', 'rawaxes', 'group', 'heatmap', 'barchart', ... + 'histogram', 'scatter', 'image', 'multistatus', 'divider', ... + 'iconcard', 'chipbar', 'sparkline'}, ... + {'Time-series plot (FastSenseWidget)', ... + 'Single numeric value with trend (NumberWidget)', ... + 'Status indicator with dot and label (StatusWidget)', ... + 'Gauge display in arc/donut/bar/thermometer style (GaugeWidget)', ... + 'Data table from sensor (TableWidget)', ... + 'Static text block (TextWidget)', ... + 'Event timeline display (EventTimelineWidget)', ... + 'Raw MATLAB axes for custom plotting (RawAxesWidget)', ... + 'Widget container with panel/collapsible/tabbed modes (GroupWidget)', ... + 'Heatmap color grid (HeatmapWidget)', ... + 'Bar chart for categories (BarChartWidget)', ... + 'Value distribution histogram (HistogramWidget)', ... + 'X vs Y scatter plot (ScatterWidget)', ... + 'Static image display (ImageWidget)', ... + 'Multi-sensor status grid (MultiStatusWidget)', ... + 'Horizontal divider line (DividerWidget)', ... + 'Icon + value card (IconCardWidget)', ... + 'Chip/badge status bar (ChipBarWidget)', ... + 'Sparkline value card (SparklineCardWidget)'}); + names = DashboardWidgetRegistry.types(); + types = cell(numel(names), 2); + for i = 1:numel(names) + types{i, 1} = names{i}; + if descMap.isKey(names{i}) + types{i, 2} = descMap(names{i}); + else + types{i, 2} = sprintf('%s widget', names{i}); + end + end + end + + function registerWidgetType(type, ctorHandle) + %REGISTERWIDGETTYPE Register a custom widget type with the dashboard. + % DashboardEngine.registerWidgetType('mytype', @MyWidget) makes + % 'mytype' usable through addWidget, serialization, and detach — the + % documented extension point for third-party widgets. The widget class + % must subclass DashboardWidget and provide a static fromStruct. + % Errors DashboardWidgetRegistry:duplicateType on a name collision. + % + % See also DashboardWidgetRegistry.register, DashboardEngine.widgetTypes. + DashboardWidgetRegistry.register(type, ctorHandle); end function obj = load(filepath, varargin) diff --git a/tests/suite/TestDashboardEngineWidgetTypes.m b/tests/suite/TestDashboardEngineWidgetTypes.m new file mode 100644 index 00000000..9538db65 --- /dev/null +++ b/tests/suite/TestDashboardEngineWidgetTypes.m @@ -0,0 +1,62 @@ +classdef TestDashboardEngineWidgetTypes < matlab.unittest.TestCase +%TESTDASHBOARDENGINEWIDGETTYPES Engine widget-type dispatch through the registry. +% Figure-free: exercises the static widgetTypes()/registerWidgetType API and +% addWidget dispatch (no render), so it is fast and headless-safe. + + methods (TestClassSetup) + function addPaths(testCase) + addpath(fullfile(fileparts(mfilename('fullpath')), '..', '..')); + install(); + end + end + + methods (TestMethodTeardown) + function resetRegistry(~) + DashboardWidgetRegistry.reset(); + end + end + + methods (Test) + function testWidgetTypesIncludesDriftedTypes(testCase) + % The stale static list was missing these three (the drift bug). + t = DashboardEngine.widgetTypes(); + names = t(:, 1); + testCase.verifyTrue(ismember('iconcard', names)); + testCase.verifyTrue(ismember('chipbar', names)); + testCase.verifyTrue(ismember('sparkline', names)); + % Now derived from the registry => 19 entries. + testCase.verifyEqual(size(t, 1), 19); + end + + function testAddWidgetDispatchesViaRegistry(testCase) + d = DashboardEngine('T'); + w = d.addWidget('number', 'Title', 'N', 'Position', [1 1 4 2]); + testCase.verifyClass(w, 'NumberWidget'); + end + + function testKpiStillDeprecatedAndReturnsNumber(testCase) + d = DashboardEngine('T'); + lastwarn(''); + w = d.addWidget('kpi', 'Title', 'K', 'Position', [1 1 4 2]); + [~, id] = lastwarn(); + testCase.verifyEqual(id, 'DashboardEngine:deprecated'); + testCase.verifyClass(w, 'NumberWidget'); + end + + function testRegisterWidgetTypeThenAdd(testCase) + % The documented extension surface: register once, works everywhere. + DashboardEngine.registerWidgetType('mytype', @NumberWidget); + tt = DashboardEngine.widgetTypes(); + testCase.verifyTrue(ismember('mytype', tt(:, 1))); + d = DashboardEngine('T'); + w = d.addWidget('mytype', 'Title', 'X', 'Position', [1 1 4 2]); + testCase.verifyClass(w, 'NumberWidget'); + end + + function testUnknownTypeStillErrors(testCase) + d = DashboardEngine('T'); + testCase.verifyError(@() d.addWidget('definitelynotatype', 'Title', 'x'), ... + 'DashboardEngine:unknownType'); + end + end +end From 2cd26007bcbf95a7f3c92e0f269ca25ce33801b9 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:49:39 +0200 Subject: [PATCH 07/14] refactor(dashboard): route serializer + detach through the widget registry (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DashboardSerializer.createWidgetFromStruct (21-case switch) and DetachedMirror.cloneWidget (19-case switch) now both dispatch through DashboardWidgetRegistry.fromStruct — the last two drift sites are gone, so a registerWidgetType'd custom type now round-trips through save/load AND detach with no core edits. 'kpi' resolves to NumberWidget via the registry alias on the load path. Preserved exactly: the serializer's warn-and-skip for unknown types (DashboardSerializer:unknownType), the mirror's DetachedMirror:unknownType error, and the 'mock' test-widget special-case (kept serializer-only and guarded by exist() so the library keeps zero dependency on tests/). Tests: custom type deserializes via the registry (was dropped before); kpi struct loads as NumberWidget; unknown type still warns+returns []; a 'mock' widget still errors DetachedMirror:unknownType through the mirror. Existing all-types round-trip and detach suites stay green. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 0a38060378ff0038c87825cf94aa41aecd6b61d4) --- libs/Dashboard/DashboardSerializer.m | 65 ++++++------------- libs/Dashboard/DetachedMirror.m | 52 +++------------ tests/suite/TestDashboardDetach.m | 11 ++++ .../suite/TestDashboardSerializerRoundTrip.m | 33 ++++++++++ 4 files changed, 71 insertions(+), 90 deletions(-) diff --git a/libs/Dashboard/DashboardSerializer.m b/libs/Dashboard/DashboardSerializer.m index 8b951ae3..4aa6c48a 100644 --- a/libs/Dashboard/DashboardSerializer.m +++ b/libs/Dashboard/DashboardSerializer.m @@ -332,58 +332,31 @@ function saveJSON(config, filepath) function w = createWidgetFromStruct(ws) %CREATEWIDGETFROMSTRUCT Create a single widget from a struct. + % Dispatches through DashboardWidgetRegistry — the single source of + % truth for widget type->class. The deprecated 'kpi' resolves to + % NumberWidget via the registry alias. 'mock' is a test-only widget + % kept as a thin special-case here: MockDashboardWidget lives under + % tests/ and is intentionally NOT seeded into the library registry, + % so the library has no dependency on test code. Unknown types warn + % DashboardSerializer:unknownType and return []. w = []; - switch ws.type - case 'fastsense' - w = FastSenseWidget.fromStruct(ws); - case 'number' - w = NumberWidget.fromStruct(ws); - case 'kpi' - w = NumberWidget.fromStruct(ws); - case 'status' - w = StatusWidget.fromStruct(ws); - case 'text' - w = TextWidget.fromStruct(ws); - case 'gauge' - w = GaugeWidget.fromStruct(ws); - case 'table' - w = TableWidget.fromStruct(ws); - case 'rawaxes' - w = RawAxesWidget.fromStruct(ws); - case 'timeline' - w = EventTimelineWidget.fromStruct(ws); - case 'group' - w = GroupWidget.fromStruct(ws); - case 'heatmap' - w = HeatmapWidget.fromStruct(ws); - case 'barchart' - w = BarChartWidget.fromStruct(ws); - case 'histogram' - w = HistogramWidget.fromStruct(ws); - case 'scatter' - w = ScatterWidget.fromStruct(ws); - case 'image' - w = ImageWidget.fromStruct(ws); - case 'multistatus' - w = MultiStatusWidget.fromStruct(ws); - case 'divider' - w = DividerWidget.fromStruct(ws); - case 'iconcard' - w = IconCardWidget.fromStruct(ws); - case 'chipbar' - w = ChipBarWidget.fromStruct(ws); - case 'sparkline' - w = SparklineCardWidget.fromStruct(ws); - case 'mock' - % MockDashboardWidget used in tests — load via fromStruct if available + resolved = DashboardWidgetRegistry.resolveAlias(ws.type); + if strcmp(resolved, 'mock') + if exist('MockDashboardWidget', 'class') == 8 || ... + exist('MockDashboardWidget', 'file') == 2 try w = MockDashboardWidget.fromStruct(ws); catch w = []; end - otherwise - warning('DashboardSerializer:unknownType', ... - 'Unknown widget type: %s — skipping', ws.type); + end + return; + end + if DashboardWidgetRegistry.isRegistered(resolved) + w = DashboardWidgetRegistry.fromStruct(ws.type, ws); + else + warning('DashboardSerializer:unknownType', ... + 'Unknown widget type: %s — skipping', ws.type); end end diff --git a/libs/Dashboard/DetachedMirror.m b/libs/Dashboard/DetachedMirror.m index 4f816ea9..905382cb 100644 --- a/libs/Dashboard/DetachedMirror.m +++ b/libs/Dashboard/DetachedMirror.m @@ -131,8 +131,9 @@ function onFigureClose(obj) function w = cloneWidget(original) %CLONEWIDGET Clone a DashboardWidget via toStruct/fromStruct round-trip. % - % Dispatch is explicit to ensure all 15 widget types are handled. - % After fromStruct, live references lost by serialization are restored: + % Dispatches through DashboardWidgetRegistry (the single source of truth + % for widget type->class). After fromStruct, live references lost by + % serialization are restored: % - FastSenseWidget: Sensor reference + UseGlobalTime = false (DETACH-05) % - RawAxesWidget: PlotFcn and DataRangeFcn function handles @@ -143,48 +144,11 @@ function onFigureClose(obj) % restoreLiveRefs copies live Tag references directly afterward. s = DetachedMirror.stripSensorRefs(s); - switch s.type - case 'fastsense' - w = FastSenseWidget.fromStruct(s); - case 'number' - w = NumberWidget.fromStruct(s); - case 'status' - w = StatusWidget.fromStruct(s); - case 'text' - w = TextWidget.fromStruct(s); - case 'gauge' - w = GaugeWidget.fromStruct(s); - case 'table' - w = TableWidget.fromStruct(s); - case 'rawaxes' - w = RawAxesWidget.fromStruct(s); - case 'timeline' - w = EventTimelineWidget.fromStruct(s); - case 'group' - w = GroupWidget.fromStruct(s); - case 'heatmap' - w = HeatmapWidget.fromStruct(s); - case 'barchart' - w = BarChartWidget.fromStruct(s); - case 'histogram' - w = HistogramWidget.fromStruct(s); - case 'scatter' - w = ScatterWidget.fromStruct(s); - case 'image' - w = ImageWidget.fromStruct(s); - case 'multistatus' - w = MultiStatusWidget.fromStruct(s); - case 'divider' - w = DividerWidget.fromStruct(s); - case 'iconcard' - w = IconCardWidget.fromStruct(s); - case 'chipbar' - w = ChipBarWidget.fromStruct(s); - case 'sparkline' - w = SparklineCardWidget.fromStruct(s); - otherwise - error('DetachedMirror:unknownType', ... - 'Unknown widget type: %s', s.type); + if DashboardWidgetRegistry.isRegistered(DashboardWidgetRegistry.resolveAlias(s.type)) + w = DashboardWidgetRegistry.fromStruct(s.type, s); + else + error('DetachedMirror:unknownType', ... + 'Unknown widget type: %s', s.type); end % Restore live references lost during serialization round-trip diff --git a/tests/suite/TestDashboardDetach.m b/tests/suite/TestDashboardDetach.m index 3d59647f..4890a988 100644 --- a/tests/suite/TestDashboardDetach.m +++ b/tests/suite/TestDashboardDetach.m @@ -348,6 +348,17 @@ function testMirrorIsReadOnly(testCase) 'mirror.Widget handle must differ from original widget handle (not same reference)'); end + function testMirrorUnknownTypeErrors(testCase) + % P1: cloneWidget now dispatches through DashboardWidgetRegistry. A type not + % in the library registry — here 'mock', a test-only widget intentionally + % NOT seeded into the registry — must still error DetachedMirror:unknownType. + mockW = MockDashboardWidget('Title', 'M', 'Position', [1 1 6 2]); + themeStruct = DashboardTheme('light'); + noop = @() []; + testCase.verifyError(@() DetachedMirror(mockW, themeStruct, noop), ... + 'DetachedMirror:unknownType'); + end + end end diff --git a/tests/suite/TestDashboardSerializerRoundTrip.m b/tests/suite/TestDashboardSerializerRoundTrip.m index 3a5791c1..6c0b7df4 100644 --- a/tests/suite/TestDashboardSerializerRoundTrip.m +++ b/tests/suite/TestDashboardSerializerRoundTrip.m @@ -244,5 +244,38 @@ function testSaveEmitsNumberBinding(testCase) testCase.verifyTrue(contains(txt, 'ValueFcn'), ... 'save() must emit the number ValueFcn binding'); end + + function testCustomTypeDeserializesViaRegistry(testCase) + %TESTCUSTOMTYPEDESERIALIZESVIAREGISTRY P1: a registered custom type + % deserializes through the registry (the old switch warned + dropped it). + testCase.addTeardown(@() DashboardWidgetRegistry.reset()); + DashboardWidgetRegistry.register('mytype', @NumberWidget); + base = NumberWidget('Title', 'X', 'Position', [1 1 4 2], 'StaticValue', 7); + s = base.toStruct(); + s.type = 'mytype'; + w = DashboardSerializer.createWidgetFromStruct(s); + testCase.verifyClass(w, 'NumberWidget'); + end + + function testKpiStructDeserializesToNumber(testCase) + %TESTKPISTRUCTDESERIALIZESTONUMBER P1: 'kpi' resolves to NumberWidget via the registry alias. + base = NumberWidget('Title', 'K', 'Position', [1 1 4 2], 'StaticValue', 1); + s = base.toStruct(); + s.type = 'kpi'; + w = DashboardSerializer.createWidgetFromStruct(s); + testCase.verifyClass(w, 'NumberWidget'); + end + + function testUnknownTypeWarnsAndReturnsEmpty(testCase) + %TESTUNKNOWNTYPEWARNSANDRETURNSEMPTY P1: unknown type still warns+skips (preserved). + base = NumberWidget('Title', 'U', 'Position', [1 1 4 2], 'StaticValue', 1); + s = base.toStruct(); + s.type = 'definitelynope'; + lastwarn(''); + w = DashboardSerializer.createWidgetFromStruct(s); + testCase.verifyEmpty(w); + [~, id] = lastwarn(); + testCase.verifyEqual(id, 'DashboardSerializer:unknownType'); + end end end From 9fddafb6916b156be09cb28ab567338c77da4996 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 09:52:17 +0200 Subject: [PATCH 08/14] feat(dashboard): stamp + check serialization schemaVersion; resolve aliases on load (P1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saved dashboard configs now carry a top-level schemaVersion (=1), stamped by widgetsToConfig and widgetsPagesToConfig via a single currentSchemaVersion() constant, so both .json and .m save paths are covered. loadJSON now warns DashboardSerializer:schemaVersionNewer when it reads a config whose schemaVersion exceeds what this build supports, and treats a MISSING field as v1 — so every existing (pre-versioning) dashboard still loads silently. Combined with the registry alias from earlier in this refactor, a legacy 'kpi' widget now survives the full JSON load path (loadJSON -> configToWidgets) as a NumberWidget instead of being dropped. The .m export path is intentionally unchanged (it reconstructs via addWidget, so it needs no version field). Tests: config + JSON carry schemaVersion==1; an old config without the field loads with no warning; a schemaVersion=999 file warns; a 'kpi' JSON widget loads as NumberWidget through the full path. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 3f17ed412144f2a09b9ce5596a315cd6ad904ada) --- libs/Dashboard/DashboardSerializer.m | 29 +++++++++ .../suite/TestDashboardSerializerRoundTrip.m | 63 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/libs/Dashboard/DashboardSerializer.m b/libs/Dashboard/DashboardSerializer.m index 4aa6c48a..e7742ef4 100644 --- a/libs/Dashboard/DashboardSerializer.m +++ b/libs/Dashboard/DashboardSerializer.m @@ -237,6 +237,32 @@ function saveJSON(config, filepath) result = feval(funcname); end + function v = currentSchemaVersion() + %CURRENTSCHEMAVERSION Supported dashboard config schema version. + % Writers stamp this into every config (widgetsToConfig / + % widgetsPagesToConfig). The loader warns + % (DashboardSerializer:schemaVersionNewer) when it reads a config + % whose schemaVersion exceeds this value. Bump only when the on-disk + % config shape changes in a way older loaders cannot read. + v = 1; + end + + function checkSchemaVersion_(config, source) + %CHECKSCHEMAVERSION_ Warn if a loaded config is newer than supported. + % A missing schemaVersion is treated as v1 (pre-versioning files) and + % loads silently. + if ~isfield(config, 'schemaVersion') || isempty(config.schemaVersion) + return; + end + sv = config.schemaVersion; + if sv > DashboardSerializer.currentSchemaVersion() + warning('DashboardSerializer:schemaVersionNewer', ... + ['Dashboard ''%s'' was saved with schemaVersion %g but this build ' ... + 'supports up to %g. Some widgets may not load.'], ... + source, sv, DashboardSerializer.currentSchemaVersion()); + end + end + function config = loadJSON(filepath) %LOADJSON Legacy: read dashboard config from JSON file. fid = fopen(filepath, 'r'); @@ -264,6 +290,7 @@ function saveJSON(config, filepath) config.widgets = {}; end end + DashboardSerializer.checkSchemaVersion_(config, filepath); end function config = widgetsToConfig(name, theme, liveInterval, widgets, infoFile) @@ -278,6 +305,7 @@ function saveJSON(config, filepath) config.infoFile = infoFile; end config.grid = struct('columns', 24); + config.schemaVersion = DashboardSerializer.currentSchemaVersion(); config.widgets = cell(1, numel(widgets)); for i = 1:numel(widgets) config.widgets{i} = widgets{i}.toStruct(); @@ -298,6 +326,7 @@ function saveJSON(config, filepath) config.infoFile = infoFile; end config.grid = struct('columns', 24); + config.schemaVersion = DashboardSerializer.currentSchemaVersion(); config.activePage = activePage; config.pages = cell(1, numel(pages)); for i = 1:numel(pages) diff --git a/tests/suite/TestDashboardSerializerRoundTrip.m b/tests/suite/TestDashboardSerializerRoundTrip.m index 6c0b7df4..550630c6 100644 --- a/tests/suite/TestDashboardSerializerRoundTrip.m +++ b/tests/suite/TestDashboardSerializerRoundTrip.m @@ -277,5 +277,68 @@ function testUnknownTypeWarnsAndReturnsEmpty(testCase) [~, id] = lastwarn(); testCase.verifyEqual(id, 'DashboardSerializer:unknownType'); end + + function testSavedConfigHasSchemaVersion(testCase) + %TESTSAVEDCONFIGHASSCHEMAVERSION P1: every saved config is stamped with schemaVersion. + config = DashboardSerializer.widgetsToConfig('SV', 'dark', 5, ... + {NumberWidget('Title', 'N', 'Position', [1 1 4 2], 'StaticValue', 1)}); + testCase.verifyTrue(isfield(config, 'schemaVersion')); + testCase.verifyEqual(config.schemaVersion, 1); + end + + function testSavedJSONContainsSchemaVersion(testCase) + %TESTSAVEDJSONCONTAINSSCHEMAVERSION P1: schemaVersion survives to JSON and back. + config = DashboardSerializer.widgetsToConfig('SV', 'dark', 5, ... + {NumberWidget('Title', 'N', 'Position', [1 1 4 2], 'StaticValue', 1)}); + fp = fullfile(testCase.TempDir, 'sv.json'); + DashboardSerializer.saveJSON(config, fp); + txt = fileread(fp); + testCase.verifyTrue(contains(txt, 'schemaVersion')); + cfg = DashboardSerializer.loadJSON(fp); + testCase.verifyEqual(cfg.schemaVersion, 1); + end + + function testOldConfigWithoutSchemaVersionLoadsSilently(testCase) + %TESTOLDCONFIGWITHOUTSCHEMAVERSIONLOADSSILENTLY P1 backward-compat: a pre-versioning + % file (no schemaVersion field) loads as v1 with NO warning. + config = DashboardSerializer.widgetsToConfig('Old', 'dark', 5, ... + {NumberWidget('Title', 'N', 'Position', [1 1 4 2], 'StaticValue', 1)}); + if isfield(config, 'schemaVersion') + config = rmfield(config, 'schemaVersion'); + end + fp = fullfile(testCase.TempDir, 'old.json'); + DashboardSerializer.saveJSON(config, fp); + lastwarn(''); + cfg = DashboardSerializer.loadJSON(fp); %#ok + [~, id] = lastwarn(); + testCase.verifyFalse(strcmp(id, 'DashboardSerializer:schemaVersionNewer')); + end + + function testNewerSchemaVersionWarnsOnLoad(testCase) + %TESTNEWERSCHEMAVERSIONWARNSONLOAD P1: a file newer than supported warns on load. + config = DashboardSerializer.widgetsToConfig('New', 'dark', 5, ... + {NumberWidget('Title', 'N', 'Position', [1 1 4 2], 'StaticValue', 1)}); + config.schemaVersion = 999; + fp = fullfile(testCase.TempDir, 'new.json'); + DashboardSerializer.saveJSON(config, fp); + lastwarn(''); + cfg = DashboardSerializer.loadJSON(fp); %#ok + [~, id] = lastwarn(); + testCase.verifyEqual(id, 'DashboardSerializer:schemaVersionNewer'); + end + + function testKpiJSONLoadsAsNumberThroughFullPath(testCase) + %TESTKPIJSONLOADSASNUMBERTHROUGHFULLPATH P1: a legacy 'kpi' widget survives the + % full JSON load path (loadJSON -> configToWidgets) as a NumberWidget. + base = NumberWidget('Title', 'K', 'Position', [1 1 4 2], 'StaticValue', 3); + config = DashboardSerializer.widgetsToConfig('Kpi', 'dark', 5, {base}); + config.widgets{1}.type = 'kpi'; % simulate a pre-rename serialized widget + fp = fullfile(testCase.TempDir, 'kpi.json'); + DashboardSerializer.saveJSON(config, fp); + cfg = DashboardSerializer.loadJSON(fp); + rebuilt = DashboardSerializer.configToWidgets(cfg); + testCase.verifyEqual(numel(rebuilt), 1); + testCase.verifyClass(rebuilt{1}, 'NumberWidget'); + end end end From da29702bcb8d559cce216667da3568386c543423 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 10:06:02 +0200 Subject: [PATCH 09/14] fix(dashboard): GroupWidget.removeChild handles tabbed mode (idx, tabName) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit removeChild only ever removed from obj.Children, so a tabbed GroupWidget (whose children live in obj.Tabs{}.widgets) could not be edited programmatically — the headline nested-layout feature was a one-way street. removeChild now takes an optional tabName mirroring addChild: with a tab name it removes from that tab (GroupWidget:unknownTab if absent, GroupWidget:invalidIndex if out of range); with none it keeps the exact prior Children behaviour, so existing removeChild(idx) calls are byte-identical. Adds tabbed removeChild + unknown-tab + out-of-range tests; panel-mode test unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 049634812a59c4febad288049c69a76fd519c370) --- libs/Dashboard/GroupWidget.m | 29 ++++++++++++++++++++++++----- tests/suite/TestGroupWidget.m | 28 ++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/libs/Dashboard/GroupWidget.m b/libs/Dashboard/GroupWidget.m index 0a7e1d5f..9357f3e4 100644 --- a/libs/Dashboard/GroupWidget.m +++ b/libs/Dashboard/GroupWidget.m @@ -60,12 +60,31 @@ function addChild(obj, widget, tabName) end end - function removeChild(obj, idx) - if idx < 1 || idx > numel(obj.Children) - error('GroupWidget:invalidIndex', ... - 'Child index %d out of range [1, %d]', idx, numel(obj.Children)); + function removeChild(obj, idx, tabName) + %REMOVECHILD Remove a child widget by index. + % removeChild(idx) removes obj.Children(idx) — panel/collapsible mode. + % removeChild(idx, tabName) removes the idx-th widget of the named tab + % (tabbed mode), mirroring addChild(widget, tabName). Throws + % GroupWidget:unknownTab for an unknown tab and GroupWidget:invalidIndex + % for an out-of-range index in either mode. + if nargin >= 3 && ~isempty(tabName) + tIdx = obj.findTab(tabName); + if tIdx == 0 + error('GroupWidget:unknownTab', ... + 'No tab named ''%s''.', tabName); + end + if idx < 1 || idx > numel(obj.Tabs{tIdx}.widgets) + error('GroupWidget:invalidIndex', ... + 'Child index %d out of range [1, %d]', idx, numel(obj.Tabs{tIdx}.widgets)); + end + obj.Tabs{tIdx}.widgets(idx) = []; + else + if idx < 1 || idx > numel(obj.Children) + error('GroupWidget:invalidIndex', ... + 'Child index %d out of range [1, %d]', idx, numel(obj.Children)); + end + obj.Children(idx) = []; end - obj.Children(idx) = []; end function render(obj, parentPanel) diff --git a/tests/suite/TestGroupWidget.m b/tests/suite/TestGroupWidget.m index d4a99730..79a215c9 100644 --- a/tests/suite/TestGroupWidget.m +++ b/tests/suite/TestGroupWidget.m @@ -370,5 +370,33 @@ function testTabContrastAllThemes(testCase) preset, fgDelta)); end end + + function testRemoveChildFromTab(testCase) + % Tabbed groups must be editable: removeChild(idx, tabName) removes + % from the named tab and leaves other tabs intact. + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); + g.addChild(MockDashboardWidget('Title', 'A2'), 'TabA'); + g.addChild(MockDashboardWidget('Title', 'B1'), 'TabB'); + % Tabs append in first-seen order: TabA = 1, TabB = 2 (Tabs is public). + testCase.verifyEqual(g.Tabs{1}.name, 'TabA'); + testCase.verifyLength(g.Tabs{1}.widgets, 2); + g.removeChild(1, 'TabA'); + testCase.verifyLength(g.Tabs{1}.widgets, 1); + testCase.verifyEqual(g.Tabs{1}.widgets{1}.Title, 'A2'); + testCase.verifyLength(g.Tabs{2}.widgets, 1); % TabB untouched + end + + function testRemoveChildUnknownTabErrors(testCase) + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); + testCase.verifyError(@() g.removeChild(1, 'NoSuchTab'), 'GroupWidget:unknownTab'); + end + + function testRemoveChildTabIndexOutOfRange(testCase) + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); + testCase.verifyError(@() g.removeChild(99, 'TabA'), 'GroupWidget:invalidIndex'); + end end end From c4793b7a6f8a4a1f79ecc350dd534dd595c5312a Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 10:06:53 +0200 Subject: [PATCH 10/14] feat(dashboard): GroupWidget.removeTab(tabName) Completes the tabbed-group editing API begun in the previous commit: removeTab drops an entire tab and its widgets, errors GroupWidget:unknownTab for an unknown name, and moves ActiveTab to the first surviving tab (or '' when none remain) when the removed tab was active. Purely additive. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 9c5695bf8f2213f4b50c24181902e20e4a882bb8) --- libs/Dashboard/GroupWidget.m | 21 +++++++++++++++++++++ tests/suite/TestGroupWidget.m | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/libs/Dashboard/GroupWidget.m b/libs/Dashboard/GroupWidget.m index 9357f3e4..61e6a824 100644 --- a/libs/Dashboard/GroupWidget.m +++ b/libs/Dashboard/GroupWidget.m @@ -87,6 +87,27 @@ function removeChild(obj, idx, tabName) end end + function removeTab(obj, tabName) + %REMOVETAB Remove an entire tab and its widgets (tabbed mode). + % Throws GroupWidget:unknownTab if the tab does not exist. When the + % removed tab was the active one, ActiveTab moves to the first remaining + % tab, or '' when none remain. + tIdx = obj.findTab(tabName); + if tIdx == 0 + error('GroupWidget:unknownTab', ... + 'No tab named ''%s''.', tabName); + end + wasActive = strcmp(obj.ActiveTab, tabName); + obj.Tabs(tIdx) = []; + if wasActive + if isempty(obj.Tabs) + obj.ActiveTab = ''; + else + obj.ActiveTab = obj.Tabs{1}.name; + end + end + end + function render(obj, parentPanel) obj.hPanel = parentPanel; theme = obj.getTheme(); diff --git a/tests/suite/TestGroupWidget.m b/tests/suite/TestGroupWidget.m index 79a215c9..1b340c4c 100644 --- a/tests/suite/TestGroupWidget.m +++ b/tests/suite/TestGroupWidget.m @@ -398,5 +398,38 @@ function testRemoveChildTabIndexOutOfRange(testCase) g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); testCase.verifyError(@() g.removeChild(99, 'TabA'), 'GroupWidget:invalidIndex'); end + + function testRemoveTab(testCase) + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); + g.addChild(MockDashboardWidget('Title', 'B1'), 'TabB'); + testCase.verifyLength(g.Tabs, 2); + g.removeTab('TabA'); + testCase.verifyLength(g.Tabs, 1); + testCase.verifyEqual(g.Tabs{1}.name, 'TabB'); + end + + function testRemoveActiveTabReassignsActiveTab(testCase) + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); % ActiveTab defaults to TabA + g.addChild(MockDashboardWidget('Title', 'B1'), 'TabB'); + testCase.verifyEqual(g.ActiveTab, 'TabA'); + g.removeTab('TabA'); + testCase.verifyEqual(g.ActiveTab, 'TabB'); + end + + function testRemoveLastTabClearsActiveTab(testCase) + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); + g.removeTab('TabA'); + testCase.verifyEmpty(g.Tabs); + testCase.verifyEqual(g.ActiveTab, ''); + end + + function testRemoveTabUnknownErrors(testCase) + g = GroupWidget('Label', 'Test', 'Mode', 'tabbed'); + g.addChild(MockDashboardWidget('Title', 'A1'), 'TabA'); + testCase.verifyError(@() g.removeTab('Nope'), 'GroupWidget:unknownTab'); + end end end From 4bdf43b9068d11409397715cfb269bcd3e207aff Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 10:07:55 +0200 Subject: [PATCH 11/14] docs(dashboard): GroupWidget class header + correct Description property doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GroupWidget (the class behind the headline collapsible/tabbed nested layouts) had no class header, so `help GroupWidget` was empty. Adds a standard header documenting the three Modes, the add/remove/switch/collapse contract, the 2-level nesting cap, key properties, and a usage example. Also corrects DashboardWidget.Description: it is shown in a click-to-open info popup via the widget's (i) button, not a hover tooltip as the comment claimed. Docs only — no behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 70324122b9a1bcd6c39ee8eec4df1ce6f43c6b62) --- libs/Dashboard/DashboardWidget.m | 2 +- libs/Dashboard/GroupWidget.m | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/libs/Dashboard/DashboardWidget.m b/libs/Dashboard/DashboardWidget.m index 89a617de..ae6971c4 100644 --- a/libs/Dashboard/DashboardWidget.m +++ b/libs/Dashboard/DashboardWidget.m @@ -13,7 +13,7 @@ Position = [1 1 6 2] % [col, row, width, height] in grid units ThemeOverride = struct() % Per-widget theme overrides (merged on top of dashboard theme) UseGlobalTime = true % false when user manually zooms this widget - Description = '' % Optional tooltip text shown via info icon hover + Description = '' % Doc text shown in a popup when the widget's info (i) button is clicked Tag = [] % v2.0 Tag API — any Tag subclass ParentTheme = [] % Theme inherited from DashboardEngine Dirty = true % true when widget needs refresh (data changed) diff --git a/libs/Dashboard/GroupWidget.m b/libs/Dashboard/GroupWidget.m index 61e6a824..bfe132d1 100644 --- a/libs/Dashboard/GroupWidget.m +++ b/libs/Dashboard/GroupWidget.m @@ -1,4 +1,38 @@ classdef GroupWidget < DashboardWidget +%GROUPWIDGET Container widget that groups child widgets in one of three modes. +% +% GroupWidget delivers the dashboard's nested-layout feature. Set Mode to: +% 'panel' — children laid out in a sub-grid inside a bordered panel +% 'collapsible' — like panel, with a header bar that collapses/expands +% 'tabbed' — children organised into named tabs with a tab strip +% +% Children are added and removed through parallel APIs that depend on Mode: +% addChild(widget) — panel/collapsible (appends to Children) +% addChild(widget, tabName) — tabbed (appends to the named tab, creating it) +% removeChild(idx) — panel/collapsible +% removeChild(idx, tabName) — tabbed +% removeTab(tabName) — drop a whole tab +% Navigation / state: switchTab(tabName), collapse(), expand(). +% +% Groups may nest one level deep (a GroupWidget inside a GroupWidget); a +% maximum nesting depth of 2 is enforced by addChild (GroupWidget:maxDepth). +% +% Key public properties: +% Mode — 'panel' | 'collapsible' | 'tabbed' +% Label — header bar title +% Children — cell of child widgets (panel/collapsible) +% Tabs — cell of struct('name', ..., 'widgets', {{...}}) (tabbed) +% ActiveTab — current tab name (tabbed) +% ChildColumns — sub-grid column count +% +% Example: +% g = GroupWidget('Label', 'Analysis', 'Mode', 'tabbed'); +% g.addChild(NumberWidget('Title', 'RPM'), 'Overview'); +% g.addChild(HistogramWidget('Title', 'Dist'), 'Details'); +% g.switchTab('Details'); +% +% See also DashboardWidget, DashboardEngine, NumberWidget. + properties (Access = public) Mode = 'panel' % 'panel', 'collapsible', 'tabbed' Label = '' % Title shown in header bar From 7e4dc6413a0e3dada8f902c1a02c6c68b9ebc083 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 10:10:04 +0200 Subject: [PATCH 12/14] feat(dashboard): DashboardEngine.removePage(idx) mirrors removeWidget (P1 symmetry) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pages could be added but never removed — the add/remove asymmetry the API review flagged. removePage drops the page at idx (DashboardEngine:invalidIndex on a bad index), deletes the page's widgets and the DashboardPage handle, keeps ActivePage valid (decrement when removing a page before it, clamp when removing the active page, reset to 0 when none remain), and re-renders when a figure is live — exactly mirroring removeWidget. Purely additive. Adds drop / before-active / active-clamp / last-page / invalid-index tests. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 99dac88ec3a5c5b3fd0ab33df2fef49f154715d8) --- libs/Dashboard/DashboardEngine.m | 30 +++++++++++++++++ tests/suite/TestDashboardMultiPage.m | 48 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/libs/Dashboard/DashboardEngine.m b/libs/Dashboard/DashboardEngine.m index e3cce331..ac7aa140 100644 --- a/libs/Dashboard/DashboardEngine.m +++ b/libs/Dashboard/DashboardEngine.m @@ -1483,6 +1483,36 @@ function removeWidget(obj, idx) end end + function removePage(obj, idx) + %REMOVEPAGE Remove the page at index idx, keeping ActivePage valid. + % Mirror of removeWidget for pages. Throws DashboardEngine:invalidIndex + % on a bad index. Deletes the page's widgets and the page, adjusts + % ActivePage (decrements when removing a page before it; clamps when + % removing the active page; resets to 0 when no pages remain), and + % re-renders when a figure is live. + if idx < 1 || idx > numel(obj.Pages) + error('DashboardEngine:invalidIndex', ... + 'Page index %d out of range [1, %d].', idx, numel(obj.Pages)); + end + pg = obj.Pages{idx}; + ws = pg.Widgets; + for i = 1:numel(ws) + delete(ws{i}); + end + obj.Pages(idx) = []; + delete(pg); + if isempty(obj.Pages) + obj.ActivePage = 0; + elseif idx < obj.ActivePage + obj.ActivePage = obj.ActivePage - 1; + elseif idx == obj.ActivePage + obj.ActivePage = max(1, min(obj.ActivePage, numel(obj.Pages))); + end + if ~isempty(obj.hFigure) && ishandle(obj.hFigure) + obj.rerenderWidgets(); + end + end + function setWidgetPosition(obj, idx, pos) %SETWIDGETPOSITION Set the grid position of a widget by index. % Clamps width to grid columns and resolves overlaps with other diff --git a/tests/suite/TestDashboardMultiPage.m b/tests/suite/TestDashboardMultiPage.m index 90b01463..beadb69d 100644 --- a/tests/suite/TestDashboardMultiPage.m +++ b/tests/suite/TestDashboardMultiPage.m @@ -183,6 +183,54 @@ function testSinglePageWidgetAddressingUnchanged(testCase) testCase.verifyTrue(w.Dirty); end + function testRemovePageDropsPage(testCase) + %TESTREMOVEPAGEDROPSPAGE removePage drops the page and keeps the rest. + d = DashboardEngine('mp'); + d.addPage('P1'); d.addPage('P2'); d.addPage('P3'); + testCase.verifyEqual(numel(d.Pages), 3); + d.removePage(2); + testCase.verifyEqual(numel(d.Pages), 2); + testCase.verifyEqual(d.Pages{1}.Name, 'P1'); + testCase.verifyEqual(d.Pages{2}.Name, 'P3'); + end + + function testRemovePageBeforeActiveKeepsActive(testCase) + %TESTREMOVEPAGEBEFOREACTIVEKEEPSACTIVE Removing a page before ActivePage keeps the same page active. + d = DashboardEngine('mp'); + d.addPage('P1'); d.addPage('P2'); d.addPage('P3'); + d.switchPage(3); + testCase.verifyEqual(d.ActivePage, 3); + d.removePage(1); + testCase.verifyEqual(numel(d.Pages), 2); + testCase.verifyEqual(d.Pages{d.ActivePage}.Name, 'P3'); + end + + function testRemoveActivePageClampsActive(testCase) + %TESTREMOVEACTIVEPAGECLAMPSACTIVE Removing the active (last) page leaves ActivePage valid. + d = DashboardEngine('mp'); + d.addPage('P1'); d.addPage('P2'); + d.switchPage(2); + d.removePage(2); + testCase.verifyEqual(numel(d.Pages), 1); + testCase.verifyTrue(d.ActivePage >= 1 && d.ActivePage <= numel(d.Pages)); + end + + function testRemoveLastRemainingPage(testCase) + %TESTREMOVELASTREMAININGPAGE Removing the only page resets ActivePage to 0. + d = DashboardEngine('mp'); + d.addPage('P1'); + d.removePage(1); + testCase.verifyEmpty(d.Pages); + testCase.verifyEqual(d.ActivePage, 0); + end + + function testRemovePageInvalidIndexErrors(testCase) + %TESTREMOVEPAGEINVALIDINDEXERRORS Out-of-range index errors with a namespaced id. + d = DashboardEngine('mp'); + d.addPage('P1'); + testCase.verifyError(@() d.removePage(99), 'DashboardEngine:invalidIndex'); + end + end end From 25c3b9d07b2729ee7243071aaba2b0acadfe0776 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 10:40:59 +0200 Subject: [PATCH 13/14] test(dashboard): cover chart-widget fromStruct restore + asciiRender getXY (codecov patch) Adds non-rendering coverage for the P0-2/P0-3 chart-widget changes (render-based tests don't contribute coverage in headless CI): - Histogram/Heatmap DataFcn survives toStruct/fromStruct round-trip. - BarChart fromStruct warns BarChartWidget:sourceUnresolved on an unknown source type. - Scatter fromStruct warns ScatterWidget:sourceUnresolved when a sensor key is absent from TagRegistry (exercises resolveTag_'s catch path). - Histogram/Scatter asciiRender probe Tag data via getXY() (DerivedTag bound). Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 7cf87544037f3d4c1cb11c2887b8b3a985dbf01b) --- tests/suite/TestBarChartWidget.m | 12 ++++++++++++ tests/suite/TestHeatmapWidget.m | 9 +++++++++ tests/suite/TestHistogramWidget.m | 21 +++++++++++++++++++++ tests/suite/TestScatterWidget.m | 28 ++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+) diff --git a/tests/suite/TestBarChartWidget.m b/tests/suite/TestBarChartWidget.m index bdebfad9..5b2aaf5c 100644 --- a/tests/suite/TestBarChartWidget.m +++ b/tests/suite/TestBarChartWidget.m @@ -55,5 +55,17 @@ function testDerivedTagRefreshNoCrash(testCase) end testCase.verifyFalse(threw, msg); end + + function testBarChartUnresolvedSourceWarns(testCase) + %TESTBARCHARTUNRESOLVEDSOURCEWARNS P0-3: an unknown source.type warns (no throw). + s = struct('type', 'barchart', 'title', 'B', ... + 'position', struct('col', 1, 'row', 1, 'width', 8, 'height', 4), ... + 'source', struct('type', 'weirdtype', 'function', '@() 1')); + lastwarn(''); + w = BarChartWidget.fromStruct(s); + [~, id] = lastwarn(); + testCase.verifyEqual(id, 'BarChartWidget:sourceUnresolved'); + testCase.verifyEmpty(w.DataFcn); + end end end diff --git a/tests/suite/TestHeatmapWidget.m b/tests/suite/TestHeatmapWidget.m index 5edd8583..da281eb4 100644 --- a/tests/suite/TestHeatmapWidget.m +++ b/tests/suite/TestHeatmapWidget.m @@ -55,5 +55,14 @@ function testDerivedTagRefreshNoCrash(testCase) end testCase.verifyFalse(threw, msg); end + + function testHeatmapCallbackRoundTrip(testCase) + %TESTHEATMAPCALLBACKROUNDTRIP P0-3: DataFcn survives toStruct/fromStruct. + w = HeatmapWidget('Title', 'H'); + w.DataFcn = @() magic(4); + w2 = HeatmapWidget.fromStruct(w.toStruct()); + testCase.verifyNotEmpty(w2.DataFcn); + testCase.verifyEqual(func2str(w2.DataFcn), func2str(w.DataFcn)); + end end end diff --git a/tests/suite/TestHistogramWidget.m b/tests/suite/TestHistogramWidget.m index 0b2b6555..645c57f2 100644 --- a/tests/suite/TestHistogramWidget.m +++ b/tests/suite/TestHistogramWidget.m @@ -55,5 +55,26 @@ function testDerivedTagRefreshNoCrash(testCase) end testCase.verifyFalse(threw, msg); end + + function testHistogramCallbackRoundTrip(testCase) + %TESTHISTOGRAMCALLBACKROUNDTRIP P0-3: DataFcn survives toStruct/fromStruct. + w = HistogramWidget('Title', 'H'); + w.DataFcn = @() [1 2 3 4 5]; + w2 = HistogramWidget.fromStruct(w.toStruct()); + testCase.verifyNotEmpty(w2.DataFcn); + testCase.verifyEqual(func2str(w2.DataFcn), func2str(w.DataFcn)); + end + + function testHistogramAsciiRenderTag(testCase) + %TESTHISTOGRAMASCIIRENDERTAG P0-2: asciiRender probes Tag data via getXY(). + a = SensorTag('a', 'X', 1:10, 'Y', 1:10); + b = SensorTag('b', 'X', 1:10, 'Y', 2:11); + d = DerivedTag('d', {a, b}, @(p) deal(p{1}.X, p{1}.Y + p{2}.Y)); + w = HistogramWidget('Title', 'H'); + w.Tag = d; + lines = w.asciiRender(24, 3); + testCase.verifyEqual(numel(lines), 3); + testCase.verifyTrue(contains(lines{2}, 'data points')); + end end end diff --git a/tests/suite/TestScatterWidget.m b/tests/suite/TestScatterWidget.m index 65d7e51e..defb5bb9 100644 --- a/tests/suite/TestScatterWidget.m +++ b/tests/suite/TestScatterWidget.m @@ -43,5 +43,33 @@ function testDerivedTagRefreshNoCrash(testCase) end testCase.verifyFalse(threw, msg); end + + function testScatterAsciiRenderTags(testCase) + %TESTSCATTERASCIIRENDERTAGS P0-2: asciiRender probes SensorX/SensorY via getXY(). + a = SensorTag('a', 'X', 1:10, 'Y', 1:10); + b = SensorTag('b', 'X', 1:10, 'Y', 2:11); + dx = DerivedTag('dx', {a, b}, @(p) deal(p{1}.X, p{1}.Y + p{2}.Y)); + dy = DerivedTag('dy', {a, b}, @(p) deal(p{1}.X, p{2}.Y)); + w = ScatterWidget('Title', 'S'); + w.SensorX = dx; + w.SensorY = dy; + lines = w.asciiRender(24, 3); + testCase.verifyEqual(numel(lines), 3); + testCase.verifyTrue(contains(lines{2}, 'points')); + end + + function testScatterUnresolvedSensorWarns(testCase) + %TESTSCATTERUNRESOLVEDSENSORWARNS P0-3: a sensor key absent from TagRegistry warns (no throw). + TagRegistry.clear(); + testCase.addTeardown(@() TagRegistry.clear()); + s = struct('type', 'scatter', 'title', 'S', ... + 'position', struct('col', 1, 'row', 1, 'width', 8, 'height', 4), ... + 'sensorX', 'missingX', 'sensorY', 'missingY'); + lastwarn(''); + w = ScatterWidget.fromStruct(s); + [~, id] = lastwarn(); + testCase.verifyEqual(id, 'ScatterWidget:sourceUnresolved'); + testCase.verifyEmpty(w.SensorX); + end end end From caaaa81170cc91b2806d75e2bcfa797842e4be09 Mon Sep 17 00:00:00 2001 From: Hannes Suhr Date: Tue, 2 Jun 2026 16:08:39 +0200 Subject: [PATCH 14/14] fix(dashboard): consolidate threshold-violation check; fix MultiStatus inclusive bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The threshold-violation predicate was copy-pasted into ~8 loops across 3 widgets with inconsistent boundary semantics: MultiStatusWidget used inclusive >= / <= while StatusWidget, ChipBarWidget, IconCardWidget and GaugeWidget all used strict > / <. A sensor sitting exactly on a threshold limit appeared red in a MultiStatus tile but green in every other widget. Fixes: - New shared isThresholdViolated(threshold, value) helper (strict > / < throughout, matching the SensorThreshold engine convention). Returns false on empty inputs and for CompositeThresholds with no values. - MultiStatusWidget's 3 first-violation loops (2 static-state, 1 legacy-Sensor) routed through the helper, fixing all 3 inclusive comparisons. - ChipBarWidget's 2 and IconCardWidget's 2 first-violation loops also routed through the helper — behaviour-preserving (they were already strict), but they can no longer drift back. - StatusWidget and GaugeWidget left as-is: they rank violations by distance to colour the worst threshold, which a boolean predicate can't replace. New: MockThreshold stub (IsUpper + allValues) for pure-logic testing without the SensorThreshold class graph; TestIsThresholdViolated (4 tests: upper/lower strict, multi-value, empty guards); regression test in TestMultiStatusWidget documenting the exact on-limit fix. Regression sweep: 628 tests, 614 pass, 14 fail = all known pre-existing env/flaky. New failures: 0. Co-Authored-By: Claude Opus 4.8 (1M context) (cherry picked from commit 37329135aefca47cc91ac039372721dd226a2ecc) --- libs/Dashboard/ChipBarWidget.m | 19 ++++-------- libs/Dashboard/IconCardWidget.m | 21 ++++--------- libs/Dashboard/MultiStatusWidget.m | 39 ++++++++---------------- libs/Dashboard/isThresholdViolated.m | 42 ++++++++++++++++++++++++++ tests/suite/MockThreshold.m | 27 +++++++++++++++++ tests/suite/TestIsThresholdViolated.m | 43 +++++++++++++++++++++++++++ tests/suite/TestMultiStatusWidget.m | 17 +++++++++++ 7 files changed, 153 insertions(+), 55 deletions(-) create mode 100644 libs/Dashboard/isThresholdViolated.m create mode 100644 tests/suite/MockThreshold.m create mode 100644 tests/suite/TestIsThresholdViolated.m diff --git a/libs/Dashboard/ChipBarWidget.m b/libs/Dashboard/ChipBarWidget.m index 5c6772d5..086ea888 100644 --- a/libs/Dashboard/ChipBarWidget.m +++ b/libs/Dashboard/ChipBarWidget.m @@ -303,12 +303,9 @@ function clearRenderLock_(obj) val = chip.value; end if isempty(val), chipColor = [0.5 0.5 0.5]; return; end - tVals = t.allValues(); state = 'ok'; - for v = 1:numel(tVals) - if (t.IsUpper && val > tVals(v)) || (~t.IsUpper && val < tVals(v)) - state = 'alarm'; break; - end + if isThresholdViolated(t, val) + state = 'alarm'; end elseif isfield(chip, 'statusFcn') && ~isempty(chip.statusFcn) try @@ -322,16 +319,10 @@ function clearRenderLock_(obj) latestY = sensor.Y(end); state = 'ok'; for k = 1:numel(sensor.Thresholds) - t = sensor.Thresholds{k}; - tVals = t.allValues(); - for v = 1:numel(tVals) - if (t.IsUpper && latestY > tVals(v)) || ... - (~t.IsUpper && latestY < tVals(v)) - state = 'alarm'; - break; - end + if isThresholdViolated(sensor.Thresholds{k}, latestY) + state = 'alarm'; + break; end - if strcmp(state, 'alarm'), break; end end else state = 'ok'; diff --git a/libs/Dashboard/IconCardWidget.m b/libs/Dashboard/IconCardWidget.m index 65fc2c9d..626fbb86 100644 --- a/libs/Dashboard/IconCardWidget.m +++ b/libs/Dashboard/IconCardWidget.m @@ -419,13 +419,9 @@ function relayout_(obj) end val = obj.CurrentValue; if isempty(val), state = 'inactive'; return; end - tVals = obj.Threshold.allValues(); - for v = 1:numel(tVals) - if (obj.Threshold.IsUpper && val > tVals(v)) || ... - (~obj.Threshold.IsUpper && val < tVals(v)) - state = 'alarm'; - return; - end + if isThresholdViolated(obj.Threshold, val) + state = 'alarm'; + return; end end @@ -441,14 +437,9 @@ function relayout_(obj) end latestY = obj.Sensor.Y(end); for i = 1:numel(obj.Sensor.Thresholds) - t = obj.Sensor.Thresholds{i}; - tVals = t.allValues(); - for v = 1:numel(tVals) - if (t.IsUpper && latestY > tVals(v)) || ... - (~t.IsUpper && latestY < tVals(v)) - state = 'alarm'; - return; - end + if isThresholdViolated(obj.Sensor.Thresholds{i}, latestY) + state = 'alarm'; + return; end end end diff --git a/libs/Dashboard/MultiStatusWidget.m b/libs/Dashboard/MultiStatusWidget.m index 748630e0..cebb4460 100644 --- a/libs/Dashboard/MultiStatusWidget.m +++ b/libs/Dashboard/MultiStatusWidget.m @@ -210,16 +210,10 @@ function refresh(obj) val = s.Y(end); violated = false; for r = 1:numel(s.Thresholds) - t = s.Thresholds{r}; - tVals = t.allValues(); - for v = 1:numel(tVals) - if (t.IsUpper && val > tVals(v)) || ... - (~t.IsUpper && val < tVals(v)) - violated = true; - break; - end + if isThresholdViolated(s.Thresholds{r}, val) + violated = true; + break; end - if violated, break; end end if ~violated nOk = nOk + 1; @@ -423,17 +417,15 @@ function relayout_(obj) val = item.value; end if isempty(val), return; end - % Check violation - tVals = t.allValues(); - for v = 1:numel(tVals) - if (t.IsUpper && val >= tVals(v)) || (~t.IsUpper && val <= tVals(v)) - if ~isempty(t.Color) - color = t.Color; - else - color = theme.StatusAlarmColor; - end - return; + % Check violation (strict, via the shared predicate — was inclusive >=/<=, + % which lit a sensor on its limit red here but green in every other widget). + if isThresholdViolated(t, val) + if ~isempty(t.Color) + color = t.Color; + else + color = theme.StatusAlarmColor; end + return; end end @@ -475,13 +467,8 @@ function relayout_(obj) for k = 1:numel(sensor.Thresholds) t = sensor.Thresholds{k}; if isempty(t.Color), continue; end - tVals = t.allValues(); - for v = 1:numel(tVals) - if t.IsUpper && val >= tVals(v) - color = t.Color; - elseif ~t.IsUpper && val <= tVals(v) - color = t.Color; - end + if isThresholdViolated(t, val) + color = t.Color; end end end diff --git a/libs/Dashboard/isThresholdViolated.m b/libs/Dashboard/isThresholdViolated.m new file mode 100644 index 00000000..1fbf6d99 --- /dev/null +++ b/libs/Dashboard/isThresholdViolated.m @@ -0,0 +1,42 @@ +function tf = isThresholdViolated(threshold, value) +%ISTHRESHOLDVIOLATED True when VALUE strictly breaches THRESHOLD. +% tf = isThresholdViolated(threshold, value) returns true when VALUE violates +% any of threshold.allValues() under threshold.IsUpper, using the STRICT +% convention: a value strictly ABOVE an upper limit, or strictly BELOW a lower +% limit, is a violation. A value sitting EXACTLY on a limit is NOT a violation +% — matching the SensorThreshold engine and the majority of dashboard widgets +% (this helper removes the lone inclusive >=/<= comparison that made a sensor +% on its limit light red in a MultiStatus tile but green in a Status dot). +% +% This is the single source of truth for the dashboard's threshold-violation +% check, replacing the copy-pasted comparison loops in MultiStatusWidget, +% ChipBarWidget and IconCardWidget. (StatusWidget and GaugeWidget keep their +% own loops because they additionally rank violations by distance to choose +% the most-violated threshold's colour, which a boolean predicate cannot do.) +% +% Inputs: +% threshold — a threshold object exposing IsUpper (logical) and a +% allValues() method returning a numeric vector of condition +% values (CompositeThreshold returns [], i.e. never violated here). +% value — numeric scalar (latest sample or static value). +% +% Output: +% tf — logical scalar. Returns false for an empty threshold or value +% (hot-path tolerant — never throws on missing data). +% +% See also severityColor, MultiStatusWidget, ChipBarWidget, IconCardWidget. + + tf = false; + if isempty(threshold) || isempty(value) + return; + end + + tVals = threshold.allValues(); + isUpper = threshold.IsUpper; + for v = 1:numel(tVals) + if (isUpper && value > tVals(v)) || (~isUpper && value < tVals(v)) + tf = true; + return; + end + end +end diff --git a/tests/suite/MockThreshold.m b/tests/suite/MockThreshold.m new file mode 100644 index 00000000..fbb895c2 --- /dev/null +++ b/tests/suite/MockThreshold.m @@ -0,0 +1,27 @@ +classdef MockThreshold < handle +%MOCKTHRESHOLD Minimal threshold stub for testing isThresholdViolated. +% Exposes the IsUpper property and allValues() method (plus Color) that the +% dashboard widgets and isThresholdViolated rely on, without depending on the +% full SensorThreshold class graph. +% +% Usage: +% t = MockThreshold(true, 10); % upper limit at 10 +% t = MockThreshold(false, [2 5]); % lower limit, composite values + + properties + IsUpper = true + Values = [] + Color = [] + end + + methods + function obj = MockThreshold(isUpper, values) + if nargin >= 1, obj.IsUpper = isUpper; end + if nargin >= 2, obj.Values = values; end + end + + function vals = allValues(obj) + vals = obj.Values; + end + end +end diff --git a/tests/suite/TestIsThresholdViolated.m b/tests/suite/TestIsThresholdViolated.m new file mode 100644 index 00000000..6cddb338 --- /dev/null +++ b/tests/suite/TestIsThresholdViolated.m @@ -0,0 +1,43 @@ +classdef TestIsThresholdViolated < matlab.unittest.TestCase +%TESTISTHRESHOLDVIOLATED Tests the shared strict threshold-violation predicate. + + methods (TestClassSetup) + function addPaths(testCase) + addpath(fullfile(fileparts(mfilename('fullpath')), '..', '..')); + install(); + end + end + + methods (Test) + function testUpperStrict(testCase) + t = MockThreshold(true, 10); + testCase.verifyTrue(isThresholdViolated(t, 11)); % strictly above -> violated + testCase.verifyFalse(isThresholdViolated(t, 10)); % exactly on limit -> NOT violated + testCase.verifyFalse(isThresholdViolated(t, 9)); % below -> ok + end + + function testLowerStrict(testCase) + t = MockThreshold(false, 5); + testCase.verifyTrue(isThresholdViolated(t, 4)); % strictly below -> violated + testCase.verifyFalse(isThresholdViolated(t, 5)); % exactly on limit -> NOT violated + testCase.verifyFalse(isThresholdViolated(t, 6)); % above -> ok + end + + function testMultipleValues(testCase) + t = MockThreshold(true, [10 20 30]); + testCase.verifyTrue(isThresholdViolated(t, 25)); % 25 > 10 -> violated + testCase.verifyFalse(isThresholdViolated(t, 5)); % below all limits -> ok + % Clean boundary test: single value so "on limit" is unambiguous. + tSingle = MockThreshold(true, 30); + testCase.verifyFalse(isThresholdViolated(tSingle, 30)); % exactly on -> NOT violated (strict) + testCase.verifyTrue(isThresholdViolated(tSingle, 31)); % just above -> violated + end + + function testEmptyGuards(testCase) + testCase.verifyFalse(isThresholdViolated([], 5)); + testCase.verifyFalse(isThresholdViolated(MockThreshold(true, 10), [])); + % Composite threshold (no condition values) -> never violated here. + testCase.verifyFalse(isThresholdViolated(MockThreshold(true, []), 5)); + end + end +end diff --git a/tests/suite/TestMultiStatusWidget.m b/tests/suite/TestMultiStatusWidget.m index 197c2e1b..a49a6369 100644 --- a/tests/suite/TestMultiStatusWidget.m +++ b/tests/suite/TestMultiStatusWidget.m @@ -23,5 +23,22 @@ function testToStruct(testCase) testCase.verifyEqual(s.columns, 4); testCase.verifyEqual(s.iconStyle, 'square'); end + + function testThresholdOnLimitNotViolated(testCase) + %TESTTHRESHOLDONLIMITNOTVIOLATED Regression for the inclusive (>=) bug in + % deriveColorFromThreshold: a value sitting EXACTLY on a threshold limit + % should NOT be a violation (strict > / < convention matching all other + % dashboard widgets). The private method now delegates to isThresholdViolated. + upper = MockThreshold(true, 10); + lower = MockThreshold(false, 5); + % On the limit — must NOT be a violation. + testCase.verifyFalse(isThresholdViolated(upper, 10), ... + 'val == upper limit must NOT be a violation (was inclusive >= before fix)'); + testCase.verifyFalse(isThresholdViolated(lower, 5), ... + 'val == lower limit must NOT be a violation (was inclusive <= before fix)'); + % Strictly beyond the limit — must be a violation. + testCase.verifyTrue(isThresholdViolated(upper, 11)); + testCase.verifyTrue(isThresholdViolated(lower, 4)); + end end end