From 572c8a9317de2686291769cbc65ad102feb146dd Mon Sep 17 00:00:00 2001 From: Ingo Fischer Date: Fri, 27 Dec 2024 09:20:35 +0100 Subject: [PATCH] Optimize New-Session-Reconnect handling (#1566) * Optimize New-Session-Reconnect handling There were cases where reconnection processes were overlapping. this change addresses this. * Expose initiation info from session and onle trigger reconnection when external session was opened --- packages/matter.js/src/device/PairedNode.ts | 37 ++++++++++++++----- .../protocol/src/session/SecureSession.ts | 7 ++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/packages/matter.js/src/device/PairedNode.ts b/packages/matter.js/src/device/PairedNode.ts index 270cf75f45..28c3e1da1a 100644 --- a/packages/matter.js/src/device/PairedNode.ts +++ b/packages/matter.js/src/device/PairedNode.ts @@ -73,7 +73,7 @@ import { asClusterClientInternal, isClusterClient } from "./TypeHelpers.js"; const logger = Logger.get("PairedNode"); /** Delay after receiving a changed partList from a device to update the device structure */ -const STRUCTURE_UPDATE_TIMEOUT_MS = 5_000; // 5 seconds, TODO: Verify if this value makes sense in practice +const STRUCTURE_UPDATE_TIMEOUT_MS = 5_000; // 5 seconds /** Delay after a disconnect to try to reconnect to the device */ const RECONNECT_DELAY_MS = 15_000; // 15 seconds @@ -88,7 +88,7 @@ const RECONNECT_MAX_DELAY_MS = 600_000; // 10 minutes * Delay after a new session was opened by the device while in discovery state. * This usually happens for devices that support persisted subscriptions. */ -const NEW_SESSION_WHILE_DISCOVERY_RECONNECT_DELAY = 5_000; +const NEW_SESSION_WHILE_DISCOVERY_RECONNECT_DELAY_MS = 5_000; export enum NodeStates { /** @@ -216,6 +216,23 @@ export class PairedNode { readonly #endpoints = new Map(); #interactionClient: InteractionClient; #reconnectDelayTimer?: Timer; + #newChannelReconnectDelayTimer = Time.getTimer( + "New Channel Reconnect Delay", + NEW_SESSION_WHILE_DISCOVERY_RECONNECT_DELAY_MS, + () => { + if ( + this.#connectionState === NodeStates.WaitingForDeviceDiscovery || + this.#connectionState === NodeStates.Reconnecting + ) { + logger.info( + `Node ${this.nodeId}: Still not connected after new session establishment, trying to reconnect ...`, + ); + // Try last known address first to speed up reconnection + this.#setConnectionState(NodeStates.Reconnecting); + this.#scheduleReconnect(0); + } + }, + ); #reconnectErrorCount = 0; readonly #updateEndpointStructureTimer = Time.getTimer( "Endpoint structure update", @@ -334,15 +351,14 @@ export class PairedNode { logger.info(`Node ${this.nodeId}: Created paired node with device data`, this.#nodeDetails.meta); sessions.added.on(session => { - if (session.peerNodeId !== this.nodeId) return; - if (this.state === NodeStates.WaitingForDeviceDiscovery) { - logger.info( - `Session for peer node ${session.peerNodeId} connected while node is in discovery mode ...`, - ); - // Try last known address first to speed up reconnection - this.#setConnectionState(NodeStates.Reconnecting); - this.#scheduleReconnect(NEW_SESSION_WHILE_DISCOVERY_RECONNECT_DELAY); + if ( + session.isInitiator || // If we initiated the session we do not need to react on it + session.peerNodeId !== this.nodeId || // no session for this node + this.state !== NodeStates.WaitingForDeviceDiscovery + ) { + return; } + this.#newChannelReconnectDelayTimer.stop().start(); }); this.#construction = Construction(this, async () => { @@ -1233,6 +1249,7 @@ export class PairedNode { } close(sendDecommissionedStatus = false) { + this.#newChannelReconnectDelayTimer.stop(); this.#reconnectDelayTimer?.stop(); this.#reconnectDelayTimer = undefined; this.#updateEndpointStructureTimer.stop(); diff --git a/packages/protocol/src/session/SecureSession.ts b/packages/protocol/src/session/SecureSession.ts index d7ef2f2eb4..5454eed38f 100644 --- a/packages/protocol/src/session/SecureSession.ts +++ b/packages/protocol/src/session/SecureSession.ts @@ -41,6 +41,7 @@ export class SecureSession extends Session { #closingAfterExchangeFinished = false; #sendCloseMessageWhenClosing = true; readonly #id: number; + readonly #isInitiator: boolean; #fabric: Fabric | undefined; readonly #peerNodeId: NodeId; readonly #peerSessionId: number; @@ -135,6 +136,7 @@ export class SecureSession extends Session { encryptKey, attestationKey, caseAuthenticatedTags, + isInitiator, } = args; this.#id = id; @@ -145,6 +147,7 @@ export class SecureSession extends Session { this.#encryptKey = encryptKey; this.#attestationKey = attestationKey; this.#caseAuthenticatedTags = caseAuthenticatedTags ?? []; + this.#isInitiator = isInitiator; manager?.sessions.add(this); fabric?.addSession(this); @@ -189,6 +192,10 @@ export class SecureSession extends Session { return this.#subscriptions; } + get isInitiator() { + return this.#isInitiator; + } + get isClosing() { return this.#isClosing; }