fix: Prevent "action loops" (#1978)

- Closes: #1967
- Closes: #1969

src/card-controller/actions/actions-manager.ts

@@ -8,7 +8,10 @@ import {
   AuxillaryActionConfig,
 } from '../../config/schema/actions/types.js';
 import { forwardHaptic } from '../../ha/haptic.js';
-import { getActionConfigGivenAction } from '../../utils/action.js';
+import {
+  getActionConfigGivenAction,
+  isAdvancedCameraCardCustomAction,
+} from '../../utils/action.js';
 import { allPromises } from '../../utils/basic.js';
 import { TemplateRenderer } from '../templates/index.js';
 import { CardActionsManagerAPI } from '../types.js';
@@ -101,7 +104,16 @@ export class ActionsManager {
       // https://github.com/custom-cards/custom-card-helpers/blob/master/src/fire-event.ts#L70
       return;
     }
-    await this.executeActions(ev.detail as ActionConfig);
+    const action: ActionConfig = ev.detail;
+
+    // If the received action is not a custom action specifically for this card
+    // to handle, ignore it. Otherwise, we can get action "loops". See:
+    // https://github.com/dermotduffy/advanced-camera-card/issues/1969
+    if (!isAdvancedCameraCardCustomAction(action)) {
+      return;
+    }
+
+    await this.executeActions(action);
   };
 
   /**

src/card-controller/actions/actions/ptz-digital.ts

@@ -67,7 +67,7 @@ export class PTZDigitalAction extends AdvancedCameraCardAction<PTZDigitialAction
 
     /* istanbul ignore else: the else path cannot be reached -- @preserve */
     if (this._action.ptz_phase === 'start') {
-      stopInProgressForThisTarget(targetID, this._context.ptzDigital);
+      await stopInProgressForThisTarget(targetID, this._context.ptzDigital);
       setInProgressForThisTarget(targetID, this._context, 'ptzDigital', this);
 
       await this._stepChange(api, targetID);
@@ -75,7 +75,7 @@ export class PTZDigitalAction extends AdvancedCameraCardAction<PTZDigitialAction
         this._stepChange(api, targetID),
       );
     } else if (this._action.ptz_phase === 'stop') {
-      stopInProgressForThisTarget(targetID, this._context.ptzDigital);
+      await stopInProgressForThisTarget(targetID, this._context.ptzDigital);
       delete this._context.ptzDigital?.[targetID];
     }
   }

src/card-controller/actions/actions/ptz.ts

@@ -22,8 +22,10 @@ declare module 'action' {
 
 export class PTZAction extends AdvancedCameraCardAction<PTZActionConfig> {
   protected _timer = new Timer();
+  protected _stopped = false;
 
   public async stop(): Promise<void> {
+    this._stopped = true;
     this._timer.stop();
   }
 
@@ -79,7 +81,7 @@ export class PTZAction extends AdvancedCameraCardAction<PTZActionConfig> {
 
     if (this._action.ptz_phase === 'start') {
       // Scenario: Asked to start a continuous move, camera only supports relative moves natively.
-      stopInProgressForThisTarget(ptzCameraID, this._context.ptz);
+      await stopInProgressForThisTarget(ptzCameraID, this._context.ptz);
       setInProgressForThisTarget(ptzCameraID, this._context, 'ptz', this);
 
       const singleStep = async (): Promise<void> => {
@@ -89,15 +91,26 @@ export class PTZAction extends AdvancedCameraCardAction<PTZActionConfig> {
             .executePTZAction(ptzCameraID, this._action.ptz_action, {
               preset: this._action.ptz_preset,
             }));
-        // Only start the timer for the next step after this step returns.
-        this._timer.start(ptzConfiguration.r2c_delay_between_calls_seconds, singleStep);
+
+        if (!this._stopped) {
+          // Only start the timer for the next step after this step returns, and
+          // only if this action has not been stopped.
+          // See: https://github.com/dermotduffy/advanced-camera-card/issues/1967
+          this._timer.start(
+            ptzConfiguration.r2c_delay_between_calls_seconds,
+            singleStep,
+          );
+        }
       };
 
+      this._stopped = false;
       await singleStep();
     } else if (this._action.ptz_phase === 'stop') {
       // Scenario: Asked to stop continuous move, camera only supports relative moves natively.
-      stopInProgressForThisTarget(ptzCameraID, this._context.ptz);
+      await stopInProgressForThisTarget(ptzCameraID, this._context.ptz);
     } else {
+      this._stopped = false;
+
       // Relative move (but camera only supports continuous).
       await api
         .getCameraManager()

src/card-controller/actions/utils/action-state.ts

@@ -2,11 +2,11 @@ import merge from 'lodash-es/merge';
 import { Action, TargetedActionContext } from '../types';
 import { ActionContext } from 'action';
 
-export const stopInProgressForThisTarget = (
+export const stopInProgressForThisTarget = async (
   targetID: string,
   context?: TargetedActionContext,
-): void => {
-  context?.[targetID]?.inProgressAction?.stop();
+): Promise<void> => {
+  await context?.[targetID]?.inProgressAction?.stop();
 };
 
 export const setInProgressForThisTarget = (
@@ -14,7 +14,7 @@ export const setInProgressForThisTarget = (
   context: ActionContext,
   contextKey: keyof ActionContext,
   action: Action,
-) => {
+): void => {
   merge(context, {
     [contextKey]: {
       [targetID]: {

tests/card-controller/actions/actions-manager.test.ts

@@ -206,7 +206,7 @@ describe('ActionsManager', () => {
       vi.restoreAllMocks();
     });
 
-    it('should handle event', async () => {
+    it('should handle advanced camera card event', async () => {
       const action = createLogAction('Hello, world!');
       const event = new CustomEvent('ll-custom', {
         detail: action,
@@ -220,6 +220,27 @@ describe('ActionsManager', () => {
       expect(consoleSpy).toBeCalled();
     });
 
+    it('should not handle generic event', async () => {
+      const event = new CustomEvent('ll-custom', {
+        detail: {
+          type: 'fire-dom-event',
+          foo: 'bar',
+        },
+      });
+
+      const card = document.createElement('div');
+      const handler = vi.fn();
+      card.addEventListener('ll-custom', handler);
+
+      const api = createCardAPI();
+      vi.mocked(api.getCardElementManager().getElement).mockReturnValue(card);
+      const manager = new ActionsManager(api);
+
+      await manager.handleCustomActionEvent(event);
+
+      expect(handler).not.toBeCalled();
+    });
+
     it('should not handle event without detail', async () => {
       const manager = new ActionsManager(createCardAPI());
 

tests/card-controller/actions/actions/ptz.test.ts

@@ -478,7 +478,8 @@ describe('should handle ptz action', () => {
           }),
         },
       ]);
-      vi.mocked(api.getCameraManager).mockReturnValue(createCameraManager(store));
+      const cameraManager = createCameraManager(store);
+      vi.mocked(api.getCameraManager).mockReturnValue(cameraManager);
       vi.mocked(api.getViewManager().getView).mockReturnValue(
         createView({ camera: 'camera.office' }),
       );
@@ -497,11 +498,24 @@ describe('should handle ptz action', () => {
       await vi.runOnlyPendingTimersAsync();
       expect(api.getCameraManager().executePTZAction).toBeCalledTimes(2);
 
+      // Emulate the stop being called while the action is running, but before
+      // the *next* timer is scheduled.
+      let resolve: () => void;
+      const promise: Promise<void> = new Promise((_resolve) => {
+        resolve = _resolve;
+      });
+      vi.mocked(cameraManager.executePTZAction).mockReturnValueOnce(promise);
+
+      await vi.runOnlyPendingTimersAsync();
+      expect(api.getCameraManager().executePTZAction).toBeCalledTimes(3);
+
       action.stop();
+
+      resolve!();
       await vi.runOnlyPendingTimersAsync();
 
       // There should be no additional calls.
-      expect(api.getCameraManager().executePTZAction).toBeCalledTimes(2);
+      expect(api.getCameraManager().executePTZAction).toBeCalledTimes(3);
     });
   });
 });