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
project-chip · Dec 16, 2024 · 10b790d · 10b790d
1 parent 342f5a2
commit 10b790d
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -906,9 +906,11 @@ export class FabricScopedAttributeServer<T> extends AttributeServer<T> {
             value,
             <number>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
@@ -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
@@ -659,7 +659,7 @@ export class InteractionServer implements ProtocolHandler, InteractionRecipient
         const clusterDataVersionInfo = new Map<string, number>();
         const inaccessiblePaths = new Set<string>();
 
-        // 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
@@ -720,24 +720,27 @@ export class MdnsScanner implements Scanner {
     #handleOperationalRecords(answers: DnsRecord<any>[], formerAnswers: DnsRecord<any>[], 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
@@ -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<OperationalPeer>, ObservableSet<Ope
         this.#runningPeerDiscoveries.set(address, { type: NodeDiscoveryType.RetransmissionDiscovery });
         this.#scanners
             .scannerFor(ChannelType.UDP)
-            ?.findOperationalDevice(fabric, nodeId, RETRANSMISSION_DISCOVERY_TIMEOUT_MS, true)
+            ?.findOperationalDevice(fabric, nodeId, RETRANSMISSION_DISCOVERY_TIMEOUT_S, true)
             .catch(error => {
                 logger.error(`Failed to discover ${address} after resubmission started.`, error);
             })