From 8dffeef34e93366397be45a3a6248691a0b370fc Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Wed, 12 Aug 2026 23:22:42 +0300 Subject: [PATCH 1/3] fix(gauges): clear the reading when re-pointed at another path dataAvailable, value and textValue were written only inside the stream callback, and a rebuilt subscription against a silent path replays nothing -- the leading null is suppressed with a fresh closure -- so the previous path's needle stayed on the dial as a live reading of the new one. Gate a reset on the path signature so a theme change, which rebuilds the same subscription, leaves the reading alone. Fixes #534 --- .../directives/widget-streams.directive.ts | 40 ++++++++++++++--- .../widget-gauge-ng-compass.component.spec.ts | 42 +++++++++++++++-- .../widget-gauge-ng-compass.component.ts | 32 +++++++++++-- .../widget-gauge-ng-linear.component.spec.ts | 45 +++++++++++++++++-- .../widget-gauge-ng-linear.component.ts | 29 ++++++++++-- .../widget-gauge-ng-radial.component.spec.ts | 41 ++++++++++++++++- .../widget-gauge-ng-radial.component.ts | 32 +++++++++++-- 7 files changed, 237 insertions(+), 24 deletions(-) diff --git a/src/app/core/directives/widget-streams.directive.ts b/src/app/core/directives/widget-streams.directive.ts index 8635e94d..80c3bab4 100644 --- a/src/app/core/directives/widget-streams.directive.ts +++ b/src/app/core/directives/widget-streams.directive.ts @@ -8,6 +8,38 @@ import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; /** Fixed stale-data TTL (ms) applied to every widget whose enableTimeout is on; not user-configurable. */ const FIXED_DATA_TIMEOUT_MS = 5000; +/** The subset of a path config that decides which reading a subscription delivers. */ +interface IPathIdentity { + path: string | null; + pathType?: string | null; + convertUnitTo?: string | null; + source?: string | null; + suppressBootstrapNull?: boolean; +} + +/** Trim a configured path to its canonical form; undefined when it is not a usable path. */ +export function normalizeWidgetPath(path: unknown): string | undefined { + const trimmed = typeof path === 'string' ? path.trim() : ''; + return trimmed.length ? trimmed : undefined; +} + +/** + * Identity of a configured path as the subscription diff computes it. Two configs that share a + * signature reuse one subscription; a different signature means the widget is now watching another + * reading. A widget that holds presentation state derived from the stream — a needle position, a + * last value, an alarm colour — needs this to tell a re-point apart from an unrelated reconfigure + * such as a theme change, because a rebuilt subscription may replay nothing at all and leave that + * state showing the previous path. + * + * Returns null for a config with no usable path, which has no identity to compare. + */ +export function widgetPathSignature(pathCfg: IPathIdentity | undefined | null): string | null { + const normalizedPath = normalizeWidgetPath(pathCfg?.path); + if (!pathCfg || !normalizedPath) return null; + const src = (pathCfg.source?.trim() || 'default'); + return [normalizedPath, pathCfg.pathType, pathCfg.convertUnitTo, src, pathCfg.suppressBootstrapNull ? '1' : '0'].join('|'); +} + @Directive({ selector: '[widget-streams]', exportAs: 'widgetStreams' @@ -63,9 +95,7 @@ export class WidgetStreamsDirective implements OnDestroy { } private computePathSignature(pathCfg: { path: string; pathType: string; convertUnitTo?: string; source?: string; suppressBootstrapNull?: boolean }): string { - const normalizedPath = this.normalizePath(pathCfg.path) ?? ''; - const src = (pathCfg.source?.trim() || 'default'); - return [normalizedPath, pathCfg.pathType, pathCfg.convertUnitTo, src, pathCfg.suppressBootstrapNull ? '1' : '0'].join('|'); + return widgetPathSignature(pathCfg) ?? ''; } private computeBaseKey(path: string, source?: string): string { @@ -75,9 +105,7 @@ export class WidgetStreamsDirective implements OnDestroy { } private normalizePath(path: unknown): string | undefined { - if (typeof path !== 'string') return undefined; - const trimmed = path.trim(); - return trimmed.length ? trimmed : undefined; + return normalizeWidgetPath(path); } private computeRootSignature(cfg: IWidgetSvcConfig | undefined): string { diff --git a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts index 8dbb8b8e..7bb308b7 100644 --- a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts @@ -1,4 +1,4 @@ -import { signal } from '@angular/core'; +import { WritableSignal, signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { beforeEach, describe, expect, it } from 'vitest'; import { WidgetGaugeNgCompassComponent } from './widget-gauge-ng-compass.component'; @@ -20,6 +20,7 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { let fixture: ComponentFixture; let internals: CompassInternals; let capturedNext: ((u: IPathUpdate) => void) | undefined; + let options: WritableSignal; interface CompassInternals { value: () => number | null | undefined; @@ -29,12 +30,12 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { gaugeOptions: { needle?: boolean }; } - const makeConfig = (): IWidgetSvcConfig => { + const makeConfig = (path = 'self.navigation.headingTrue'): IWidgetSvcConfig => { const dflt = WidgetGaugeNgCompassComponent.DEFAULT_CONFIG; const gaugePath = (dflt.paths as IPathArray)['gaugePath']; return { ...dflt, - paths: { gaugePath: { ...gaugePath, path: 'self.navigation.headingTrue' } } + paths: { gaugePath: { ...gaugePath, path } } }; }; @@ -43,7 +44,7 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { beforeEach(async () => { capturedNext = undefined; - const options = signal(makeConfig()); + options = signal(makeConfig()); const streamsFake = { observe(_pathName: string, next: (u: IPathUpdate) => void) { capturedNext = next; } }; @@ -96,4 +97,37 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { expect(internals.dataAvailable()).toBe(false); expect(internals.textValue()).toBe('--'); }); + + // #534: a rebuilt subscription against a silent path replays nothing (the leading null is + // suppressed), so the callback never runs and the previous heading stayed on the rose. + it('clears the heading when re-pointed at a path that reports nothing', () => { + capturedNext?.(update(142)); + expect(internals.dataAvailable()).toBe(true); + + options.set(makeConfig('self.navigation.headingMagnetic')); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(false); + expect(internals.value()).toBeUndefined(); + expect(internals.textValue()).toBe('--'); + }); + + // The same effect re-runs on a theme change, so an unconditional clear would blink the needle + // off and back on at every switch. + it('keeps the heading when the config changes without changing the path', () => { + capturedNext?.(update(142)); + + fixture.componentRef.setInput('theme', { + contrast: 'rgba(0,0,0,1)', contrastDim: 'rgba(60,60,60,1)', + contrastDimmer: 'rgba(120,120,120,1)', cardColor: 'rgba(238,238,238,1)', + background: 'rgba(255,255,255,1)', zoneAlarm: 'rgba(255,0,0,1)', + zoneWarn: 'rgba(255,170,0,1)', zoneAlert: 'rgba(255,0,255,1)', + zoneEmergency: 'rgba(255,0,0,1)' + }); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(142); + expect(internals.textValue()).toBe('142'); + }); }); diff --git a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts index 6ae6454f..a369e8ee 100644 --- a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts +++ b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts @@ -15,7 +15,7 @@ import { States } from '../../core/interfaces/signalk-interfaces'; import { getColors } from '../../core/utils/themeColors.utils'; import { SkipResizeObserverDirective } from '../../core/directives/skip-resize-observer.directive'; import { WidgetRuntimeDirective } from '../../core/directives/widget-runtime.directive'; -import { WidgetStreamsDirective } from '../../core/directives/widget-streams.directive'; +import { WidgetStreamsDirective, widgetPathSignature } from '../../core/directives/widget-streams.directive'; import { ITheme } from '../../core/services/app-service'; import { UnitsService } from '../../core/services/units.service'; @@ -124,6 +124,28 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { protected colorStrokeTicks = ''; private currentState = signal(States.Normal); private lastAppliedState: States | null = null; + /** Identity of the path the reading state describes; null until the first subscription. */ + private lastPathSignature: string | null = null; + + /** + * Drop the reading when the widget is re-pointed at another path. + * + * The subscription is rebuilt on every run of the data effect, a theme change included, and + * `suppressBootstrapNull` gives each rebuild a fresh suppression closure. Against a path that + * reports nothing the replayed leading null is therefore filtered and the stream callback never + * runs — leaving the previous path's needle and value on screen, presented as a live reading of + * the new one. Clearing unconditionally here is wrong for the same reason: this effect re-runs on + * theme changes, which would blink the needle off and back on at every switch. + */ + private clearReadingOnRepoint(signature: string | null): void { + if (this.lastPathSignature !== null && this.lastPathSignature !== signature) { + this.dataAvailable.set(false); + this.value.set(undefined); + this.textValue.set('--'); + this.currentState.set(States.Normal); + } + this.lastPathSignature = signature; + } private readonly negToPortPaths = [ "self.environment.wind.angleApparent", @@ -147,7 +169,10 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { if (!cfg || !theme) return; const pCfg = cfg.paths?.['gaugePath']; if (!pCfg?.path) return; - untracked(() => this.streams.observe('gaugePath', pkt => { + const signature = widgetPathSignature(pCfg); + untracked(() => { + this.clearReadingOnRepoint(signature); + this.streams.observe('gaugePath', pkt => { let raw = (pkt?.data?.value as number) ?? null; this.dataAvailable.set(raw != null); if (raw == null) { @@ -161,7 +186,8 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { } const newState = (pkt?.state ?? States.Normal) as States; if (newState !== this.currentState()) this.currentState.set(newState); - })); + }); + }); }); // Build options when config/theme changes diff --git a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts index f5a62d24..787aebfd 100644 --- a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts @@ -9,6 +9,7 @@ import { WidgetMetadataDirective } from '../../core/directives/widget-metadata.d import { UnitsService } from '../../core/services/units.service'; import { IWidgetSvcConfig, IPathArray } from '../../core/interfaces/widgets-interface'; import { States } from '../../core/interfaces/signalk-interfaces'; +import { IPathUpdate } from '../../core/services/data.service'; /** * The label and the unit are rendered by the component as one header row on the card, so the gauge @@ -26,11 +27,13 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { let internals: LinearInternals; let options: WritableSignal; let sizeUpdates: LinearGaugeOptions[]; + let capturedNext: ((u: IPathUpdate) => void) | undefined; interface LinearInternals { effectiveUnit: WritableSignal; dataAvailable: WritableSignal; currentState: WritableSignal; + value: () => number | null | undefined; barColor: (cfg: IWidgetSvcConfig, theme: unknown, state: string) => string; optionsReady: () => boolean; textValue: () => string; @@ -45,17 +48,20 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { cardColor: '#111', background: '#000' }; - const makeConfig = (subType = 'vertical'): IWidgetSvcConfig => { + const makeConfig = (subType = 'vertical', path = 'self.navigation.speedOverGround'): IWidgetSvcConfig => { const dflt = WidgetGaugeNgLinearComponent.DEFAULT_CONFIG; const gaugePath = (dflt.paths as IPathArray)['gaugePath']; return { ...dflt, ignoreZones: true, gauge: { ...dflt.gauge, type: 'ngLinear', subType }, - paths: { gaugePath: { ...gaugePath, path: 'self.navigation.speedOverGround', convertUnitTo: 'knots' } } + paths: { gaugePath: { ...gaugePath, path, convertUnitTo: 'knots' } } }; }; + const update = (value: unknown, measure?: string): IPathUpdate => + ({ data: { value, timestamp: null, measure }, state: States.Normal }) as unknown as IPathUpdate; + const unitsFake = { convertBetweenMeasures: (from: string, to: string, value: number): number => from === to ? value : value, getUnitDisplaySymbol: (measure: string | null | undefined): string => measure ?? '', @@ -70,12 +76,13 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { beforeEach(async () => { options = signal(makeConfig()); sizeUpdates = []; + capturedNext = undefined; await TestBed.configureTestingModule({ imports: [WidgetGaugeNgLinearComponent], providers: [ { provide: WidgetRuntimeDirective, useValue: { options } }, - { provide: WidgetStreamsDirective, useValue: { observe: () => undefined } }, + { provide: WidgetStreamsDirective, useValue: { observe: (_p: string, next: (u: IPathUpdate) => void) => { capturedNext = next; } } }, { provide: WidgetMetadataDirective, useValue: { zones: () => [], observe: () => undefined } }, { provide: UnitsService, useValue: unitsFake } ] @@ -216,4 +223,36 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { expect(internals.barColor(cfg, themed, States.Alarm)).toBe('rgba(0,0,0,0)'); }); }); + + // #534: a rebuilt subscription against a silent path replays nothing (the leading null is + // suppressed), so the callback never runs and the previous path's reading stayed on the bar. + describe('re-point', () => { + it('clears the reading when re-pointed at a path that reports nothing', () => { + capturedNext?.(update(6.5, 'knots')); + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(6.5); + + options.set(makeConfig('vertical', 'self.environment.depth.belowTransducer')); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(false); + expect(internals.value()).toBeUndefined(); + expect(internals.textValue()).toBe('--'); + expect(internals.effectiveUnit()).toBe(''); + }); + + // The same effect re-runs on a theme change, so an unconditional clear would blink the bar off + // and back on at every switch. + it('keeps the reading when the config changes without changing the path', () => { + capturedNext?.(update(6.5, 'knots')); + expect(internals.dataAvailable()).toBe(true); + + fixture.componentRef.setInput('theme', { ...theme, cardColor: '#eee', background: '#fff' }); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(6.5); + expect(internals.effectiveUnit()).toBe('knots'); + }); + }); }); diff --git a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts index d6220ce6..4f0ec022 100644 --- a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts +++ b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts @@ -16,7 +16,7 @@ import { getColors } from '../../core/utils/themeColors.utils'; import { States } from '../../core/interfaces/signalk-interfaces'; import { SkipResizeObserverDirective } from '../../core/directives/skip-resize-observer.directive'; import { WidgetRuntimeDirective } from '../../core/directives/widget-runtime.directive'; -import { WidgetStreamsDirective } from '../../core/directives/widget-streams.directive'; +import { WidgetStreamsDirective, widgetPathSignature } from '../../core/directives/widget-streams.directive'; import { WidgetMetadataDirective } from '../../core/directives/widget-metadata.directive'; import { UnitsService } from '../../core/services/units.service'; import { ITheme } from '../../core/services/app-service'; @@ -103,6 +103,29 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { private currentState = signal(States.Normal); /** Measure the incoming value was converted to (server-resolved for this display path). '' = boot placeholder. */ private effectiveUnit = signal(''); + /** Identity of the path the state above describes; null until the first subscription. */ + private lastPathSignature: string | null = null; + + /** + * Drop the reading when the widget is re-pointed at another path. + * + * The subscription is rebuilt on every run of the data effect, a theme change included, and + * `suppressBootstrapNull` gives each rebuild a fresh suppression closure. Against a path that + * reports nothing the replayed leading null is therefore filtered and the stream callback never + * runs — leaving the previous path's bar, value and unit on screen, presented as a live reading + * of the new one. Clearing unconditionally here is wrong for the same reason: this effect re-runs + * on theme changes, which would blink the bar off and back on at every switch. + */ + private clearReadingOnRepoint(signature: string | null): void { + if (this.lastPathSignature !== null && this.lastPathSignature !== signature) { + this.dataAvailable.set(false); + this.value.set(undefined); + this.textValue.set('--'); + this.effectiveUnit.set(''); + this.currentState.set(States.Normal); + } + this.lastPathSignature = signature; + } protected adjustedScale = computed(() => { const cfg = this.runtime.options(); @@ -144,10 +167,10 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { const theme = this.theme(); if (!cfg || !theme) return; if (!cfg.paths?.['gaugePath'].path) return; + const signature = widgetPathSignature(cfg.paths['gaugePath']); untracked(() => { - // Reset the tagged measure so a stale unit never paints the new subscription's value. - this.effectiveUnit.set(''); + this.clearReadingOnRepoint(signature); this.streams.observe('gaugePath', path => { const raw = (path?.data?.value as number) ?? null; const measure = path.data.measure ?? ''; diff --git a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts index 29ed4e42..d1957274 100644 --- a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts @@ -44,7 +44,7 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' } // convertUnitTo is the stored authoring unit; a tagged measure that differs is the flip target. - const makeConfig = (): IWidgetSvcConfig => { + const makeConfig = (path = 'self.test.soc'): IWidgetSvcConfig => { const dflt = WidgetGaugeNgRadialComponent.DEFAULT_CONFIG; const gaugePath = (dflt.paths as IPathArray)['gaugePath']; return { @@ -53,7 +53,7 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' displayScale: { lower: 10, upper: 100, type: 'linear' }, gauge: { ...dflt.gauge, type: 'ngRadial', subType: 'capacity' }, paths: { - gaugePath: { ...gaugePath, path: 'self.test.soc', convertUnitTo: 'ratio' } + gaugePath: { ...gaugePath, path, convertUnitTo: 'ratio' } } }; }; @@ -175,4 +175,41 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' // With the tag cleared, the scale falls back to the stored convertUnitTo bounds again. expect(internals.adjustedScale()).toEqual({ min: 10, max: 100, majorTicks: [] }); }); + + // #534: a rebuilt subscription against a silent path replays nothing (the leading null is + // suppressed), so the stream callback never runs and the previous path's reading stayed on the + // dial, presented as a live reading of the new path. + it('clears the reading when re-pointed at a path that reports nothing', () => { + capturedNext?.(update(42, 'percent')); + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(42); // within the reinterpreted 20..200 scale, so unclamped + + options.set(makeConfig('self.test.silent')); + fixture.detectChanges(); + + // The new subscription delivers nothing at all — exactly the case that used to leave the old + // needle in place. + expect(internals.dataAvailable()).toBe(false); + expect(internals.value()).toBeUndefined(); + expect(internals.textValue()).toBe('--'); + expect(internals.effectiveUnit()).toBe(''); + }); + + // The same effect re-runs on a theme change, so an unconditional clear would blink the needle off + // and back on at every switch. + it('keeps the reading when the config changes without changing the path', () => { + capturedNext?.(update(42, 'percent')); + expect(internals.dataAvailable()).toBe(true); + + fixture.componentRef.setInput('theme', { + contrast: '#000', contrastDim: '#333', contrastDimmer: '#666', + cardColor: '#eee', background: '#fff' + }); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(42); + expect(internals.textValue()).toBe(''); + expect(internals.effectiveUnit()).toBe('percent'); + }); }); diff --git a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts index a8addda8..4cbaadc1 100644 --- a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts +++ b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts @@ -16,7 +16,7 @@ import { getHighlights } from '../../core/utils/zones-highlight.utils'; import { getColors } from '../../core/utils/themeColors.utils'; import { SkipResizeObserverDirective } from '../../core/directives/skip-resize-observer.directive'; import { WidgetRuntimeDirective } from '../../core/directives/widget-runtime.directive'; -import { WidgetStreamsDirective } from '../../core/directives/widget-streams.directive'; +import { WidgetStreamsDirective, widgetPathSignature } from '../../core/directives/widget-streams.directive'; import { WidgetMetadataDirective } from '../../core/directives/widget-metadata.directive'; import { UnitsService } from '../../core/services/units.service'; import { ITheme } from '../../core/services/app-service'; @@ -143,6 +143,28 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { private pathDataState = signal(null); private viewReady = signal(false); protected gaugeOptions: RadialGaugeOptions = {} as RadialGaugeOptions; + /** Identity of the path the state below describes; null until the first subscription. */ + private lastPathSignature: string | null = null; + + /** + * Drop the reading when the widget is re-pointed at another path. + * + * The subscription is rebuilt on every run of the data effect, a theme change included, and + * `suppressBootstrapNull` gives each rebuild a fresh suppression closure. Against a path that + * reports nothing the replayed leading null is therefore filtered and the stream callback never + * runs — leaving the previous path's needle and value on screen, presented as a live reading of + * the new one. Clearing unconditionally here is wrong for the same reason: this effect re-runs on + * theme changes, which would blink the needle off and back on at every switch. + */ + private clearReadingOnRepoint(signature: string | null): void { + if (this.lastPathSignature !== null && this.lastPathSignature !== signature) { + this.dataAvailable.set(false); + this.value.set(undefined); + this.effectiveUnit.set(''); + this.pathDataState.set(null); + } + this.lastPathSignature = signature; + } constructor() { // Data subscription effect @@ -151,8 +173,11 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { const theme = this.theme(); if (!cfg || !theme) return; if (!cfg.paths?.['gaugePath'].path) return; + const signature = widgetPathSignature(cfg.paths['gaugePath']); - untracked(() => this.streams.observe('gaugePath', path => { + untracked(() => { + this.clearReadingOnRepoint(signature); + this.streams.observe('gaugePath', path => { if (path.state !== this.pathDataState()) { this.pathDataState.set((path.state as States) || null); } @@ -172,7 +197,8 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { // clamp this.value.set(Math.min(Math.max(raw, lower), upper)); } - })); + }); + }); }); // Metadata observation (idempotent) – only when zones not ignored From 4888cf759d06936365497cfa526fdb40baee8f5b Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Thu, 13 Aug 2026 00:15:21 +0300 Subject: [PATCH 2/3] fix(gauges): clear on a cleared path, and separate the first-run sentinel Review found the no-path early return ran before the clear, so emptying a widget's path tore the subscription down and left its reading on the dial -- the same lie, reached another way. The signature is now computed first and gates the observe, and `undefined` marks "never run" so a null signature is a real identity rather than a reset of that guard. Tests: re-point to a path that DOES report (the case separating "clears stale data" from "clears all data"), a cleared path, the re-point after one, zone-state reset, positive controls on the theme tests, and direct coverage for widgetPathSignature. Fixes #534 --- .../widget-streams.directive.spec.ts | 50 ++++++++++++- .../widget-gauge-ng-compass.component.spec.ts | 27 ++++++- .../widget-gauge-ng-compass.component.ts | 11 ++- .../widget-gauge-ng-linear.component.spec.ts | 28 ++++++- .../widget-gauge-ng-linear.component.ts | 10 ++- .../widget-gauge-ng-radial.component.spec.ts | 75 +++++++++++++++++++ .../widget-gauge-ng-radial.component.ts | 10 ++- 7 files changed, 196 insertions(+), 15 deletions(-) diff --git a/src/app/core/directives/widget-streams.directive.spec.ts b/src/app/core/directives/widget-streams.directive.spec.ts index 3d8cf85d..5c8fe26c 100644 --- a/src/app/core/directives/widget-streams.directive.spec.ts +++ b/src/app/core/directives/widget-streams.directive.spec.ts @@ -1,7 +1,7 @@ import { TestBed } from '@angular/core/testing'; import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; import { Subject, BehaviorSubject, Observable } from 'rxjs'; -import { WidgetStreamsDirective } from './widget-streams.directive'; +import { WidgetStreamsDirective, widgetPathSignature, normalizeWidgetPath } from './widget-streams.directive'; import { DataService, IPathUpdate } from '../services/data.service'; import { UnitsService } from '../services/units.service'; import { IWidgetSvcConfig, IWidgetPath } from '../interfaces/widgets-interface'; @@ -901,3 +901,51 @@ describe('WidgetStreamsDirective TTL value reset (#1069)', () => { expect(hits[hits.length - 1]).toBeNull(); }); }); + +/** + * The identity a widget compares to tell a re-point apart from an unrelated reconfigure. The + * directive computes it to decide whether to rebuild a subscription; the gauges compute it to decide + * whether the reading on screen still describes the path being watched. Both must agree, which is + * why it is one exported function rather than two implementations. + */ +describe('widgetPathSignature', () => { + const base = { path: 'navigation.speedOverGround', pathType: 'number', convertUnitTo: 'knots', source: null, suppressBootstrapNull: true }; + + it('returns null for a config with no usable path', () => { + expect(widgetPathSignature(undefined)).toBeNull(); + expect(widgetPathSignature(null)).toBeNull(); + expect(widgetPathSignature({ ...base, path: null })).toBeNull(); + expect(widgetPathSignature({ ...base, path: '' })).toBeNull(); + // Whitespace passes the widget-options required check, so it has to normalize to "no path" + // here rather than becoming an identity of its own. + expect(widgetPathSignature({ ...base, path: ' ' })).toBeNull(); + }); + + it('treats a trimmed path and its padded form as the same reading', () => { + expect(widgetPathSignature({ ...base, path: ' navigation.speedOverGround ' })) + .toBe(widgetPathSignature(base)); + }); + + it('treats an unset source and the default source as the same reading', () => { + expect(widgetPathSignature({ ...base, source: null })).toBe(widgetPathSignature({ ...base, source: 'default' })); + expect(widgetPathSignature({ ...base, source: ' ' })).toBe(widgetPathSignature({ ...base, source: 'default' })); + }); + + it('separates readings that differ in path, source, type, unit or bootstrap-null policy', () => { + const sig = widgetPathSignature(base); + expect(widgetPathSignature({ ...base, path: 'navigation.speedThroughWater' })).not.toBe(sig); + expect(widgetPathSignature({ ...base, source: 'gps-2' })).not.toBe(sig); + expect(widgetPathSignature({ ...base, pathType: 'string' })).not.toBe(sig); + // A unit change re-expresses the number, so the displayed reading is no longer the same one. + expect(widgetPathSignature({ ...base, convertUnitTo: 'kph' })).not.toBe(sig); + expect(widgetPathSignature({ ...base, suppressBootstrapNull: false })).not.toBe(sig); + }); + + it('normalizeWidgetPath yields undefined for anything that is not a usable path', () => { + expect(normalizeWidgetPath(' a.b ')).toBe('a.b'); + expect(normalizeWidgetPath('')).toBeUndefined(); + expect(normalizeWidgetPath(' ')).toBeUndefined(); + expect(normalizeWidgetPath(null)).toBeUndefined(); + expect(normalizeWidgetPath(42)).toBeUndefined(); + }); +}); diff --git a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts index 7bb308b7..8aae7f02 100644 --- a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts @@ -21,6 +21,8 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { let internals: CompassInternals; let capturedNext: ((u: IPathUpdate) => void) | undefined; let options: WritableSignal; + let observeCount: number; + let replayOnObserve: IPathUpdate | undefined; interface CompassInternals { value: () => number | null | undefined; @@ -45,8 +47,16 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { beforeEach(async () => { capturedNext = undefined; options = signal(makeConfig()); + observeCount = 0; + replayOnObserve = undefined; const streamsFake = { - observe(_pathName: string, next: (u: IPathUpdate) => void) { capturedNext = next; } + observe(_pathName: string, next: (u: IPathUpdate) => void) { + capturedNext = next; + observeCount++; + // The real directive replays a BehaviorSubject, so a path holding a value delivers it + // synchronously inside the same effect run as the clear. + if (replayOnObserve) next(replayOnObserve); + } }; const unitsFake = { getUnitDisplaySymbol: (measure: string | null | undefined): string => measure ?? '', @@ -114,8 +124,21 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { // The same effect re-runs on a theme change, so an unconditional clear would blink the needle // off and back on at every switch. + it('shows the new path\'s heading immediately when it has one, without surfacing the clear', () => { + capturedNext?.(update(142)); + + replayOnObserve = update(271); + options.set(makeConfig('self.navigation.headingMagnetic')); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(271); + expect(internals.textValue()).toBe('271'); + }); + it('keeps the heading when the config changes without changing the path', () => { capturedNext?.(update(142)); + const before = observeCount; fixture.componentRef.setInput('theme', { contrast: 'rgba(0,0,0,1)', contrastDim: 'rgba(60,60,60,1)', @@ -126,6 +149,8 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { }); fixture.detectChanges(); + // Positive control: the effect really did re-run, so "no clear" is a decision, not a no-op. + expect(observeCount).toBeGreaterThan(before); expect(internals.dataAvailable()).toBe(true); expect(internals.value()).toBe(142); expect(internals.textValue()).toBe('142'); diff --git a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts index a369e8ee..0a54cdd1 100644 --- a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts +++ b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts @@ -125,7 +125,7 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { private currentState = signal(States.Normal); private lastAppliedState: States | null = null; /** Identity of the path the reading state describes; null until the first subscription. */ - private lastPathSignature: string | null = null; + private lastPathSignature: string | null | undefined = undefined; /** * Drop the reading when the widget is re-pointed at another path. @@ -138,7 +138,7 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { * theme changes, which would blink the needle off and back on at every switch. */ private clearReadingOnRepoint(signature: string | null): void { - if (this.lastPathSignature !== null && this.lastPathSignature !== signature) { + if (this.lastPathSignature !== undefined && this.lastPathSignature !== signature) { this.dataAvailable.set(false); this.value.set(undefined); this.textValue.set('--'); @@ -168,10 +168,13 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { const theme = this.theme(); if (!cfg || !theme) return; const pCfg = cfg.paths?.['gaugePath']; - if (!pCfg?.path) return; + // Computed before the no-path bail-out, and null exactly when there is no usable path: the + // streams directive drops the subscription in that case, so the reading has to go with it. const signature = widgetPathSignature(pCfg); untracked(() => { this.clearReadingOnRepoint(signature); + const path = pCfg?.path; + if (!signature || !path) return; this.streams.observe('gaugePath', pkt => { let raw = (pkt?.data?.value as number) ?? null; this.dataAvailable.set(raw != null); @@ -179,7 +182,7 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { this.value.set(0); this.textValue.set('--'); } else { - if (this.negToPortPaths.includes(pCfg.path)) raw = convertNegToPortDegree(raw); + if (this.negToPortPaths.includes(path)) raw = convertNegToPortDegree(raw); const clamped = Math.min(Math.max(raw, 0), 360); this.value.set(clamped); this.textValue.set(clamped.toFixed(0)); diff --git a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts index 787aebfd..84d4c1a2 100644 --- a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts @@ -28,6 +28,8 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { let options: WritableSignal; let sizeUpdates: LinearGaugeOptions[]; let capturedNext: ((u: IPathUpdate) => void) | undefined; + let observeCount: number; + let replayOnObserve: IPathUpdate | undefined; interface LinearInternals { effectiveUnit: WritableSignal; @@ -77,12 +79,20 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { options = signal(makeConfig()); sizeUpdates = []; capturedNext = undefined; + observeCount = 0; + replayOnObserve = undefined; await TestBed.configureTestingModule({ imports: [WidgetGaugeNgLinearComponent], providers: [ { provide: WidgetRuntimeDirective, useValue: { options } }, - { provide: WidgetStreamsDirective, useValue: { observe: (_p: string, next: (u: IPathUpdate) => void) => { capturedNext = next; } } }, + { provide: WidgetStreamsDirective, useValue: { observe: (_p: string, next: (u: IPathUpdate) => void) => { + capturedNext = next; + observeCount++; + // The real directive replays a BehaviorSubject, so a path holding a value delivers it + // synchronously inside the same effect run as the clear. + if (replayOnObserve) next(replayOnObserve); + } } }, { provide: WidgetMetadataDirective, useValue: { zones: () => [], observe: () => undefined } }, { provide: UnitsService, useValue: unitsFake } ] @@ -243,13 +253,29 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { // The same effect re-runs on a theme change, so an unconditional clear would blink the bar off // and back on at every switch. + it('shows the new path\'s reading immediately when it has one, without surfacing the clear', () => { + capturedNext?.(update(6.5, 'knots')); + + replayOnObserve = update(31.2, 'm'); + options.set(makeConfig('vertical', 'self.environment.depth.belowTransducer')); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(31.2); + // The rendered header, not just the signal: this is what the user reads. + expect(fixture.nativeElement.querySelector('.gaugeUnit').textContent.trim()).toBe('m'); + }); + it('keeps the reading when the config changes without changing the path', () => { capturedNext?.(update(6.5, 'knots')); expect(internals.dataAvailable()).toBe(true); + const before = observeCount; fixture.componentRef.setInput('theme', { ...theme, cardColor: '#eee', background: '#fff' }); fixture.detectChanges(); + // Positive control: the effect really did re-run, so "no clear" is a decision, not a no-op. + expect(observeCount).toBeGreaterThan(before); expect(internals.dataAvailable()).toBe(true); expect(internals.value()).toBe(6.5); expect(internals.effectiveUnit()).toBe('knots'); diff --git a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts index 4f0ec022..87422a82 100644 --- a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts +++ b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts @@ -104,7 +104,7 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { /** Measure the incoming value was converted to (server-resolved for this display path). '' = boot placeholder. */ private effectiveUnit = signal(''); /** Identity of the path the state above describes; null until the first subscription. */ - private lastPathSignature: string | null = null; + private lastPathSignature: string | null | undefined = undefined; /** * Drop the reading when the widget is re-pointed at another path. @@ -117,7 +117,7 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { * on theme changes, which would blink the bar off and back on at every switch. */ private clearReadingOnRepoint(signature: string | null): void { - if (this.lastPathSignature !== null && this.lastPathSignature !== signature) { + if (this.lastPathSignature !== undefined && this.lastPathSignature !== signature) { this.dataAvailable.set(false); this.value.set(undefined); this.textValue.set('--'); @@ -166,11 +166,13 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { const cfg = this.runtime.options(); const theme = this.theme(); if (!cfg || !theme) return; - if (!cfg.paths?.['gaugePath'].path) return; - const signature = widgetPathSignature(cfg.paths['gaugePath']); + // Computed before the no-path bail-out, and null exactly when there is no usable path: the + // streams directive drops the subscription in that case, so the reading has to go with it. + const signature = widgetPathSignature(cfg.paths?.['gaugePath']); untracked(() => { this.clearReadingOnRepoint(signature); + if (!signature) return; this.streams.observe('gaugePath', path => { const raw = (path?.data?.value as number) ?? null; const measure = path.data.measure ?? ''; diff --git a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts index d1957274..9507a29b 100644 --- a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.spec.ts @@ -9,6 +9,7 @@ import { UnitsService } from '../../core/services/units.service'; import { IPathUpdate } from '../../core/services/data.service'; import { IWidgetSvcConfig, IPathArray } from '../../core/interfaces/widgets-interface'; import { IScale } from '../../core/utils/dataScales.util'; +import { States } from '../../core/interfaces/signalk-interfaces'; /** * Regression tests for the gauge's displayScale reinterpretation — the P2b unit-flip mechanic. @@ -33,6 +34,7 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' let capturedNext: ((u: IPathUpdate) => void) | undefined; let observeCount: number; let lastObservedPath: string; + let replayOnObserve: IPathUpdate | undefined; interface GaugeInternals { effectiveUnit: WritableSignal; @@ -41,6 +43,7 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' textValue: () => string; dataAvailable: () => boolean; optionsReady: () => boolean; + pathDataState: () => States | null; } // convertUnitTo is the stored authoring unit; a tagged measure that differs is the flip target. @@ -75,11 +78,15 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' observeCount = 0; lastObservedPath = ''; + replayOnObserve = undefined; const streamsFake = { observe(pathName: string, next: (u: IPathUpdate) => void) { lastObservedPath = pathName; capturedNext = next; observeCount++; + // The real directive's base is a BehaviorSubject, so a path that already holds a value + // replays it synchronously, inside the same effect run as the clear. + if (replayOnObserve) next(replayOnObserve); } }; const metadataFake = { zones: () => [], observe: () => undefined }; @@ -195,18 +202,86 @@ describe('WidgetGaugeNgRadialComponent displayScale reinterpretation (P2b flip)' expect(internals.effectiveUnit()).toBe(''); }); + // The clear is only correct because it runs in the same synchronous block as the resubscribe: the + // directive's base is a BehaviorSubject, so a path that already holds a value replays it at once. + // Separating the two would blank the gauge on every re-point, which is the difference between + // clearing STALE data and clearing ALL data. + it('shows the new path\'s reading immediately when it has one, without surfacing the clear', () => { + capturedNext?.(update(42, 'percent')); + expect(internals.value()).toBe(42); + + replayOnObserve = update(71, 'percent'); + options.set(makeConfig('self.test.live')); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(true); + expect(internals.value()).toBe(71); + expect(internals.textValue()).toBe(''); + }); + + // Zone colours are driven by the path's state, so carrying an old path's alarm onto a new one is + // the same lie as carrying its value. + it('clears the zone state on a re-point, so an old alarm colour cannot carry over', () => { + capturedNext?.({ data: { value: 42, timestamp: null, measure: 'percent' }, state: States.Alarm }); + expect(internals.pathDataState()).toBe(States.Alarm); + + options.set(makeConfig('self.test.silent')); + fixture.detectChanges(); + + expect(internals.pathDataState()).toBeNull(); + }); + + // Clearing the path tears the subscription down in the directive, so the reading has to go too — + // otherwise the dial keeps a number with nothing feeding it. + it('clears the reading when the path is cleared entirely', () => { + capturedNext?.(update(42, 'percent')); + expect(internals.dataAvailable()).toBe(true); + + const cleared = makeConfig(); + (cleared.paths as IPathArray)['gaugePath'].path = null; + options.set(cleared); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(false); + expect(internals.value()).toBeUndefined(); + expect(internals.textValue()).toBe('--'); + }); + + // A path-less config has no signature, so it must not read as "nothing has been shown yet" and + // suppress the clear on the re-point after it. + it('still clears on the re-point that follows a cleared path', () => { + capturedNext?.(update(42, 'percent')); + + const cleared = makeConfig(); + (cleared.paths as IPathArray)['gaugePath'].path = null; + options.set(cleared); + fixture.detectChanges(); + + options.set(makeConfig('self.test.live')); + fixture.detectChanges(); + capturedNext?.(update(7, 'percent')); + expect(internals.dataAvailable()).toBe(true); + + options.set(makeConfig('self.test.silent')); + fixture.detectChanges(); + expect(internals.dataAvailable()).toBe(false); + }); + // The same effect re-runs on a theme change, so an unconditional clear would blink the needle off // and back on at every switch. it('keeps the reading when the config changes without changing the path', () => { capturedNext?.(update(42, 'percent')); expect(internals.dataAvailable()).toBe(true); + const before = observeCount; fixture.componentRef.setInput('theme', { contrast: '#000', contrastDim: '#333', contrastDimmer: '#666', cardColor: '#eee', background: '#fff' }); fixture.detectChanges(); + // Positive control: the effect really did re-run, so "no clear" is a decision, not a no-op. + expect(observeCount).toBeGreaterThan(before); expect(internals.dataAvailable()).toBe(true); expect(internals.value()).toBe(42); expect(internals.textValue()).toBe(''); diff --git a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts index 4cbaadc1..543ba0bd 100644 --- a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts +++ b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts @@ -144,7 +144,7 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { private viewReady = signal(false); protected gaugeOptions: RadialGaugeOptions = {} as RadialGaugeOptions; /** Identity of the path the state below describes; null until the first subscription. */ - private lastPathSignature: string | null = null; + private lastPathSignature: string | null | undefined = undefined; /** * Drop the reading when the widget is re-pointed at another path. @@ -157,7 +157,7 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { * theme changes, which would blink the needle off and back on at every switch. */ private clearReadingOnRepoint(signature: string | null): void { - if (this.lastPathSignature !== null && this.lastPathSignature !== signature) { + if (this.lastPathSignature !== undefined && this.lastPathSignature !== signature) { this.dataAvailable.set(false); this.value.set(undefined); this.effectiveUnit.set(''); @@ -172,11 +172,13 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { const cfg = this.runtime.options(); const theme = this.theme(); if (!cfg || !theme) return; - if (!cfg.paths?.['gaugePath'].path) return; - const signature = widgetPathSignature(cfg.paths['gaugePath']); + // Computed before the no-path bail-out, and null exactly when there is no usable path: the + // streams directive drops the subscription in that case, so the reading has to go with it. + const signature = widgetPathSignature(cfg.paths?.['gaugePath']); untracked(() => { this.clearReadingOnRepoint(signature); + if (!signature) return; this.streams.observe('gaugePath', path => { if (path.state !== this.pathDataState()) { this.pathDataState.set((path.state as States) || null); From 80bc27f4eda2a5b53fb44be8f421cf26bcc72a06 Mon Sep 17 00:00:00 2001 From: Matti Airas Date: Thu, 13 Aug 2026 10:40:23 +0300 Subject: [PATCH 3/3] fix(gauges): cover the cleared-path and zone-state branches of the re-point Mutation testing found three branches the specs did not hold. Moving the clear below the no-path bail-out left linear and compass green; deleting their currentState reset left them green too. Both are now pinned, matching the radial. The compass matched its negative-to-port list against the raw configured path while the signature and the subscription both used the normalized one, so a padded path subscribed correctly and then clamped a negative apparent wind angle to 0 instead of converting it to a bearing. The sentinel's doc comment named the wrong value for the first-run state and left null's meaning unstated, in all three copies. The dependency on observe() receiving a fresh closure each run is now written down where it can be read. --- CLAUDE.md | 2 +- .../widget-gauge-ng-compass.component.spec.ts | 30 ++++++++++++++++++- .../widget-gauge-ng-compass.component.ts | 19 ++++++++++-- .../widget-gauge-ng-linear.component.spec.ts | 28 ++++++++++++++++- .../widget-gauge-ng-linear.component.ts | 12 +++++++- .../widget-gauge-ng-radial.component.ts | 12 +++++++- 6 files changed, 95 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6d056d86..b39fdf29 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,7 +76,7 @@ The mock serves Skip's full session/config surface (`loginStatus`, `applicationD **Runtime data pipeline** (`src/app/core/services/`): `SignalKConnectionService` (endpoint discovery) → `ConnectionStateMachine` (explicit connection lifecycle; registers callbacks so it carries no upward deps) → `SignalKDeltaService` (parses SK delta messages) → `DataService` (central hub mapping deltas to per-path observables; every value is written to both a `default` bucket and a per-`$source` bucket) → widgets. The DI graph is intentionally acyclic: `connection ← auth ← storage ← settings`, `data ← delta ← connection`. -**Widgets** (`src/app/widgets/`, ~46) are standalone components composed with three **host directives** that own runtime concerns: `WidgetRuntimeDirective` (config merge), `WidgetStreamsDirective` (diff-based path subscriptions), `WidgetMetadataDirective` (zones/meta). `WidgetService` is the registry — `kipWidgets` is a getter over the `_widgetDefinition` array; the full electrical family (bms, solar-charger, charger, alternator, inverter, ac) is registered and live. +**Widgets** (`src/app/widgets/`, ~46) are standalone components composed with three **host directives** that own runtime concerns: `WidgetRuntimeDirective` (config merge), `WidgetStreamsDirective` (diff-based path subscriptions), `WidgetMetadataDirective` (zones/meta). A widget that holds stream-derived presentation state owns clearing it: `WidgetStreamsDirective` rebuilds the subscription on a re-point, but `suppressBootstrapNull: true` filters the replayed leading null, so against a path that reports nothing the callback never runs and the previous path's reading stays on screen as a live reading of the new one. Compare `widgetPathSignature()` across effect runs and clear on a change — the three ng-gauges do; the other `streams.observe` callers do not yet (#585). `WidgetService` is the registry — `kipWidgets` is a getter over the `_widgetDefinition` array; the full electrical family (bms, solar-charger, charger, alternator, inverter, ac) is registered and live. **Config & persistence**: `SettingsService` holds in-memory config plus sync getters and observable getters. `StorageService` persists through the SK server's **applicationData REST API**, which has exactly two scopes — `user` (the authenticated user's private store) and `global` (a single shared bucket). All writes go through a **sequential JSON-Patch queue** because SK can't handle concurrent applicationData writes. `ConfigurationUpgradeService` migrates older config file versions (preserve the stored version on write). diff --git a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts index 8aae7f02..9c69a756 100644 --- a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.spec.ts @@ -7,6 +7,7 @@ import { WidgetStreamsDirective } from '../../core/directives/widget-streams.dir import { UnitsService } from '../../core/services/units.service'; import { IPathUpdate } from '../../core/services/data.service'; import { IWidgetSvcConfig, IPathArray } from '../../core/interfaces/widgets-interface'; +import { States } from '../../core/interfaces/signalk-interfaces'; /** * A compass with no heading must still show its rose, with no needle and a '--' readout — a needle @@ -28,11 +29,12 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { value: () => number | null | undefined; textValue: () => string; dataAvailable: () => boolean; + currentState: () => string; optionsReady: () => boolean; gaugeOptions: { needle?: boolean }; } - const makeConfig = (path = 'self.navigation.headingTrue'): IWidgetSvcConfig => { + const makeConfig = (path: string | null = 'self.navigation.headingTrue'): IWidgetSvcConfig => { const dflt = WidgetGaugeNgCompassComponent.DEFAULT_CONFIG; const gaugePath = (dflt.paths as IPathArray)['gaugePath']; return { @@ -122,6 +124,32 @@ describe('WidgetGaugeNgCompassComponent no-data state', () => { expect(internals.textValue()).toBe('--'); }); + // Clearing the path entirely drops the subscription, so the heading has to go with it. This is + // what pins the clear ahead of the no-path bail-out rather than after it. + it('clears the heading when the path is cleared entirely', () => { + capturedNext?.(update(142)); + expect(internals.dataAvailable()).toBe(true); + + options.set(makeConfig(null)); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(false); + expect(internals.value()).toBeUndefined(); + expect(internals.textValue()).toBe('--'); + }); + + // currentState colours the heading text independently of dataAvailable, so without the reset an + // alarm-red readout from the old path survives onto the new one. + it('clears the zone state on a re-point', () => { + capturedNext?.({ ...update(142), state: States.Alarm } as IPathUpdate); + expect(internals.currentState()).toBe(States.Alarm); + + options.set(makeConfig('self.navigation.headingMagnetic')); + fixture.detectChanges(); + + expect(internals.currentState()).toBe(States.Normal); + }); + // The same effect re-runs on a theme change, so an unconditional clear would blink the needle // off and back on at every switch. it('shows the new path\'s heading immediately when it has one, without surfacing the clear', () => { diff --git a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts index 0a54cdd1..c501b9b3 100644 --- a/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts +++ b/src/app/widgets/widget-gauge-ng-compass/widget-gauge-ng-compass.component.ts @@ -15,7 +15,7 @@ import { States } from '../../core/interfaces/signalk-interfaces'; import { getColors } from '../../core/utils/themeColors.utils'; import { SkipResizeObserverDirective } from '../../core/directives/skip-resize-observer.directive'; import { WidgetRuntimeDirective } from '../../core/directives/widget-runtime.directive'; -import { WidgetStreamsDirective, widgetPathSignature } from '../../core/directives/widget-streams.directive'; +import { WidgetStreamsDirective, widgetPathSignature, normalizeWidgetPath } from '../../core/directives/widget-streams.directive'; import { ITheme } from '../../core/services/app-service'; import { UnitsService } from '../../core/services/units.service'; @@ -124,7 +124,12 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { protected colorStrokeTicks = ''; private currentState = signal(States.Normal); private lastAppliedState: States | null = null; - /** Identity of the path the reading state describes; null until the first subscription. */ + /** + * Identity of the path the reading state describes. Three states: `undefined` before the first + * effect run (nothing has been shown, so there is nothing to clear), `null` for a config with no + * usable path, and the signature otherwise. `null` is a real identity rather than a second + * "not yet" — a cleared path must still compare unequal to the path that follows it. + */ private lastPathSignature: string | null | undefined = undefined; /** @@ -136,6 +141,11 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { * runs — leaving the previous path's needle and value on screen, presented as a live reading of * the new one. Clearing unconditionally here is wrong for the same reason: this effect re-runs on * theme changes, which would blink the needle off and back on at every switch. + * + * The reading comes back because `observe()` below is passed a new closure on every effect run: + * the directive compares callback identity as well as the signature, so it rebuilds and replays + * the new path's value into this component. A stable callback reference would make that + * early-return instead, and the gauge would stay blank on a live path until the next delta. */ private clearReadingOnRepoint(signature: string | null): void { if (this.lastPathSignature !== undefined && this.lastPathSignature !== signature) { @@ -173,7 +183,10 @@ export class WidgetGaugeNgCompassComponent implements AfterViewInit { const signature = widgetPathSignature(pCfg); untracked(() => { this.clearReadingOnRepoint(signature); - const path = pCfg?.path; + // Normalized, like the signature and the subscription: the widget-options required check + // accepts a padded path, which would subscribe fine and then miss this list, clamping a + // negative apparent wind angle to 0 instead of converting it to its 0-360 bearing. + const path = normalizeWidgetPath(pCfg?.path); if (!signature || !path) return; this.streams.observe('gaugePath', pkt => { let raw = (pkt?.data?.value as number) ?? null; diff --git a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts index 84d4c1a2..47056cea 100644 --- a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts +++ b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.spec.ts @@ -50,7 +50,7 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { cardColor: '#111', background: '#000' }; - const makeConfig = (subType = 'vertical', path = 'self.navigation.speedOverGround'): IWidgetSvcConfig => { + const makeConfig = (subType = 'vertical', path: string | null = 'self.navigation.speedOverGround'): IWidgetSvcConfig => { const dflt = WidgetGaugeNgLinearComponent.DEFAULT_CONFIG; const gaugePath = (dflt.paths as IPathArray)['gaugePath']; return { @@ -266,6 +266,32 @@ describe('WidgetGaugeNgLinearComponent header row and sizing', () => { expect(fixture.nativeElement.querySelector('.gaugeUnit').textContent.trim()).toBe('m'); }); + // Clearing the path entirely drops the subscription, so the reading has to go with it. This is + // what pins the clear ahead of the no-path bail-out rather than after it. + it('clears the reading when the path is cleared entirely', () => { + capturedNext?.(update(6.5, 'knots')); + expect(internals.dataAvailable()).toBe(true); + + options.set(makeConfig('vertical', null)); + fixture.detectChanges(); + + expect(internals.dataAvailable()).toBe(false); + expect(internals.value()).toBeUndefined(); + expect(internals.textValue()).toBe('--'); + }); + + // currentState drives the value text's colour independently of dataAvailable, so without the + // reset an alarm-red readout from the old path survives onto the new one. + it('clears the zone state on a re-point', () => { + capturedNext?.({ ...update(6.5, 'knots'), state: States.Alarm }); + expect(internals.currentState()).toBe(States.Alarm); + + options.set(makeConfig('vertical', 'self.environment.depth.belowTransducer')); + fixture.detectChanges(); + + expect(internals.currentState()).toBe(States.Normal); + }); + it('keeps the reading when the config changes without changing the path', () => { capturedNext?.(update(6.5, 'knots')); expect(internals.dataAvailable()).toBe(true); diff --git a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts index 87422a82..0c7df2ca 100644 --- a/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts +++ b/src/app/widgets/widget-gauge-ng-linear/widget-gauge-ng-linear.component.ts @@ -103,7 +103,12 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { private currentState = signal(States.Normal); /** Measure the incoming value was converted to (server-resolved for this display path). '' = boot placeholder. */ private effectiveUnit = signal(''); - /** Identity of the path the state above describes; null until the first subscription. */ + /** + * Identity of the path the reading state describes. Three states: `undefined` before the first + * effect run (nothing has been shown, so there is nothing to clear), `null` for a config with no + * usable path, and the signature otherwise. `null` is a real identity rather than a second + * "not yet" — a cleared path must still compare unequal to the path that follows it. + */ private lastPathSignature: string | null | undefined = undefined; /** @@ -115,6 +120,11 @@ export class WidgetGaugeNgLinearComponent implements AfterViewInit { * runs — leaving the previous path's bar, value and unit on screen, presented as a live reading * of the new one. Clearing unconditionally here is wrong for the same reason: this effect re-runs * on theme changes, which would blink the bar off and back on at every switch. + * + * The reading comes back because `observe()` below is passed a new closure on every effect run: + * the directive compares callback identity as well as the signature, so it rebuilds and replays + * the new path's value into this component. A stable callback reference would make that + * early-return instead, and the gauge would stay blank on a live path until the next delta. */ private clearReadingOnRepoint(signature: string | null): void { if (this.lastPathSignature !== undefined && this.lastPathSignature !== signature) { diff --git a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts index 543ba0bd..1e2418a6 100644 --- a/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts +++ b/src/app/widgets/widget-gauge-ng-radial/widget-gauge-ng-radial.component.ts @@ -143,7 +143,12 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { private pathDataState = signal(null); private viewReady = signal(false); protected gaugeOptions: RadialGaugeOptions = {} as RadialGaugeOptions; - /** Identity of the path the state below describes; null until the first subscription. */ + /** + * Identity of the path the reading state describes. Three states: `undefined` before the first + * effect run (nothing has been shown, so there is nothing to clear), `null` for a config with no + * usable path, and the signature otherwise. `null` is a real identity rather than a second + * "not yet" — a cleared path must still compare unequal to the path that follows it. + */ private lastPathSignature: string | null | undefined = undefined; /** @@ -155,6 +160,11 @@ export class WidgetGaugeNgRadialComponent implements AfterViewInit { * runs — leaving the previous path's needle and value on screen, presented as a live reading of * the new one. Clearing unconditionally here is wrong for the same reason: this effect re-runs on * theme changes, which would blink the needle off and back on at every switch. + * + * The reading comes back because `observe()` below is passed a new closure on every effect run: + * the directive compares callback identity as well as the signature, so it rebuilds and replays + * the new path's value into this component. A stable callback reference would make that + * early-return instead, and the gauge would stay blank on a live path until the next delta. */ private clearReadingOnRepoint(signature: string | null): void { if (this.lastPathSignature !== undefined && this.lastPathSignature !== signature) {