fix: ignore state changes reported while a controller is being restored

src/lib/model/ControllerStorage.spec.ts

@@ -0,0 +1,78 @@
+import { Accessory } from "../Accessory";
+import { ControllerIdentifier, ControllerServiceMap, SerializableController, StateChangeDelegate } from "../controller";
+import * as uuid from "../util/uuid";
+import { ControllerStorage } from "./ControllerStorage";
+
+interface State {
+  first?: string;
+  second?: string;
+}
+
+// restores its state in two steps and reports a state change in between, like a controller whose
+// characteristic updates during deserialize trigger its own change listeners
+class SteppedController implements SerializableController<ControllerServiceMap, State> {
+  state: State = {};
+  private stateChangeDelegate?: StateChangeDelegate;
+
+  controllerId(): ControllerIdentifier {
+    return "stepped";
+  }
+
+  constructServices(): ControllerServiceMap {
+    return {};
+  }
+
+  initWithServices(): void | ControllerServiceMap {
+    return undefined;
+  }
+
+  configureServices(): void {
+    // nothing to configure
+  }
+
+  handleControllerRemoved(): void {
+    // nothing to clean up
+  }
+
+  serialize(): State | undefined {
+    return { ...this.state };
+  }
+
+  deserialize(serialized: State): void {
+    this.state.first = serialized.first;
+    this.stateChangeDelegate?.();
+    this.state.second = serialized.second;
+  }
+
+  setupStateChangeDelegate(delegate?: StateChangeDelegate): void {
+    this.stateChangeDelegate = delegate;
+  }
+
+  handleFactoryReset(): void {
+    this.state = {};
+  }
+}
+
+describe("ControllerStorage", () => {
+  test("state changes reported while restoring do not overwrite the stored data", () => {
+    const accessory = new Accessory("Test", uuid.generate("controller-storage"));
+    const storage = new ControllerStorage(accessory);
+    const controller = new SteppedController();
+    storage.trackController(controller);
+
+    // @ts-expect-error: private access
+    storage.init([{ type: "stepped", controllerData: { data: { first: "a", second: "b" } } }]);
+
+    expect(controller.state).toEqual({ first: "a", second: "b" });
+    // @ts-expect-error: private access
+    expect(storage.controllerData.stepped.data).toEqual({ first: "a", second: "b" });
+
+    controller.state.second = "c";
+    // @ts-expect-error: private access
+    storage.handleStateChange(controller);
+    // @ts-expect-error: private access
+    expect(storage.controllerData.stepped.data).toEqual({ first: "a", second: "c" });
+    // @ts-expect-error: private access
+    clearTimeout(storage.queuedSaveTimeout);
+  });
+});

src/lib/model/ControllerStorage.ts

@@ -47,6 +47,11 @@ export class ControllerStorage {
   private controllerData: Record<ControllerIdentifier, ControllerData> = {};
   private restoredAccessories?: Record<string, StoredControllerData[]>; // indexed by accessory UUID
 
+  // deserializing a controller updates its characteristics, which can report state changes (its own, or
+  // those of another controller listening on a shared service) before the restore is complete.
+  // Those must not overwrite the stored data with a partially restored state.
+  private restoring = false;
+
   private parent?: ControllerStorage;
   private linkedAccessories?: ControllerStorage[];
 
@@ -133,6 +138,9 @@ export class ControllerStorage {
   }
 
   private handleStateChange(controller: SerializableController) {
+    if (this.restoring) {
+      return;
+    }
     const id = controller.controllerId();
     const serialized = controller.serialize();
 
@@ -174,11 +182,15 @@ export class ControllerStorage {
       this.accessoryUUID, controller.controllerId(), !!controllerData);
 
     if (controllerData) {
+      const wasRestoring = this.restoring;
+      this.restoring = true;
       try {
         controller.deserialize(controllerData.data);
       } catch (error) {
         console.warn(`Could not initialize controller of type '${controller.controllerId()}' from data stored on disk. Resetting to default: ${error.stack}`);
         controller.handleFactoryReset();
+      } finally {
+        this.restoring = wasRestoring;
       }
       controllerData.purgeOnNextLoad = undefined;
     }
@@ -202,10 +214,15 @@ export class ControllerStorage {
     }
 
     const restoredControllers: ControllerIdentifier[] = [];
-    this.trackedControllers.forEach(controller => {
-      this.restoreController(controller);
-      restoredControllers.push(controller.controllerId());
-    });
+    this.restoring = true;
+    try {
+      this.trackedControllers.forEach(controller => {
+        this.restoreController(controller);
+        restoredControllers.push(controller.controllerId());
+      });
+    } finally {
+      this.restoring = false;
+    }
     this.trackedControllers.splice(0, this.trackedControllers.length); // clear tracking list
 
     let purgedData = false;