diff --git a/src/card-controller/actions/actions/effect.ts b/src/card-controller/actions/actions/effect.ts index 9e28a397..b0fe35a4 100644 --- a/src/card-controller/actions/actions/effect.ts +++ b/src/card-controller/actions/actions/effect.ts @@ -9,13 +9,21 @@ export class EffectAction extends AdvancedCameraCardAction { const action = this._getAction(); switch (action.effect_action) { case 'start': - void api.getEffectsManager().startEffect(action.effect); + // An effect runs for its own duration, so it is deliberately not + // awaited: doing so would stall the rest of the action set. + api + .getEffectsManager() + .startEffect(action.effect) + .catch(() => {}); break; case 'stop': api.getEffectsManager().stopEffect(action.effect); break; case 'toggle': - void api.getEffectsManager().toggleEffect(action.effect); + api + .getEffectsManager() + .toggleEffect(action.effect) + .catch(() => {}); break; } } diff --git a/src/card-controller/actions/actions/ptz-digital.ts b/src/card-controller/actions/actions/ptz-digital.ts index 5b0463c9..dd5d4ed9 100644 --- a/src/card-controller/actions/actions/ptz-digital.ts +++ b/src/card-controller/actions/actions/ptz-digital.ts @@ -6,12 +6,12 @@ import { ZOOM_DEFAULT_SCALE, type PartialZoomSettings, } from '../../../components-lib/zoom/types'; -import { generateViewContextForZoom } from '../../../components-lib/zoom/zoom-view-context'; import type { PTZDigitialActionConfig } from '../../../config/schema/actions/custom/ptz-digital'; import { ZOOM_MAX, ZOOM_MIN } from '../../../config/schema/common/zoom'; import { getPTZTarget } from '../../../utils/ptz'; import { Timer } from '../../../utils/timer'; import type { CardActionsAPI } from '../../types'; +import { ZoomRequestViewModifier } from '../../view/modifiers/zoom-request'; import type { TargetedActionContext } from '../types'; import { setInProgressForThisTarget, @@ -33,13 +33,16 @@ export class PTZDigitalAction extends AdvancedCameraCardAction { - api.getViewManager().setViewWithMergedContext( - generateViewContextForZoom(targetID, { - requested: this._convertActionToZoomSettings( - api.getViewManager().getView()?.context?.zoom?.[targetID]?.observed, + api + .getViewManager() + .setViewWithModifiers([ + new ZoomRequestViewModifier( + targetID, + this._convertActionToZoomSettings( + api.getViewManager().getView()?.context?.zoom?.[targetID]?.observed, + ), ), - }), - ); + ]); } public async stop(): Promise { diff --git a/src/card-controller/actions/actions/ptz-multi.ts b/src/card-controller/actions/actions/ptz-multi.ts index 5c6d4a42..c4daf6f7 100644 --- a/src/card-controller/actions/actions/ptz-multi.ts +++ b/src/card-controller/actions/actions/ptz-multi.ts @@ -28,7 +28,7 @@ export class PTZMultiAction extends AdvancedCameraCardAction { +): View => { modifiers?.forEach((modifier) => modifier.modify(view)); + return view; }; diff --git a/src/card-controller/view/modifiers/zoom-request.ts b/src/card-controller/view/modifiers/zoom-request.ts new file mode 100644 index 00000000..54c02adc --- /dev/null +++ b/src/card-controller/view/modifiers/zoom-request.ts @@ -0,0 +1,30 @@ +import type { PartialZoomSettings } from '../../../components-lib/zoom/types'; +import { generateViewContextForZoom } from '../../../components-lib/zoom/zoom-view-context'; +import type { View } from '../../../view/view'; +import type { ViewModifier } from '../types'; + +// The single write path for a zoom request. The settings are *assigned* rather than +// *merged*, so a new request always replaces the one before it. This is +// necessary as a request to reset to default is expressed as an empty object, +// and a deep merge cannot use that to *erase* the values of an earlier request. +export class ZoomRequestViewModifier implements ViewModifier { + private _targetID: string; + private _requested: PartialZoomSettings; + + constructor(targetID: string, requested: PartialZoomSettings) { + this._targetID = targetID; + this._requested = requested; + } + + public modify(view: View): void { + const target = view.context?.zoom?.[this._targetID]; + if (target) { + target.requested = this._requested; + return; + } + + view.mergeInContext( + generateViewContextForZoom(this._targetID, { requested: this._requested }), + ); + } +} diff --git a/src/card-controller/view/types.ts b/src/card-controller/view/types.ts index 9c448055..0bf60dcd 100644 --- a/src/card-controller/view/types.ts +++ b/src/card-controller/view/types.ts @@ -82,6 +82,7 @@ export interface ViewManagerInterface { setViewByParametersWithExistingQuery(options?: ViewFactoryOptions): Promise; setViewWithMergedContext(context: ViewContext | null): void; + setViewWithModifiers(modifiers: ViewModifier[]): void; hasMajorMediaChange(oldView?: View | null, newView?: View | null): boolean; } diff --git a/src/card-controller/view/view-manager.ts b/src/card-controller/view/view-manager.ts index 317ea192..79857077 100644 --- a/src/card-controller/view/view-manager.ts +++ b/src/card-controller/view/view-manager.ts @@ -334,6 +334,12 @@ export class ViewManager implements ViewManagerInterface { } } + public setViewWithModifiers(modifiers: ViewModifier[]): void { + if (this._view) { + return this._setView(applyViewModifiers(this._view.clone(), modifiers)); + } + } + /** * Detect if the current view has a major "media change" for the given previous view. * @param oldView The previous view. diff --git a/src/components-lib/zoom/zoom-view-context.ts b/src/components-lib/zoom/zoom-view-context.ts index 70b3f316..329e167b 100644 --- a/src/components-lib/zoom/zoom-view-context.ts +++ b/src/components-lib/zoom/zoom-view-context.ts @@ -8,7 +8,9 @@ interface ZoomViewContext { observed?: ZoomSettingsObserved; // Populate this to request zoom to a particular scale/x/y. An empty object - // will reset to default, null will make no change. + // will reset to default, null will make no change. Requests must be written + // by `ZoomRequestViewModifier` since an empty object needs to mean "reset to + // default" (and a merged empty object != an assigned empty object). requested?: PartialZoomSettings | null; } diff --git a/tests/card-controller/actions/actions/effect.test.ts b/tests/card-controller/actions/actions/effect.test.ts index cb45c42d..9c088921 100644 --- a/tests/card-controller/actions/actions/effect.test.ts +++ b/tests/card-controller/actions/actions/effect.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { EffectAction } from '../../../../src/card-controller/actions/actions/effect'; import { createEffectAction } from '../../../../src/utils/action'; @@ -7,6 +7,7 @@ import { createCardAPI } from '../../../test-utils'; describe('EffectAction', () => { it('should call startEffect when action is start', async () => { const api = createCardAPI(); + vi.mocked(api.getEffectsManager().startEffect).mockResolvedValue(undefined); const actionConfig = createEffectAction('snow', 'start'); const action = new EffectAction({}, actionConfig); @@ -27,6 +28,7 @@ describe('EffectAction', () => { it('should call toggleEffect when action is toggle', async () => { const api = createCardAPI(); + vi.mocked(api.getEffectsManager().toggleEffect).mockResolvedValue(undefined); const actionConfig = createEffectAction('snow', 'toggle'); const action = new EffectAction({}, actionConfig); @@ -35,6 +37,24 @@ describe('EffectAction', () => { expect(api.getEffectsManager().toggleEffect).toHaveBeenCalledWith('snow'); }); + it('should tolerate a failure to start the effect', async () => { + const api = createCardAPI(); + vi.mocked(api.getEffectsManager().startEffect).mockRejectedValue(new Error()); + const actionConfig = createEffectAction('snow', 'start'); + const action = new EffectAction({}, actionConfig); + + await expect(action.execute(api)).resolves.toBeUndefined(); + }); + + it('should tolerate a failure to toggle the effect', async () => { + const api = createCardAPI(); + vi.mocked(api.getEffectsManager().toggleEffect).mockRejectedValue(new Error()); + const actionConfig = createEffectAction('snow', 'toggle'); + const action = new EffectAction({}, actionConfig); + + await expect(action.execute(api)).resolves.toBeUndefined(); + }); + it('should have a no-op stop method', async () => { const actionConfig = createEffectAction('snow', 'stop'); const action = new EffectAction({}, actionConfig); diff --git a/tests/card-controller/actions/actions/ptz-digital.test.ts b/tests/card-controller/actions/actions/ptz-digital.test.ts index edc23bd1..a2171a56 100644 --- a/tests/card-controller/actions/actions/ptz-digital.test.ts +++ b/tests/card-controller/actions/actions/ptz-digital.test.ts @@ -1,14 +1,25 @@ +import type { ViewContext } from 'view'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { PTZDigitalAction } from '../../../../src/card-controller/actions/actions/ptz-digital'; +import type { CardController } from '../../../../src/card-controller/controller'; import type { PartialZoomSettings, ZoomSettingsObserved, } from '../../../../src/components-lib/zoom/types'; import type { PTZAction } from '../../../../src/config/schema/actions/custom/ptz'; +import { applySetViewModifiers } from '../../../card-controller/view/test-utils'; import { createCardAPI } from '../../../test-utils'; import { createView } from '../../../view/test-utils'; +const getRequestedZoom = ( + api: CardController, + context?: ViewContext, + n = 0, +): PartialZoomSettings | null | undefined => + applySetViewModifiers(api.getViewManager(), createView({ context }), n).context?.zoom + ?.camera?.requested; + describe('should handle ptz digital action', () => { const defaultSettings = { pan: { @@ -48,19 +59,12 @@ describe('should handle ptz digital action', () => { await action.execute(api); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledWith({ - zoom: { - camera: { - observed: undefined, - requested: { - pan: { - x: 3, - y: 4, - }, - zoom: 2, - }, - }, + expect(getRequestedZoom(api)).toEqual({ + pan: { + x: 3, + y: 4, }, + zoom: 2, }); }); @@ -78,14 +82,40 @@ describe('should handle ptz digital action', () => { await action.execute(api); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledWith({ - zoom: { - camera: { - observed: undefined, - requested: {}, + expect(getRequestedZoom(api)).toEqual({}); + }); + + // See: https://github.com/dermotduffy/advanced-camera-card/issues/2632 + it('should return to default when chained after an absolute request', async () => { + const api = createCardAPI(); + vi.mocked(api.getViewManager().getView).mockReturnValue(createView()); + + await new PTZDigitalAction( + {}, + { + action: 'fire-dom-event', + advanced_camera_card_action: 'ptz_digital', + absolute: { + zoom: 3, }, }, - }); + ).execute(api); + + await new PTZDigitalAction( + {}, + { + action: 'fire-dom-event', + advanced_camera_card_action: 'ptz_digital', + }, + ).execute(api); + + // Both actions land on a single view, as they do when an automation runs + // them back to back. + const view = createView(); + applySetViewModifiers(api.getViewManager(), view, 0); + applySetViewModifiers(api.getViewManager(), view, 1); + + expect(view.context?.zoom?.camera?.requested).toEqual({}); }); it('should do nothing without a view', async () => { @@ -102,7 +132,7 @@ describe('should handle ptz digital action', () => { await action.execute(api); - expect(api.getViewManager().setViewWithMergedContext).not.toHaveBeenCalledWith(); + expect(api.getViewManager().setViewWithModifiers).not.toHaveBeenCalled(); }); it('should do nothing without a camera', async () => { @@ -126,7 +156,7 @@ describe('should handle ptz digital action', () => { await action.execute(api); - expect(api.getViewManager().setViewWithMergedContext).not.toHaveBeenCalled(); + expect(api.getViewManager().setViewWithModifiers).not.toHaveBeenCalled(); }); describe('should honor ptz_action', () => { @@ -339,16 +369,11 @@ describe('should handle ptz digital action', () => { await action.execute(api); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledWith({ - zoom: { - camera: { - observed: undefined, - requested: { - ...defaultSettings, - ...expectedSettings, - }, - }, - }, + expect( + getRequestedZoom(api, { zoom: { camera: { observed: current } } }), + ).toEqual({ + ...defaultSettings, + ...expectedSettings, }); }, ); @@ -380,61 +405,40 @@ describe('should handle ptz digital action', () => { await action.execute(api); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenLastCalledWith({ - zoom: { - camera: { - observed: undefined, - requested: { - ...defaultSettings, - pan: { - x: 55, - y: 50, - }, - }, - }, + expect(getRequestedZoom(api)).toEqual({ + ...defaultSettings, + pan: { + x: 55, + y: 50, }, }); // Update the context to reflect the first step. + const observed = createObserved({ + pan: { + x: 55, + y: 50, + }, + }); vi.mocked(api.getViewManager().getView).mockReturnValue( - createView({ - context: { - zoom: { - camera: { - observed: createObserved({ - pan: { - x: 55, - y: 50, - }, - }), - }, - }, - }, - }), + createView({ context: { zoom: { camera: { observed } } } }), ); vi.runOnlyPendingTimers(); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenLastCalledWith({ - zoom: { - camera: { - observed: undefined, - requested: { - ...defaultSettings, - pan: { - x: 60, - y: 50, - }, - }, - }, + expect(getRequestedZoom(api, { zoom: { camera: { observed } } }, 1)).toEqual({ + ...defaultSettings, + pan: { + x: 60, + y: 50, }, }); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledTimes(2); + expect(api.getViewManager().setViewWithModifiers).toHaveBeenCalledTimes(2); action.stop(); vi.runOnlyPendingTimers(); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledTimes(2); + expect(api.getViewManager().setViewWithModifiers).toHaveBeenCalledTimes(2); }); it('stop', async () => { @@ -450,21 +454,14 @@ describe('should handle ptz digital action', () => { }); await startAction.execute(api); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenLastCalledWith({ - zoom: { - camera: { - observed: undefined, - requested: { - ...defaultSettings, - pan: { - x: 55, - y: 50, - }, - }, - }, + expect(getRequestedZoom(api)).toEqual({ + ...defaultSettings, + pan: { + x: 55, + y: 50, }, }); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledTimes(1); + expect(api.getViewManager().setViewWithModifiers).toHaveBeenCalledTimes(1); const stopAction = new PTZDigitalAction(context, { action: 'fire-dom-event', @@ -475,7 +472,7 @@ describe('should handle ptz digital action', () => { vi.runOnlyPendingTimers(); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenCalledTimes(1); + expect(api.getViewManager().setViewWithModifiers).toHaveBeenCalledTimes(1); }); }); }); diff --git a/tests/card-controller/actions/actions/ptz-multi.test.ts b/tests/card-controller/actions/actions/ptz-multi.test.ts index bd1e5b78..ba311029 100644 --- a/tests/card-controller/actions/actions/ptz-multi.test.ts +++ b/tests/card-controller/actions/actions/ptz-multi.test.ts @@ -4,6 +4,7 @@ import { Capabilities } from '../../../../src/camera-manager/capabilities'; import { PTZMultiAction } from '../../../../src/card-controller/actions/actions/ptz-multi'; import { PTZMovementType } from '../../../../src/types'; import { createCameraManager, createStore } from '../../../camera-manager/test-utils'; +import { applySetViewModifiers } from '../../../card-controller/view/test-utils'; import { createCardAPI } from '../../../test-utils'; import { createView } from '../../../view/test-utils'; @@ -47,7 +48,7 @@ describe('should handle ptz multi action', () => { preset: undefined, }, ); - expect(api.getViewManager().setViewWithMergedContext).not.toHaveBeenCalled(); + expect(api.getViewManager().setViewWithModifiers).not.toHaveBeenCalled(); }); it('should use digital ptz when camera does not have ptz support', async () => { @@ -77,19 +78,13 @@ describe('should handle ptz multi action', () => { await action.execute(api); expect(api.getCameraManager().executePTZAction).not.toHaveBeenCalled(); - expect(api.getViewManager().setViewWithMergedContext).toHaveBeenLastCalledWith({ - zoom: { - 'camera.office': { - observed: undefined, - requested: expect.objectContaining({ - pan: { - x: 55, - y: 50, - }, - zoom: 1, - }), - }, + const modifiedView = applySetViewModifiers(api.getViewManager(), createView()); + expect(modifiedView.context?.zoom?.['camera.office'].requested).toEqual({ + pan: { + x: 55, + y: 50, }, + zoom: 1, }); }); }); @@ -115,7 +110,7 @@ describe('should handle ptz multi action', () => { await action.execute(api); expect(api.getCameraManager().executePTZAction).not.toHaveBeenCalled(); - expect(api.getViewManager().setViewWithMergedContext).not.toHaveBeenCalled(); + expect(api.getViewManager().setViewWithModifiers).not.toHaveBeenCalled(); }); it('should do nothing with a media-less view without an explicit target_id', async () => { @@ -144,6 +139,6 @@ describe('should handle ptz multi action', () => { await action.execute(api); expect(api.getCameraManager().executePTZAction).not.toHaveBeenCalled(); - expect(api.getViewManager().setViewWithMergedContext).not.toHaveBeenCalled(); + expect(api.getViewManager().setViewWithModifiers).not.toHaveBeenCalled(); }); }); diff --git a/tests/card-controller/view/modifiers/zoom-request.test.ts b/tests/card-controller/view/modifiers/zoom-request.test.ts new file mode 100644 index 00000000..f649fba2 --- /dev/null +++ b/tests/card-controller/view/modifiers/zoom-request.test.ts @@ -0,0 +1,46 @@ +import { expect, it } from 'vitest'; + +import { ZoomRequestViewModifier } from '../../../../src/card-controller/view/modifiers/zoom-request'; +import { createView } from '../../../view/test-utils'; + +it('should write a request when the target has no zoom context', () => { + const view = createView(); + + new ZoomRequestViewModifier('target', { zoom: 3 }).modify(view); + + expect(view.context?.zoom?.target.requested).toEqual({ zoom: 3 }); +}); + +it('should preserve the observed settings of the target', () => { + const observed = { + pan: { x: 1, y: 2 }, + zoom: 3, + isDefault: false, + unzoomed: false, + }; + const view = createView({ context: { zoom: { target: { observed } } } }); + + new ZoomRequestViewModifier('target', { zoom: 4 }).modify(view); + + expect(view.context?.zoom?.target.observed).toEqual(observed); +}); + +it('should replace an earlier request rather than merge into it', () => { + const view = createView({ + context: { zoom: { target: { requested: { zoom: 3, pan: { x: 1, y: 2 } } } } }, + }); + + new ZoomRequestViewModifier('target', { pan: { x: 5, y: 6 } }).modify(view); + + expect(view.context?.zoom?.target.requested).toEqual({ pan: { x: 5, y: 6 } }); +}); + +it('should allow an empty request to erase an earlier request', () => { + const view = createView({ + context: { zoom: { target: { requested: { zoom: 3 } } } }, + }); + + new ZoomRequestViewModifier('target', {}).modify(view); + + expect(view.context?.zoom?.target.requested).toEqual({}); +}); diff --git a/tests/card-controller/view/test-utils.ts b/tests/card-controller/view/test-utils.ts index 0027cdd1..ce1bd051 100644 --- a/tests/card-controller/view/test-utils.ts +++ b/tests/card-controller/view/test-utils.ts @@ -1,7 +1,10 @@ -import { vi } from 'vitest'; +import { expect, vi } from 'vitest'; import type { CardController } from '../../../src/card-controller/controller'; +import { applyViewModifiers } from '../../../src/card-controller/view/modifiers'; +import type { ViewManagerInterface } from '../../../src/card-controller/view/types'; import type { RawAdvancedCameraCardConfig } from '../../../src/config/types'; +import type { View } from '../../../src/view/view'; import { createCameraManager, createCapabilities, @@ -42,3 +45,15 @@ export const createPopulatedAPI = ( vi.mocked(api.getConfigManager().getConfig).mockReturnValue(createConfig(config)); return api; }; + +// Apply the modifiers from the nth `setViewWithModifiers` call to the given +// view, so that a caller can assert on the resulting view. +export const applySetViewModifiers = ( + viewManager: ViewManagerInterface, + view: View, + n = 0, +): View => { + const mock = vi.mocked(viewManager.setViewWithModifiers).mock; + expect(mock.calls.length).greaterThan(n); + return applyViewModifiers(view, mock.calls[n][0]); +}; diff --git a/tests/card-controller/view/view-manager.test.ts b/tests/card-controller/view/view-manager.test.ts index d2b0e9a8..d401d226 100644 --- a/tests/card-controller/view/view-manager.test.ts +++ b/tests/card-controller/view/view-manager.test.ts @@ -4,6 +4,7 @@ import { mock } from 'vitest-mock-extended'; import type { CardController } from '../../../src/card-controller/controller'; import type { ViewFactory } from '../../../src/card-controller/view/factory'; +import { MergeContextViewModifier } from '../../../src/card-controller/view/modifiers/merge-context'; import { SetQueryViewModifier } from '../../../src/card-controller/view/modifiers/set-query'; import type { QueryExecutorOptions, @@ -131,6 +132,26 @@ it('should set view with merged context', () => { expect(manager.getView()?.context).toEqual(context); }); +it('should set view with modifiers', () => { + const api = createInitializedCardAPI(); + const factory = mock(); + + const manager = new ViewManager(api, { viewFactory: factory }); + const modifier = new MergeContextViewModifier({ timeline: {} }); + + // Applying modifiers with no existing view does nothing. + manager.setViewWithModifiers([modifier]); + expect(manager.getView()).toBeNull(); + + factory.getViewDefault.mockReturnValue(createView({ view: 'live', camera: 'camera' })); + manager.setViewDefault(); + manager.setViewWithModifiers([modifier]); + + expect(manager.getView()?.camera).toBe('camera'); + expect(manager.getView()?.view).toBe('live'); + expect(manager.getView()?.context).toEqual({ timeline: {} }); +}); + it('should return epoch', () => { const factory = mock(); const manager = new ViewManager(createCardAPI(), { viewFactory: factory });