Skip to content

Commit 968606f

Browse files
Traffic Light: address review — bounds clearing, request/limits guards, limits subscription, raw-value limit checks
- Clear the displayed datum on non-tick bounds changes so an empty window shows gray/no data instead of an out-of-window alarm - Guard async request and limits results by request id + telemetry identity so a removed child cannot overwrite its replacement - Subscribe to runtime limit changes via telemetry.subscribeToLimits - Compare limits against the parsed raw value, not the formatted text - Bind to the first non-string range value, matching hasNumericTelemetry - Treat evaluator results with no cssClass/name as nominal - Drop redundant TrafficLight install in spec (already default-installed) Co-Authored-By: Samantha Taylor <samantha.taylor@cognition.ai>
1 parent 664d27a commit 968606f

3 files changed

Lines changed: 112 additions & 20 deletions

File tree

src/plugins/trafficLight/components/TrafficLightComponent.vue

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ const STATUS_LABELS = {
5050
[STATUS.GRAY]: 'No data'
5151
};
5252
53+
function isNumericText(value) {
54+
return value !== '' && value != null && !Number.isNaN(Number(value));
55+
}
56+
5357
export default {
5458
mixins: [stalenessMixin],
5559
inject: ['openmct', 'domainObject', 'composition', 'renderWhenVisible'],
@@ -165,8 +169,10 @@ export default {
165169
return;
166170
}
167171
172+
const rangeValues = this.metadata.valuesForHints(['range']);
173+
168174
this.formats = this.openmct.telemetry.getFormatMap(this.metadata);
169-
this.valueMetadata = this.metadata.valuesForHints(['range'])[0];
175+
this.valueMetadata = rangeValues.find((value) => value.format !== 'string') ?? rangeValues[0];
170176
this.valueKey = this.valueMetadata?.key;
171177
this.valueSource = this.valueMetadata?.source ?? this.valueKey;
172178
this.units = this.valueMetadata?.unit ?? '';
@@ -175,21 +181,36 @@ export default {
175181
this.openmct.telemetry
176182
.getLimits(domainObject)
177183
.limits()
178-
.then(this.updateLimitDefinition)
184+
.then((limitDefinition) => {
185+
this.updateLimitDefinition(domainObject, limitDefinition);
186+
})
179187
.catch(() => {
180-
this.limitDefinition = undefined;
188+
this.updateLimitDefinition(domainObject, undefined);
181189
});
190+
this.unsubscribeLimits = this.openmct.telemetry.subscribeToLimits(
191+
domainObject,
192+
(limitDefinition) => {
193+
this.updateLimitDefinition(domainObject, limitDefinition);
194+
}
195+
);
182196
183197
this.request();
184198
this.subscribe();
185199
this.subscribeToStaleness(domainObject);
186200
},
187201
removeTelemetryObject() {
202+
this.requestId = (this.requestId ?? 0) + 1;
203+
188204
if (this.unsubscribe) {
189205
this.unsubscribe();
190206
this.unsubscribe = undefined;
191207
}
192208
209+
if (this.unsubscribeLimits) {
210+
this.unsubscribeLimits();
211+
this.unsubscribeLimits = undefined;
212+
}
213+
193214
if (this.telemetryObject) {
194215
this.triggerUnsubscribeFromStaleness(this.telemetryObject);
195216
}
@@ -200,38 +221,51 @@ export default {
200221
this.valueMetadata = undefined;
201222
this.limitEvaluator = undefined;
202223
this.limitDefinition = undefined;
203-
this.limitEvaluation = undefined;
204224
this.valueKey = undefined;
205225
this.valueSource = undefined;
206226
this.units = '';
227+
this.clearValue();
228+
},
229+
clearValue() {
207230
this.datum = undefined;
208231
this.curVal = DEFAULT_CURRENT_VALUE;
209232
this.rawValue = undefined;
233+
this.limitEvaluation = undefined;
210234
this.hasData = false;
211235
},
212236
refreshData(bounds, isTick) {
213-
if (!isTick && this.telemetryObject) {
214-
this.request();
237+
if (isTick || !this.telemetryObject) {
238+
return;
215239
}
240+
241+
this.clearValue();
242+
this.request();
216243
},
217244
request() {
245+
const requestedObject = this.telemetryObject;
246+
const requestId = (this.requestId ?? 0) + 1;
218247
const options = {
219248
strategy: 'latest',
220249
size: 1,
221250
timeContext: this.openmct.time.getContextForView([])
222251
};
223252
224-
this.openmct.telemetry.request(this.telemetryObject, options).then((values) => {
225-
if (values && values.length) {
253+
this.requestId = requestId;
254+
this.openmct.telemetry.request(requestedObject, options).then((values) => {
255+
const isCurrent = requestId === this.requestId && requestedObject === this.telemetryObject;
256+
257+
if (isCurrent && values && values.length) {
226258
this.updateValue(values[values.length - 1]);
227259
}
228260
});
229261
},
230262
subscribe() {
231263
this.unsubscribe = this.openmct.telemetry.subscribe(this.telemetryObject, this.updateValue);
232264
},
233-
updateLimitDefinition(limitDefinition) {
234-
this.limitDefinition = limitDefinition;
265+
updateLimitDefinition(domainObject, limitDefinition) {
266+
if (domainObject === this.telemetryObject) {
267+
this.limitDefinition = limitDefinition;
268+
}
235269
},
236270
updateConfiguration(configuration) {
237271
if (!configuration) {
@@ -266,16 +300,23 @@ export default {
266300
this.isRendering = true;
267301
this.renderWhenVisible(() => {
268302
this.isRendering = false;
269-
this.render(this.datum);
303+
304+
if (this.datum) {
305+
this.render(this.datum);
306+
}
270307
});
271308
},
272309
render(datum) {
273310
const formatter = this.formats?.[this.valueKey] ?? this.formats?.[this.valueSource];
274-
const formatted = formatter ? formatter.format(datum) : datum[this.valueSource];
275-
const numeric = formatted === '' || formatted == null ? NaN : Number(formatted);
276-
277-
this.rawValue = Number.isNaN(numeric) ? undefined : numeric;
278-
this.curVal = Number.isNaN(numeric) ? String(formatted) : this.formatNumber(numeric);
311+
const raw = formatter ? formatter.parse(datum) : datum[this.valueSource];
312+
const formatted = formatter ? formatter.format(datum) : raw;
313+
const numeric = typeof raw === 'number' && Number.isFinite(raw) ? raw : undefined;
314+
315+
this.rawValue = numeric;
316+
this.curVal =
317+
numeric !== undefined && isNumericText(formatted)
318+
? this.formatNumber(numeric)
319+
: String(formatted ?? DEFAULT_CURRENT_VALUE);
279320
this.limitEvaluation = this.limitEvaluator?.evaluate(datum, this.valueMetadata);
280321
this.hasData = true;
281322
},

src/plugins/trafficLight/pluginSpec.js

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ describe('Traffic Light plugin', () => {
103103
openmct.on('start', done);
104104

105105
openmct.install(openmct.plugins.example.Generator());
106-
openmct.install(openmct.plugins.TrafficLight());
107106

108107
openmct.startHeadless();
109108
});
@@ -211,6 +210,12 @@ describe('Traffic Light plugin', () => {
211210
);
212211
});
213212

213+
it('treats evaluator results without a class or name as nominal', () => {
214+
expect(statusFromLimitEvaluation({})).toBeUndefined();
215+
expect(statusFromLimitEvaluation({ cssClass: '', name: '' })).toBeUndefined();
216+
expect(statusFromLimitEvaluation({ cssClass: 'is-limit--yellow' })).toEqual(STATUS.YELLOW);
217+
});
218+
214219
it('is gray without data or when stale, and green without limits', () => {
215220
expect(resolveStatus({ hasData: false, isStale: false })).toEqual(STATUS.GRAY);
216221
expect(resolveStatus({ hasData: true, isStale: true, value: 0.95 })).toEqual(STATUS.GRAY);
@@ -221,13 +226,17 @@ describe('Traffic Light plugin', () => {
221226
describe('Traffic Light view', () => {
222227
let trafficLightView;
223228
let subscriptionCallback;
229+
let limitsCallback;
224230
let unsubscribeSpy;
231+
let unsubscribeLimitsSpy;
225232
let requestedValue;
226233

227234
beforeEach(() => {
228235
requestedValue = 0.1;
229236
subscriptionCallback = undefined;
237+
limitsCallback = undefined;
230238
unsubscribeSpy = jasmine.createSpy('unsubscribe');
239+
unsubscribeLimitsSpy = jasmine.createSpy('unsubscribeLimits');
231240

232241
const testObjectProvider = jasmine.createSpyObj('testObjectProvider', [
233242
'get',
@@ -252,13 +261,18 @@ describe('Traffic Light plugin', () => {
252261
openmct.objects.addProvider('test-namespace', testObjectProvider);
253262

254263
spyOn(openmct.telemetry, 'request').and.callFake(() =>
255-
Promise.resolve([sineDatum(requestedValue)])
264+
Promise.resolve(requestedValue === undefined ? [] : [sineDatum(requestedValue)])
256265
);
257266
spyOn(openmct.telemetry, 'subscribe').and.callFake((domainObject, callback) => {
258267
subscriptionCallback = callback;
259268

260269
return unsubscribeSpy;
261270
});
271+
spyOn(openmct.telemetry, 'subscribeToLimits').and.callFake((domainObject, callback) => {
272+
limitsCallback = callback;
273+
274+
return unsubscribeLimitsSpy;
275+
});
262276
spyOn(openmct.telemetry, 'subscribeToStaleness').and.returnValue(() => {});
263277
spyOn(openmct.telemetry, 'isStale').and.returnValue(Promise.resolve({ isStale: false }));
264278
spyOn(openmct.time, 'getBounds').and.returnValue(TIME_BOUNDS);
@@ -308,6 +322,12 @@ describe('Traffic Light plugin', () => {
308322
return nextTick();
309323
}
310324

325+
function changeBounds() {
326+
openmct.time.emit('boundsChanged', TIME_BOUNDS, false);
327+
328+
return new Promise((resolve) => setTimeout(resolve, 20)).then(nextTick);
329+
}
330+
311331
it('renders the light and the latest formatted value', () => {
312332
expect(holder.querySelectorAll('.js-traffic-light').length).toBe(1);
313333
expect(lightElement()).not.toBeNull();
@@ -349,10 +369,40 @@ describe('Traffic Light plugin', () => {
349369
expect(holder.querySelector('.js-traffic-light-value').textContent).toContain('-0.20');
350370
});
351371

372+
it('clears the light when new bounds contain no data', async () => {
373+
await emit(0.95);
374+
expect(lightElement().classList).toContain('is-red');
375+
376+
requestedValue = undefined;
377+
await changeBounds();
378+
379+
expect(lightElement().classList).toContain('is-gray');
380+
expect(holder.querySelector('.js-traffic-light-value').textContent).toContain('--');
381+
});
382+
383+
it('re-requests the latest in-bounds value when bounds change', async () => {
384+
requestedValue = 0.6;
385+
await changeBounds();
386+
387+
expect(lightElement().classList).toContain('is-yellow');
388+
expect(holder.querySelector('.js-traffic-light-value').textContent).toContain('0.60');
389+
});
390+
391+
it('applies limit definitions pushed by the limits subscription', async () => {
392+
expect(openmct.telemetry.subscribeToLimits).toHaveBeenCalled();
393+
expect(lightElement().classList).toContain('is-green');
394+
395+
limitsCallback({ CRITICAL: { high: { sin: 0.05 } } });
396+
await nextTick();
397+
398+
expect(lightElement().classList).toContain('is-red');
399+
});
400+
352401
it('cleans up subscriptions on destroy', () => {
353402
trafficLightView.destroy();
354403

355404
expect(unsubscribeSpy).toHaveBeenCalled();
405+
expect(unsubscribeLimitsSpy).toHaveBeenCalled();
356406

357407
trafficLightView = { destroy() {} };
358408
});

src/plugins/trafficLight/traffic-light-status.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,14 @@ function containsToken(text, tokens) {
4444

4545
/**
4646
* Map the result of a LimitEvaluator's evaluate() call to a traffic light status.
47-
* Any limit violation whose severity cannot be recognized is treated as a warning.
47+
* A result without a cssClass or name is treated as nominal; any other limit
48+
* violation whose severity cannot be recognized is treated as a warning.
4849
* @param {{cssClass?: string, name?: string} | undefined} limit
4950
* @returns {string | undefined} STATUS.RED, STATUS.YELLOW, or undefined when the
5051
* evaluation carried no limit violation
5152
*/
5253
export function statusFromLimitEvaluation(limit) {
53-
if (!limit) {
54+
if (!limit || (!limit.cssClass && !limit.name)) {
5455
return undefined;
5556
}
5657

0 commit comments

Comments
 (0)