From 10b790df7daafd3f501259e5ba44a4a47c12880b Mon Sep 17 00:00:00 2001 From: Ingo Fischer Date: Mon, 16 Dec 2024 23:05:24 +0100 Subject: [PATCH] MDNS FIxes and small stuff (#1530) * Process all TXT/SRV records in the MDNS message And not only the first one * Fix retransmission discovery to 5s * text fixes * Adjust testes for MDNS change * Add count to promise queue * Changelog --- CHANGELOG.md | 2 ++ packages/general/src/util/PromiseQueue.ts | 5 ++++ packages/nodejs/test/IntegrationTest.ts | 5 ++++ .../src/cluster/server/AttributeServer.ts | 6 +++-- .../protocol/src/common/FailsafeContext.ts | 2 +- .../src/interaction/InteractionServer.ts | 2 +- packages/protocol/src/mdns/MdnsScanner.ts | 23 +++++++++++-------- packages/protocol/src/peer/PeerSet.ts | 4 ++-- 8 files changed, 33 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ae2d38a89..644d1b1c39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ The main work (all changes without a GitHub username in brackets in the below li - @matter/protocol - Feature: Reworks Event server handling and optionally allow Non-Volatile event storage (currently mainly used in tests) - Fix: Corrects some Batch invoke checks and logic + - Fix: Fixes MDNS discovery duration for retransmission cases to be 5s + - Fix: process all TXT/SRV records in MDNS response and not just the first one ## 0.11.9 (2024-12-11) diff --git a/packages/general/src/util/PromiseQueue.ts b/packages/general/src/util/PromiseQueue.ts index 45ef7f0989..188f088e84 100644 --- a/packages/general/src/util/PromiseQueue.ts +++ b/packages/general/src/util/PromiseQueue.ts @@ -93,6 +93,11 @@ export class PromiseQueue { this.#queue.length = 0; } + /** Get the number of promises in the queue. */ + get count() { + return this.#queue.length; + } + /** * Close the queue and remove all outstanding promises (but do not reject them). */ diff --git a/packages/nodejs/test/IntegrationTest.ts b/packages/nodejs/test/IntegrationTest.ts index 086273d854..97116bb3a5 100644 --- a/packages/nodejs/test/IntegrationTest.ts +++ b/packages/nodejs/test/IntegrationTest.ts @@ -1911,6 +1911,11 @@ describe("Integration Test", () => { }, }, discoveryData: { + ICD: 0, + SAI: 300, + SAT: 4000, + SII: 500, + T: 0, addresses: [ { ip: SERVER_IPv6, diff --git a/packages/protocol/src/cluster/server/AttributeServer.ts b/packages/protocol/src/cluster/server/AttributeServer.ts index 8e253462ce..39ac752c28 100644 --- a/packages/protocol/src/cluster/server/AttributeServer.ts +++ b/packages/protocol/src/cluster/server/AttributeServer.ts @@ -906,9 +906,11 @@ export class FabricScopedAttributeServer extends AttributeServer { value, FabricIndex.id, session.associatedFabric.fabricIndex, - () => !preserveFabricIndex, // Noone should send any index and if we simply SHALL ignore it, biuut internally we might need it + () => !preserveFabricIndex, // No one should send any index and if we simply SHALL ignore it, but internally we might need it + ); + logger.info( + `Set remote value for fabric scoped attribute "${this.name}" to ${Logger.toJSON(value)} (delayed=${delayChangeEvents})`, ); - logger.info(`Set remote value for fabric scoped attribute "${this.name}" to ${Logger.toJSON(value)}`); super.setRemote(value, session, message, delayChangeEvents); } diff --git a/packages/protocol/src/common/FailsafeContext.ts b/packages/protocol/src/common/FailsafeContext.ts index fcf6b1fc29..c915c0bdcf 100644 --- a/packages/protocol/src/common/FailsafeContext.ts +++ b/packages/protocol/src/common/FailsafeContext.ts @@ -158,7 +158,7 @@ export abstract class FailsafeContext { */ createCertificateSigningRequest(isForUpdateNoc: boolean, sessionId: number) { if (this.#fabrics.findByKeypair(this.#fabricBuilder.keyPair)) { - throw new MatterFlowError("Key pair already exists."); // becomes Failure as StateResponse + throw new MatterFlowError("Key pair already exists."); // becomes Failure as StatusResponse } const result = this.#fabricBuilder.createCertificateSigningRequest(); diff --git a/packages/protocol/src/interaction/InteractionServer.ts b/packages/protocol/src/interaction/InteractionServer.ts index ddfa20077b..dfca915db8 100644 --- a/packages/protocol/src/interaction/InteractionServer.ts +++ b/packages/protocol/src/interaction/InteractionServer.ts @@ -659,7 +659,7 @@ export class InteractionServer implements ProtocolHandler, InteractionRecipient const clusterDataVersionInfo = new Map(); const inaccessiblePaths = new Set(); - // TODO Add handling for moreChunkedMessages here when adopting for Matter 1.3 + // TODO Add handling for moreChunkedMessages here when adopting for Matter 1.4 for (const writeRequest of writeData) { const { path: writePath, dataVersion } = writeRequest; diff --git a/packages/protocol/src/mdns/MdnsScanner.ts b/packages/protocol/src/mdns/MdnsScanner.ts index f861ae0c97..df0c799e97 100644 --- a/packages/protocol/src/mdns/MdnsScanner.ts +++ b/packages/protocol/src/mdns/MdnsScanner.ts @@ -720,24 +720,27 @@ export class MdnsScanner implements Scanner { #handleOperationalRecords(answers: DnsRecord[], formerAnswers: DnsRecord[], netInterface: string) { let recordsHandled = false; // Does the message contain data for an operational service? - const operationalTxtRecord = answers.find( + const operationalTxtRecords = answers.filter( ({ name, recordType }) => recordType === DnsRecordType.TXT && name.endsWith(MATTER_SERVICE_QNAME), ); - if (operationalTxtRecord !== undefined) { - this.#handleOperationalTxtRecord(operationalTxtRecord, netInterface); + if (operationalTxtRecords.length) { + operationalTxtRecords.forEach(record => this.#handleOperationalTxtRecord(record, netInterface)); recordsHandled = true; } - const operationalSrvRecord = - answers.find( - ({ name, recordType }) => recordType === DnsRecordType.SRV && name.endsWith(MATTER_SERVICE_QNAME), - ) ?? - formerAnswers.find( + let operationalSrvRecords = answers.filter( + ({ name, recordType }) => recordType === DnsRecordType.SRV && name.endsWith(MATTER_SERVICE_QNAME), + ); + if (!operationalSrvRecords.length) { + operationalSrvRecords = formerAnswers.filter( ({ name, recordType }) => recordType === DnsRecordType.SRV && name.endsWith(MATTER_SERVICE_QNAME), ); + } - if (operationalSrvRecord !== undefined) { - this.#handleOperationalSrvRecord(operationalSrvRecord, answers, formerAnswers, netInterface); + if (operationalSrvRecords.length) { + operationalSrvRecords.forEach(record => + this.#handleOperationalSrvRecord(record, answers, formerAnswers, netInterface), + ); recordsHandled = true; } return recordsHandled; diff --git a/packages/protocol/src/peer/PeerSet.ts b/packages/protocol/src/peer/PeerSet.ts index 703b7ecc9d..3b3ac7fbb5 100644 --- a/packages/protocol/src/peer/PeerSet.ts +++ b/packages/protocol/src/peer/PeerSet.ts @@ -45,7 +45,7 @@ import { PeerAddressStore, PeerDataStore } from "./PeerAddressStore.js"; const logger = Logger.get("PeerSet"); const RECONNECTION_POLLING_INTERVAL_MS = 600_000; // 10 minutes -const RETRANSMISSION_DISCOVERY_TIMEOUT_MS = 5_000; +const RETRANSMISSION_DISCOVERY_TIMEOUT_S = 5; const CONCURRENT_QUEUED_INTERACTIONS = 4; const INTERACTION_QUEUE_DELAY_MS = 100; @@ -736,7 +736,7 @@ export class PeerSet implements ImmutableSet, ObservableSet { logger.error(`Failed to discover ${address} after resubmission started.`, error); })