fix: Fix initialization race condition that occurs in certain circums…

…tances (#1545)

* fix: Fix initialization race condition in certain circumstances

* Minor improvements and camera iris logo

src/camera-manager/manager.ts

@@ -111,7 +111,6 @@ export class CameraManager {
   protected _api: CardCameraAPI;
   protected _engineFactory: CameraManagerEngineFactory;
   protected _store: CameraManagerStore;
-  protected _initializationLimit = new PQueue({ concurrency: 1 });
   protected _requestLimit = new PQueue();
 
   constructor(
@@ -147,16 +146,8 @@ export class CameraManager {
       recursivelyMergeObjectsNotArrays({}, cloneDeep(config?.cameras_global), camera),
     );
 
-    const resetAndInitialize = async () => {
-      await this._reset();
-      await this._initializeCameras(cameras);
-    };
-
     try {
-      // This concurrency limit prevents multiple rapidly arriving configs from
-      // generating reset-n-initialize race conditions (e.g. changing values
-      // rapidly in the config editor).
-      await this._initializationLimit.add(resetAndInitialize);
+      await this._initializeCameras(cameras);
     } catch (e: unknown) {
       this._api
         .getMessageManager()
@@ -166,7 +157,7 @@ export class CameraManager {
     return true;
   }
 
-  protected async _reset(): Promise<void> {
+  public async reset(): Promise<void> {
     await this._store.reset();
   }
 
@@ -246,6 +237,8 @@ export class CameraManager {
       async ([cameraConfig, engine]) => await engine.createCamera(hass, cameraConfig),
     );
 
+    const cameraIDs: Set<string> = new Set();
+
     // Do the additions based off the result-order, to ensure the map order is
     // preserved.
     cameras.forEach((camera) => {
@@ -258,7 +251,7 @@ export class CameraManager {
         );
       }
 
-      if (this._store.hasCameraID(cameraID)) {
+      if (cameraIDs.has(cameraID)) {
         throw new CameraInitializationError(
           localize('error.duplicate_camera_id'),
           camera.getConfig(),
@@ -267,9 +260,11 @@ export class CameraManager {
 
       // Always ensure the actual ID used in the card is in the configuration itself.
       camera.setID(cameraID);
-      this._store.addCamera(camera);
+      cameraIDs.add(cameraID);
     });
 
+    await this._store.setCameras(cameras);
+
     log(
       this._api.getConfigManager().getCardWideConfig(),
       'Frigate Card CameraManager initialized (Cameras: ',

src/camera-manager/store.ts

@@ -44,6 +44,30 @@ export class CameraManagerStore implements CameraManagerReadOnlyConfigStore {
     this._enginesByType.set(camera.getEngine().getEngineType(), camera.getEngine());
   }
 
+  public async setCameras(cameras: Camera[]): Promise<void> {
+    // In setting the store cameras, take great care to replace/add first before
+    // remove. Otherwise, there may be race conditions where the card attempts
+    // to render a view with (momentarily) no camera.
+    // See: https://github.com/dermotduffy/frigate-hass-card/issues/1533
+
+    // Replace/Add the new cameras.
+    for (const camera of cameras) {
+      const oldCamera = this._cameras.get(camera.getID());
+      if (oldCamera !== camera) {
+        this.addCamera(camera);
+        await oldCamera?.destroy();
+      }
+    }
+
+    // Remove the old cameras.
+    for (const camera of this._cameras.values()) {
+      if (!cameras.includes(camera)) {
+        await camera.destroy();
+        this._cameras.delete(camera.getID());
+      }
+    }
+  }
+
   public async reset(): Promise<void> {
     await allPromises(this._cameras.values(), (camera) => camera.destroy());
     this._cameras.clear();

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

@@ -67,7 +67,12 @@ export class CardElementManager {
     this._api.getMediaLoadedInfoManager().initialize();
     this._api.getMicrophoneManager().initialize();
     this._api.getKeyboardStateManager().initialize();
+
+    // These initializers are called when the config is updated, but on initial
+    // creation of the card hass is not yet available when the config is first
+    // loaded.
     this._api.getDefaultManager().initialize();
+    this._api.getMediaPlayerManager().initialize();
 
     this._api
       .getHASSManager()
@@ -146,6 +151,8 @@ export class CardElementManager {
     // reconnection, to ensure the state subscription/unsubscription works
     // correctly for triggers.
     this._api.getInitializationManager().uninitialize(InitializationAspect.CAMERAS);
+    this._api.getCameraManager().reset();
+
     this._element.removeEventListener(
       'mousemove',
       this._api.getInteractionManager().reportInteraction,

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

@@ -92,16 +92,20 @@ export class ConfigManager {
       camera: undefined,
     });
     this._api.getMediaLoadedInfoManager().clear();
+
+    this._api.getInitializationManager().uninitialize(InitializationAspect.VIEW);
     this._api.getViewManager().reset();
+
     this._api.getMessageManager().reset();
     this._api.getStyleManager().setPerformance();
-    this._api.getCardElementManager().update();
     this._api.getStatusBarItemManager().removeAllDynamicStatusBarItems();
 
     setKeyboardShortcutsFromConfig(this._api, this);
     setAutomationsFromConfig(this._api);
 
     this.computeOverrideConfig();
+
+    this._api.getCardElementManager().update();
   }
 
   public computeOverrideConfig(): void {
@@ -144,17 +148,18 @@ export class ConfigManager {
         .uninitialize(InitializationAspect.MICROPHONE_CONNECT);
     }
 
-    if (
-      previousConfig &&
-      !isEqual(
-        previousConfig?.view.default_reset,
-        this._overriddenConfig?.view.default_reset,
-      )
-    ) {
-      this._api
-        .getInitializationManager()
-        .uninitialize(InitializationAspect.DEFAULT_RESET);
-    }
+    /* async */ this._initializeBackground(previousConfig);
+  }
+
+  /**
+   * Initialize config dependent items in the background. For items that the
+   * card hard requires, use InitializationManager instead.
+   */
+  protected async _initializeBackground(
+    previousConfig: FrigateCardConfig | null,
+  ): Promise<void> {
+    await this._api.getDefaultManager().initializeIfNecessary(previousConfig);
+    await this._api.getMediaPlayerManager().initializeIfNecessary(previousConfig);
 
     this._api.getCardElementManager().update();
   }

src/card-controller/default-manager.ts

@@ -1,4 +1,5 @@
-import PQueue from 'p-queue';
+import { isEqual } from 'lodash-es';
+import { FrigateCardConfig } from '../config/types';
 import { createGeneralAction } from '../utils/action';
 import { isActionAllowedBasedOnInteractionState } from '../utils/interaction-mode';
 import { Timer } from '../utils/timer';
@@ -10,20 +11,45 @@ import { CardDefaultManagerAPI } from './types';
 export class DefaultManager {
   protected _timer = new Timer();
   protected _api: CardDefaultManagerAPI;
-  protected _initializationLimit = new PQueue({ concurrency: 1 });
 
   constructor(api: CardDefaultManagerAPI) {
     this._api = api;
   }
 
+  public async initializeIfNecessary(
+    previousConfig: FrigateCardConfig | null,
+  ): Promise<void> {
+    if (
+      !isEqual(
+        previousConfig?.view.default_reset,
+        this._api.getConfigManager().getConfig()?.view.default_reset,
+      )
+    ) {
+      await this.initialize();
+    }
+  }
+
   /**
-   * Initialize the default manager. Requires both hass and configuration to be
-   * effective (so cannot be called from just the configuration manager, as hass
-   * will not be available yet)
+   * This needs to be public since the first initialization requires both hass
+   * and the config, so it is not suitable from calling exclusively from the
+   * config manager.
    */
   public async initialize(): Promise<boolean> {
-    const result = await this._initializationLimit.add(() => this._reconfigure());
-    this._startTimer();
+    this.uninitialize();
+
+    const config = this._api.getConfigManager().getConfig()?.view.default_reset;
+    if (config?.entities.length) {
+      this._api
+        .getHASSManager()
+        .getStateWatcher()
+        .subscribe(this._stateChangeHandler, config.entities);
+    }
+
+    const timerSeconds = this._api.getConfigManager().getConfig()?.view
+      .default_reset.every_seconds;
+    if (timerSeconds) {
+      this._timer.startRepeated(timerSeconds, () => this._setToDefaultIfAllowed());
+    }
 
     if (this._api.getConfigManager().getConfig()?.view.default_reset.after_interaction) {
       this._api.getAutomationsManager().addAutomations([
@@ -40,7 +66,7 @@ export class DefaultManager {
       ]);
     }
 
-    return !!result;
+    return true;
   }
 
   public uninitialize(): void {
@@ -49,28 +75,6 @@ export class DefaultManager {
     this._api.getAutomationsManager().deleteAutomations(this);
   }
 
-  protected async _reconfigure(): Promise<boolean> {
-    const hass = this._api.getHASSManager().getHASS();
-    const config = this._api.getConfigManager().getConfig()?.view.default_reset;
-    if (!hass || !config) {
-      return false;
-    }
-
-    this._api.getHASSManager().getStateWatcher().unsubscribe(this._stateChangeHandler);
-    this._api
-      .getHASSManager()
-      .getStateWatcher()
-      .subscribe(this._stateChangeHandler, config.entities);
-
-    // If the timer is running, restart it with the newly configured timer.
-    if (this._timer.isRunning()) {
-      this._timer.stop();
-      this._startTimer();
-    }
-
-    return true;
-  }
-
   protected _stateChangeHandler = (): void => {
     this._setToDefaultIfAllowed();
   };
@@ -92,12 +96,4 @@ export class DefaultManager {
       )
     );
   }
-
-  protected _startTimer(): void {
-    const timerSeconds = this._api.getConfigManager().getConfig()?.view
-      .default_reset.every_seconds;
-    if (timerSeconds) {
-      this._timer.startRepeated(timerSeconds, () => this._setToDefaultIfAllowed());
-    }
-  }
 }