From 79c27c43b8bba1e843c646d5c63c932d413a60cc Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Mon, 1 Sep 2025 13:29:04 -0700 Subject: [PATCH] fix: Don't incorrectly change camera when a dependency supports media (#2171) - Closes: #2122 --- src/camera-manager/capabilities.ts | 4 +- src/camera-manager/store.ts | 79 +++++++++++++++------- src/camera-manager/types.ts | 5 +- src/card-controller/view/query-executor.ts | 4 +- src/view/view-to-cameras.ts | 17 +++-- tests/camera-manager/store.test.ts | 31 +++++++++ tests/view/view-to-cameras.test.ts | 37 +++++++--- 7 files changed, 130 insertions(+), 47 deletions(-) diff --git a/src/camera-manager/capabilities.ts b/src/camera-manager/capabilities.ts index cb3e07be..b8da5ec2 100644 --- a/src/camera-manager/capabilities.ts +++ b/src/camera-manager/capabilities.ts @@ -4,7 +4,7 @@ import { capabilityKeys, PTZCapabilities, } from '../types'; -import { CapabilitySearchOptions } from './types'; +import { CapabilitySearchKeys } from './types'; export class Capabilities { private _capabilities: CapabilitiesRaw; @@ -32,7 +32,7 @@ export class Capabilities { delete this._capabilities[capability]; } - public matches(capability: CapabilitySearchOptions): boolean { + public matches(capability: CapabilitySearchKeys): boolean { let result = true; if (typeof capability === 'string') { result &&= this.has(capability); diff --git a/src/camera-manager/store.ts b/src/camera-manager/store.ts index 468b9b23..061c82ef 100644 --- a/src/camera-manager/store.ts +++ b/src/camera-manager/store.ts @@ -4,7 +4,7 @@ import { allPromises } from '../utils/basic'; import { ViewMedia } from '../view/item'; import { Camera } from './camera'; import { CameraManagerEngine } from './engine'; -import { CapabilitySearchOptions, Engine } from './types'; +import { CapabilitySearchKeys, CapabilitySearchOptions, Engine } from './types'; type CameraManagerEngineCameraIDMap = Map>; @@ -27,11 +27,13 @@ export interface CameraManagerReadOnlyConfigStore { getDefaultCameraID(): string | null; getCameraIDsWithCapability( - capability: CapabilityKey | CapabilitySearchOptions, + capability: CapabilitySearchKeys, + options?: CapabilitySearchOptions, ): Set; getAllDependentCameras( cameraID: string, - capability?: CapabilityKey | CapabilitySearchOptions, + capability?: CapabilitySearchKeys, + options?: CapabilitySearchOptions, ): Set; } @@ -121,14 +123,23 @@ export class CameraManagerStore implements CameraManagerReadOnlyConfigStore { } public getCameraIDsWithCapability( - capability: CapabilityKey | CapabilitySearchOptions, + capability: CapabilityKey | CapabilitySearchKeys, + options?: CapabilitySearchOptions, ): Set { const output: Set = new Set(); + for (const camera of this._cameras.values()) { - if (camera.getCapabilities()?.matches(capability)) { - output.add(camera.getID()); - } + // Must use getAllDependentCameras() to recursively get all relevant + // cameras respecting the capabilitiy. + // See: https://github.com/dermotduffy/advanced-camera-card/issues/2122 + + this.getAllDependentCameras(camera.getID(), capability, options).forEach( + (cameraID) => { + output.add(cameraID); + }, + ); } + return output; } @@ -177,33 +188,51 @@ export class CameraManagerStore implements CameraManagerReadOnlyConfigStore { */ public getAllDependentCameras( cameraID: string, - capability?: CapabilitySearchOptions, + capabilitySearchKeys?: CapabilitySearchKeys, + options?: CapabilitySearchOptions, ): Set { const visitedCameraIDs = new Set(); - const matchingCameraIDs: Set = new Set(); - const getDependentCameras = (cameraID: string): void => { + + const getDependentCameras = (cameraID: string): Set => { visitedCameraIDs.add(cameraID); + const matchingCameraIDs: Set = new Set(); + const camera = this.getCamera(cameraID); const cameraConfig = camera?.getConfig(); - if (camera && cameraConfig) { - if (!capability || camera.getCapabilities()?.matches(capability)) { - matchingCameraIDs.add(cameraID); - } - const dependentCameras: Set = new Set(); - cameraConfig.dependencies.cameras.forEach((item) => dependentCameras.add(item)); - if (cameraConfig.dependencies.all_cameras) { - this.getCameraIDs().forEach((cameraID) => dependentCameras.add(cameraID)); - } - for (const dependentCameraID of dependentCameras) { - if (!visitedCameraIDs.has(dependentCameraID)) { - getDependentCameras(dependentCameraID); - } + if (!camera || !cameraConfig) { + return matchingCameraIDs; + } + + // Gather all dependent cameras... + const dependentCameras: Set = new Set(); + cameraConfig.dependencies.cameras.forEach((item) => dependentCameras.add(item)); + if (cameraConfig.dependencies.all_cameras) { + this.getCameraIDs().forEach((cameraID) => dependentCameras.add(cameraID)); + } + + const matchingChildCameraIDs: Set = new Set(); + + // ...now recurse through them. + for (const dependentCameraID of dependentCameras) { + if (!visitedCameraIDs.has(dependentCameraID)) { + getDependentCameras(dependentCameraID).forEach((dependentCameraID) => + matchingChildCameraIDs.add(dependentCameraID), + ); } } + + return new Set([ + ...(!capabilitySearchKeys || + camera.getCapabilities()?.matches(capabilitySearchKeys) || + (options?.inclusive && matchingChildCameraIDs.size) + ? [cameraID] + : []), + ...matchingChildCameraIDs, + ]); }; - getDependentCameras(cameraID); - return matchingCameraIDs; + + return getDependentCameras(cameraID); } } diff --git a/src/camera-manager/types.ts b/src/camera-manager/types.ts index e3534254..d36c388a 100644 --- a/src/camera-manager/types.ts +++ b/src/camera-manager/types.ts @@ -99,7 +99,10 @@ interface CapabilitySearchAllAny { allCapabilities?: CapabilityKey[]; anyCapabilities?: CapabilityKey[]; } -export type CapabilitySearchOptions = CapabilityKey | CapabilitySearchAllAny; +export type CapabilitySearchKeys = CapabilityKey | CapabilitySearchAllAny; +export interface CapabilitySearchOptions { + inclusive?: boolean; +} export interface CameraManagerCameraMetadata { title: string; diff --git a/src/card-controller/view/query-executor.ts b/src/card-controller/view/query-executor.ts index ce605350..9686e2a1 100644 --- a/src/card-controller/view/query-executor.ts +++ b/src/card-controller/view/query-executor.ts @@ -1,4 +1,4 @@ -import { CapabilitySearchOptions, MediaQuery } from '../../camera-manager/types'; +import { CapabilitySearchKeys, MediaQuery } from '../../camera-manager/types'; import { MEDIA_CHUNK_SIZE_DEFAULT } from '../../const'; import { ClipsOrSnapshotsOrAll } from '../../types'; import { findBestMediaTimeIndex } from '../../utils/find-best-media-time-index'; @@ -27,7 +27,7 @@ export class QueryExecutor { eventsMediaType?: ClipsOrSnapshotsOrAll; executorOptions?: QueryExecutorOptions; }): Promise { - const capabilitySearch: CapabilitySearchOptions = + const capabilitySearch: CapabilitySearchKeys = !options?.eventsMediaType || options?.eventsMediaType === 'all' ? { anyCapabilities: ['clips', 'snapshots'], diff --git a/src/view/view-to-cameras.ts b/src/view/view-to-cameras.ts index f8bdf3d2..4cc66282 100644 --- a/src/view/view-to-cameras.ts +++ b/src/view/view-to-cameras.ts @@ -11,10 +11,6 @@ export const getCameraIDsForViewName = ( viewName: AdvancedCameraCardView, cameraID?: string, ): Set => { - const capabilityMatchAnyMedia: CapabilitySearchOptions = { - anyCapabilities: ['clips', 'snapshots', 'recordings'], - }; - switch (viewName) { case 'diagnostics': case 'image': @@ -30,6 +26,9 @@ export const getCameraIDsForViewName = ( case 'snapshots': case 'recording': case 'recordings': + const options: CapabilitySearchOptions = { + inclusive: viewName !== 'live', + }; const capability = viewName === 'clip' ? 'clips' @@ -39,12 +38,12 @@ export const getCameraIDsForViewName = ( ? 'recordings' : viewName; return cameraID - ? cameraManager.getStore().getAllDependentCameras(cameraID, capability) - : cameraManager.getStore().getCameraIDsWithCapability(capability); + ? cameraManager.getStore().getAllDependentCameras(cameraID, capability, options) + : cameraManager.getStore().getCameraIDsWithCapability(capability, options); case 'timeline': - return cameraManager - .getStore() - .getCameraIDsWithCapability(capabilityMatchAnyMedia); + return cameraManager.getStore().getCameraIDsWithCapability({ + anyCapabilities: ['clips', 'snapshots', 'recordings'], + }); } }; diff --git a/tests/camera-manager/store.test.ts b/tests/camera-manager/store.test.ts index 54469c7a..d4ba7e7a 100644 --- a/tests/camera-manager/store.test.ts +++ b/tests/camera-manager/store.test.ts @@ -297,6 +297,37 @@ describe('CameraManagerStore', async () => { ); expect(store.getAllDependentCameras('one', 'clips')).toEqual(new Set(['two'])); }); + + it('should return cameras with specific capabilities inclusive of parent', () => { + const store = new CameraManagerStore(); + store.addCamera( + new Camera( + createCameraConfig({ + id: 'one', + dependencies: { + all_cameras: true, + }, + }), + engineGeneric, + ), + ); + store.addCamera( + new Camera( + createCameraConfig({ + id: 'two', + }), + engineGeneric, + { + capabilities: new Capabilities({ + clips: true, + }), + }, + ), + ); + expect(store.getAllDependentCameras('one', 'clips', { inclusive: true })).toEqual( + new Set(['one', 'two']), + ); + }); }); it('getCameraIDsWithCapability', () => { diff --git a/tests/view/view-to-cameras.test.ts b/tests/view/view-to-cameras.test.ts index e4af49b4..7d5f02bb 100644 --- a/tests/view/view-to-cameras.test.ts +++ b/tests/view/view-to-cameras.test.ts @@ -44,15 +44,9 @@ describe('getCameraIDsForViewName', () => { describe('views that respect dependencies and need a capability', () => { it.each([ ['live' as const, 'live' as const], - ['clip' as const, 'clips' as const], - ['clips' as const, 'clips' as const], - ['snapshot' as const, 'snapshots' as const], - ['snapshots' as const, 'snapshots' as const], - ['recording' as const, 'recordings' as const], - ['recordings' as const, 'recordings' as const], ['timeline' as const, 'clips' as const], - ['timeline' as const, 'snapshots' as const], ['timeline' as const, 'recordings' as const], + ['timeline' as const, 'snapshots' as const], ])('%s', (viewName: AdvancedCameraCardView, capabilityKey: CapabilityKey) => { const cameraManager = createCameraManager(); vi.mocked(cameraManager.getStore).mockReturnValue( @@ -71,8 +65,35 @@ describe('getCameraIDsForViewName', () => { expect(getCameraIDsForViewName(cameraManager, viewName)).toEqual( new Set(['camera-2']), ); + }); + + it.each([ + ['clip' as const, 'clips' as const], + ['clips' as const, 'clips' as const], + ['snapshot' as const, 'snapshots' as const], + ['snapshots' as const, 'snapshots' as const], + ['recording' as const, 'recordings' as const], + ['recordings' as const, 'recordings' as const], + ])('%s', (viewName: AdvancedCameraCardView, capabilityKey: CapabilityKey) => { + const cameraManager = createCameraManager(); + vi.mocked(cameraManager.getStore).mockReturnValue( + createStore([ + { + cameraID: 'camera-1', + config: createCameraConfig({ dependencies: { cameras: ['camera-2'] } }), + }, + { + cameraID: 'camera-2', + capabilities: createCapabilities({ [capabilityKey]: true }), + }, + ]), + ); + + expect(getCameraIDsForViewName(cameraManager, viewName)).toEqual( + new Set(['camera-1', 'camera-2']), + ); expect(getCameraIDsForViewName(cameraManager, viewName, 'camera-1')).toEqual( - new Set(['camera-2']), + new Set(['camera-1', 'camera-2']), ); expect(getCameraIDsForViewName(cameraManager, viewName, 'camera-2')).toEqual( new Set(['camera-2']),