Fixes for CHIP command pipe handling (#1565)

project-chip · Dec 26, 2024 · 061958b · 061958b
1 parent 8c38eca
commit 061958b
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 18 deletions.
diff --git a/chip-testing/src/AllClustersTestInstance.ts b/chip-testing/src/AllClustersTestInstance.ts
@@ -107,22 +107,28 @@ export class AllClustersTestInstance extends NodeTestInstance {
         switch (name) {
             case "simulateLongPress":
                 if (endpoint === undefined) {
-                    throw new Error(`Endpoint ${endpointId} not existing`);
+                    throw new Error(`Endpoint ${endpointId} not found`);
                 }
                 await SwitchSimulator.simulateLongPress(endpoint, command);
                 break;
             case "simulateMultiPress":
                 if (endpoint === undefined) {
-                    throw new Error(`Endpoint ${endpointId} not existing`);
+                    throw new Error(`Endpoint ${endpointId} not found`);
                 }
                 await SwitchSimulator.simulateMultiPress(endpoint, command);
                 break;
             case "simulateLatchPosition":
                 if (endpoint === undefined) {
-                    throw new Error(`Endpoint ${endpointId} not existing`);
+                    throw new Error(`Endpoint ${endpointId} not found`);
                 }
                 await endpoint.setStateOf(SwitchServer, { currentPosition: command.positionId });
                 break;
+            case "simulateSwitchIdle":
+                if (endpoint === undefined) {
+                    throw new Error(`Endpoint ${endpointId} not found`);
+                }
+                await endpoint.setStateOf(SwitchServer, { currentPosition: 0 });
+                break;
             default:
                 await super.backchannel(command);
         }

diff --git a/chip-testing/src/NamedPipeCommandHandler.ts b/chip-testing/src/NamedPipeCommandHandler.ts
@@ -34,7 +34,7 @@ export class NamedPipeCommandHandler extends CommandPipe {
         this.#namedPipeSocket = new Socket({ fd: this.#namedPipe.fd });
         console.log(`Named pipe opened: ${this.filename}`);
 
-        this.#namedPipeSocket.on("data", this.onData);
+        this.#namedPipeSocket.on("data", this.onData.bind(this));
 
         this.#namedPipeSocket.on("error", err => {
             console.log("Named pipe error:", err);

diff --git a/chip-testing/test/app-fast/SWTCH.test.ts b/chip-testing/test/app-fast/SWTCH.test.ts
@@ -5,6 +5,10 @@
  */
 
 describe("SWTCH", () => {
-    // There's an unnumbered python SWTCH test with multiple steps
-    chip("SWTCH/*", "SWTCH*");
+    chip(
+        "SWTCH/*",
+
+        // There's an unnumbered python SWTCH test with multiple runs
+        "SWTCH*",
+    );
 });
diff --git a/packages/testing/src/chip/chip.ts b/packages/testing/src/chip/chip.ts
@@ -333,11 +333,11 @@ Object.defineProperties(chipFn, {
         get: () => State.pics,
     },
 
-    initialize: { value: State.initialize },
-    close: { value: State.close },
-    openPipe: { value: State.openPipe },
-    onClose: { value: State.onClose },
-    testFor: { value: State.testFor },
+    initialize: { value: State.initialize.bind(State) },
+    close: { value: State.close.bind(State) },
+    openPipe: { value: State.openPipe.bind(State) },
+    onClose: { value: State.onClose.bind(State) },
+    testFor: { value: State.testFor.bind(State) },
 });
 
 export const chip = chipFn as Chip;

diff --git a/packages/testing/src/chip/command-pipe.ts b/packages/testing/src/chip/command-pipe.ts
@@ -19,7 +19,7 @@ export abstract class CommandPipe {
 
     constructor(subject: BackchannelCommand.Subject, appName: string) {
         this.#subject = subject;
-        this.#filename = `/tmp/${appName}_fifo_1`;
+        this.#filename = `/tmp/chip_${appName}_fifo_${process.pid}`;
     }
 
     get filename() {

diff --git a/packages/testing/src/chip/config.ts b/packages/testing/src/chip/config.ts
@@ -92,8 +92,9 @@ export namespace Constants {
         "--PICS",
         ContainerPaths.matterJsPics,
 
-        // Our PID is meaningless within the container but Python uses in the name of the command pipe
+        // Our PID is meaningless within the container but Python uses in the name of the command pipe.  We pass in our
+        // actual PID to ensure no collision if multiple instances run
         "--app-pid",
-        "1",
+        process.pid.toString(),
     ];
 }
diff --git a/packages/testing/src/chip/container-command-pipe.ts b/packages/testing/src/chip/container-command-pipe.ts
@@ -66,7 +66,7 @@ export class ContainerCommandPipe extends CommandPipe {
             }
         } finally {
             try {
-                await this.#container.delete(this.filename);
+                await this.#container.delete(this.filename, { force: true });
             } catch (e) {
                 console.warn(`Error deleting FIFO ${this.filename}:`, e);
             }

diff --git a/packages/testing/src/chip/state.ts b/packages/testing/src/chip/state.ts
@@ -216,8 +216,13 @@ export const State = {
 
         const pipe = new ContainerCommandPipe(State.container, this, name);
 
+        await pipe.initialize();
+
+        Values.activePipes.add(name);
+
         State.onClose(async () => {
             await pipe.close();
+            Values.activePipes.delete(name);
         });
     },
 
@@ -269,7 +274,7 @@ export const State = {
         const { progress } = State.runner;
 
         await progress.subtask("activating subject", async () => {
-            await State.container.exec(["bash", "-c", "rm -rf /tmp/*"]);
+            await State.container.exec(["bash", "-c", 'export GLOBIGNORE="/tmp/*_fifo_*"; rm -rf /tmp/*']);
 
             if (!startCommissioned) {
                 // Initialize single-use subject

diff --git a/packages/testing/src/device/backchannel.ts b/packages/testing/src/device/backchannel.ts
@@ -10,7 +10,8 @@
 export type BackchannelCommand =
     | BackchannelCommand.SimulateLongPress
     | BackchannelCommand.SimulateMultiPress
-    | BackchannelCommand.SimulateLatchedPosition
+    | BackchannelCommand.SimulateLatchPosition
+    | BackchannelCommand.SimulateSwitchIdle
     | BackchannelCommand.NoParameters;
 
 export namespace BackchannelCommand {
@@ -37,7 +38,12 @@ export namespace BackchannelCommand {
         multiPressMax: number;
     };
 
-    export type SimulateLatchedPosition = {
+    export type SimulateSwitchIdle = {
+        name: "simulateSwitchIdle";
+        endpointId: number;
+    };
+
+    export type SimulateLatchPosition = {
         name: "simulateLatchPosition";
         endpointId: number;
         positionId: number;

diff --git a/packages/testing/src/docker/terminal.ts b/packages/testing/src/docker/terminal.ts
@@ -83,6 +83,12 @@ export namespace Terminal {
                 }
 
                 await promisify(stream.end).bind(stream)();
+
+                // In the case of an exec socket closing one side is insufficient for Dockerode to terminate the stream;
+                // need to actually destroy it to ensure close
+                if ("destroy" in stream && typeof stream.destroy === "function") {
+                    stream.destroy();
+                }
             },
 
             async consume(): Promise<Uint8Array> {