[ICD]Add SAT Common TXT key (#28123)

* Add SAT key to the commissionable node adv * Add tests fix some issues * Update after rebase * Fix lint complaint * Fix Darwin Tidy issue, attempt to try Linux mdnsPlatform.Advertise tests * address naming comments * Restyled by clang-format * Fix unit test on linux * address comment --------- Co-authored-by: Restyled.io <[email protected]>
project-chip · Feb 2, 2024 · 1026296 · 1026296
1 parent afb0b57
commit 1026296
Show file tree

Hide file tree

Showing 27 changed files with 284 additions and 109 deletions.
diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp
@@ -832,7 +832,7 @@ CHIP_ERROR ReadClient::ComputeLivenessCheckTimerTimeout(System::Clock::Timeout *
     const auto & ourMrpConfig = GetDefaultMRPConfig();
     auto publisherTransmissionTimeout =
         GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout,
-                                 System::SystemClock().GetMonotonicTimestamp(), Transport::kMinActiveTime);
+                                 System::SystemClock().GetMonotonicTimestamp(), ourMrpConfig.mActiveThresholdTime);
     *aTimeout = System::Clock::Seconds16(mMaxInterval) + publisherTransmissionTimeout;
     return CHIP_NO_ERROR;
 }

diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp
@@ -35,7 +35,7 @@ using namespace chip::app;
 using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::IcdManagement;
 
-void ICDManager::ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable)
+void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable)
 {
     VerifyOrDie(storage != nullptr);
     VerifyOrDie(fabricTable != nullptr);
@@ -44,11 +44,12 @@ void ICDManager::ICDManager::Init(PersistentStorageDelegate * storage, FabricTab
 
     uint32_t activeModeInterval = IcdManagementServer::GetInstance().GetActiveModeInterval();
     VerifyOrDie(kFastPollingInterval.count() < activeModeInterval);
+
     UpdateIcdMode();
     UpdateOperationState(OperationalState::ActiveMode);
 }
 
-void ICDManager::ICDManager::Shutdown()
+void ICDManager::Shutdown()
 {
     // cancel any running timer of the icd
     DeviceLayer::SystemLayer().CancelTimer(OnIdleModeDone, this);

diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp
@@ -4651,7 +4651,7 @@ System::Clock::Timeout TestReadInteraction::ComputeSubscriptionTimeout(System::C
     const auto & ourMrpConfig = GetDefaultMRPConfig();
     auto publisherTransmissionTimeout =
         GetRetransmissionTimeout(ourMrpConfig.mActiveRetransTimeout, ourMrpConfig.mIdleRetransTimeout,
-                                 System::SystemClock().GetMonotonicTimestamp(), Transport::kMinActiveTime);
+                                 System::SystemClock().GetMonotonicTimestamp(), ourMrpConfig.mActiveThresholdTime);
 
     return publisherTransmissionTimeout + aMaxInterval + System::Clock::Milliseconds32(1000);
 }

diff --git a/src/lib/address_resolve/tool.cpp b/src/lib/address_resolve/tool.cpp
@@ -59,6 +59,7 @@ class PrintOutNodeListener : public chip::AddressResolve::NodeListener
         ChipLogProgress(Discovery, "   Supports TCP:                  %s", result.supportsTcp ? "YES" : "NO");
         ChipLogProgress(Discovery, "   MRP IDLE retransmit timeout:   %u ms", result.mrpRemoteConfig.mIdleRetransTimeout.count());
         ChipLogProgress(Discovery, "   MRP ACTIVE retransmit timeout: %u ms", result.mrpRemoteConfig.mActiveRetransTimeout.count());
+        ChipLogProgress(Discovery, "   MRP ACTIVE Threshold timet:    %u ms", result.mrpRemoteConfig.mActiveThresholdTime.count());
         NotifyDone();
     }
 

diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
@@ -213,8 +213,11 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
     struct CommonTxtEntryStorage
     {
         // +2 for all to account for '=' and terminating nullchar
-        char sleepyIdleIntervalBuf[KeySize(TxtFieldKey::kSleepyIdleInterval) + ValSize(TxtFieldKey::kSleepyIdleInterval) + 2];
-        char sleepyActiveIntervalBuf[KeySize(TxtFieldKey::kSleepyActiveInterval) + ValSize(TxtFieldKey::kSleepyActiveInterval) + 2];
+        char sessionIdleIntervalBuf[KeySize(TxtFieldKey::kSessionIdleInterval) + ValSize(TxtFieldKey::kSessionIdleInterval) + 2];
+        char sessionActiveIntervalBuf[KeySize(TxtFieldKey::kSessionActiveInterval) + ValSize(TxtFieldKey::kSessionActiveInterval) +
+                                      2];
+        char sessionActiveThresholdBuf[KeySize(TxtFieldKey::kSessionActiveThreshold) +
+                                       ValSize(TxtFieldKey::kSessionActiveThreshold) + 2];
         char tcpSupportedBuf[KeySize(TxtFieldKey::kTcpSupported) + ValSize(TxtFieldKey::kTcpSupported) + 2];
     };
     template <class Derived>
@@ -234,13 +237,13 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
                     mrp.mIdleRetransTimeout = kMaxRetryInterval;
                 }
                 size_t writtenCharactersNumber =
-                    static_cast<size_t>(snprintf(storage.sleepyIdleIntervalBuf, sizeof(storage.sleepyIdleIntervalBuf),
+                    static_cast<size_t>(snprintf(storage.sessionIdleIntervalBuf, sizeof(storage.sessionIdleIntervalBuf),
                                                  "SII=%" PRIu32, mrp.mIdleRetransTimeout.count()));
                 VerifyOrReturnError((writtenCharactersNumber > 0) &&
-                                        (writtenCharactersNumber < sizeof(storage.sleepyIdleIntervalBuf)),
+                                        (writtenCharactersNumber < sizeof(storage.sessionIdleIntervalBuf)),
                                     CHIP_ERROR_INVALID_STRING_LENGTH);
 
-                txtFields[numTxtFields++] = storage.sleepyIdleIntervalBuf;
+                txtFields[numTxtFields++] = storage.sessionIdleIntervalBuf;
             }
 
             {
@@ -251,12 +254,22 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
                     mrp.mActiveRetransTimeout = kMaxRetryInterval;
                 }
                 size_t writtenCharactersNumber =
-                    static_cast<size_t>(snprintf(storage.sleepyActiveIntervalBuf, sizeof(storage.sleepyActiveIntervalBuf),
+                    static_cast<size_t>(snprintf(storage.sessionActiveIntervalBuf, sizeof(storage.sessionActiveIntervalBuf),
                                                  "SAI=%" PRIu32, mrp.mActiveRetransTimeout.count()));
                 VerifyOrReturnError((writtenCharactersNumber > 0) &&
-                                        (writtenCharactersNumber < sizeof(storage.sleepyActiveIntervalBuf)),
+                                        (writtenCharactersNumber < sizeof(storage.sessionActiveIntervalBuf)),
                                     CHIP_ERROR_INVALID_STRING_LENGTH);
-                txtFields[numTxtFields++] = storage.sleepyActiveIntervalBuf;
+                txtFields[numTxtFields++] = storage.sessionActiveIntervalBuf;
+            }
+
+            {
+                size_t writtenCharactersNumber =
+                    static_cast<size_t>(snprintf(storage.sessionActiveThresholdBuf, sizeof(storage.sessionActiveThresholdBuf),
+                                                 "SAT=%u", mrp.mActiveThresholdTime.count()));
+                VerifyOrReturnError((writtenCharactersNumber > 0) &&
+                                        (writtenCharactersNumber < sizeof(storage.sessionActiveThresholdBuf)),
+                                    CHIP_ERROR_INVALID_STRING_LENGTH);
+                txtFields[numTxtFields++] = storage.sessionActiveThresholdBuf;
             }
         }
         if (params.GetTcpSupported().HasValue())

diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp
@@ -252,20 +252,32 @@ CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, chip::Optional<u
 }
 
 CHIP_ERROR CopyTextRecordValue(char * buffer, size_t bufferLen, const chip::Optional<ReliableMessageProtocolConfig> optional,
-                               bool isIdle)
+                               TxtFieldKey key)
 {
+    VerifyOrReturnError((key == TxtFieldKey::kSessionIdleInterval || key == TxtFieldKey::kSessionActiveInterval ||
+                         key == TxtFieldKey::kSessionActiveThreshold),
+                        CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(optional.HasValue(), CHIP_ERROR_WELL_UNINITIALIZED);
 
-    auto retryInterval = isIdle ? optional.Value().mIdleRetransTimeout : optional.Value().mActiveRetransTimeout;
-
-    if (retryInterval > kMaxRetryInterval)
+    CHIP_ERROR err;
+    if (key == TxtFieldKey::kSessionActiveThreshold)
     {
-        ChipLogProgress(Discovery, "MRP retry interval %s value exceeds allowed range of 1 hour, using maximum available",
-                        isIdle ? "idle" : "active");
-        retryInterval = kMaxRetryInterval;
+        err = CopyTextRecordValue(buffer, bufferLen, optional.Value().mActiveThresholdTime.count());
+    }
+    else
+    {
+        bool isIdle        = (key == TxtFieldKey::kSessionIdleInterval);
+        auto retryInterval = isIdle ? optional.Value().mIdleRetransTimeout : optional.Value().mActiveRetransTimeout;
+        if (retryInterval > kMaxRetryInterval)
+        {
+            ChipLogProgress(Discovery, "MRP retry interval %s value exceeds allowed range of 1 hour, using maximum available",
+                            isIdle ? "idle" : "active");
+            retryInterval = kMaxRetryInterval;
+        }
+        err = CopyTextRecordValue(buffer, bufferLen, retryInterval.count());
     }
 
-    return CopyTextRecordValue(buffer, bufferLen, retryInterval.count());
+    return err;
 }
 
 template <class T>
@@ -275,9 +287,10 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const
     {
     case TxtFieldKey::kTcpSupported:
         return CopyTextRecordValue(buffer, bufferLen, params.GetTcpSupported());
-    case TxtFieldKey::kSleepyIdleInterval:
-    case TxtFieldKey::kSleepyActiveInterval:
-        return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key == TxtFieldKey::kSleepyIdleInterval);
+    case TxtFieldKey::kSessionIdleInterval:
+    case TxtFieldKey::kSessionActiveInterval:
+    case TxtFieldKey::kSessionActiveThreshold:
+        return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key);
     default:
         return CHIP_ERROR_INVALID_ARGUMENT;
     }
@@ -552,8 +565,9 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const OperationalAdvertisingParamete
 {
     PREPARE_RECORDS(Operational);
 
-    ADD_TXT_RECORD(SleepyIdleInterval);
-    ADD_TXT_RECORD(SleepyActiveInterval);
+    ADD_TXT_RECORD(SessionIdleInterval);
+    ADD_TXT_RECORD(SessionActiveInterval);
+    ADD_TXT_RECORD(SessionActiveThreshold);
     ADD_TXT_RECORD(TcpSupported);
 
     ADD_PTR_RECORD(CompressedFabricId);
@@ -570,8 +584,9 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter
     ADD_TXT_RECORD(VendorProduct);
     ADD_TXT_RECORD(DeviceType);
     ADD_TXT_RECORD(DeviceName);
-    ADD_TXT_RECORD(SleepyIdleInterval);
-    ADD_TXT_RECORD(SleepyActiveInterval);
+    ADD_TXT_RECORD(SessionIdleInterval);
+    ADD_TXT_RECORD(SessionActiveInterval);
+    ADD_TXT_RECORD(SessionActiveThreshold);
     ADD_TXT_RECORD(TcpSupported);
 
     ADD_PTR_RECORD(VendorId);

diff --git a/src/lib/dnssd/Resolver.h b/src/lib/dnssd/Resolver.h
@@ -50,6 +50,7 @@ struct CommonResolutionData
     bool supportsTcp                      = false;
     Optional<System::Clock::Milliseconds32> mrpRetryIntervalIdle;
     Optional<System::Clock::Milliseconds32> mrpRetryIntervalActive;
+    Optional<System::Clock::Milliseconds16> mrpRetryActiveThreshold;
 
     CommonResolutionData() { Reset(); }
 
@@ -59,10 +60,12 @@ struct CommonResolutionData
     {
         const ReliableMessageProtocolConfig defaultConfig = GetDefaultMRPConfig();
         return ReliableMessageProtocolConfig(GetMrpRetryIntervalIdle().ValueOr(defaultConfig.mIdleRetransTimeout),
-                                             GetMrpRetryIntervalActive().ValueOr(defaultConfig.mActiveRetransTimeout));
+                                             GetMrpRetryIntervalActive().ValueOr(defaultConfig.mActiveRetransTimeout),
+                                             GetMrpRetryActiveThreshold().ValueOr(defaultConfig.mActiveThresholdTime));
     }
     Optional<System::Clock::Milliseconds32> GetMrpRetryIntervalIdle() const { return mrpRetryIntervalIdle; }
     Optional<System::Clock::Milliseconds32> GetMrpRetryIntervalActive() const { return mrpRetryIntervalActive; }
+    Optional<System::Clock::Milliseconds16> GetMrpRetryActiveThreshold() const { return mrpRetryActiveThreshold; }
 
     bool IsDeviceTreatedAsSleepy(const ReliableMessageProtocolConfig * defaultMRPConfig) const
     {

diff --git a/src/lib/dnssd/TxtFields.cpp b/src/lib/dnssd/TxtFields.cpp
@@ -184,6 +184,18 @@ Optional<System::Clock::Milliseconds32> GetRetryInterval(const ByteSpan & value)
     return NullOptional;
 }
 
+Optional<System::Clock::Milliseconds16> GetRetryActiveThreshold(const ByteSpan & value)
+{
+    const auto retryInterval = MakeU16FromAsciiDecimal(value);
+
+    if (retryInterval == 0)
+    {
+        return NullOptional;
+    }
+
+    return MakeOptional(System::Clock::Milliseconds16(retryInterval));
+}
+
 TxtFieldKey GetTxtFieldKey(const ByteSpan & key)
 {
     for (auto & info : txtFieldInfo)
@@ -237,12 +249,15 @@ void FillNodeDataFromTxt(const ByteSpan & key, const ByteSpan & value, CommonRes
 {
     switch (Internal::GetTxtFieldKey(key))
     {
-    case TxtFieldKey::kSleepyIdleInterval:
+    case TxtFieldKey::kSessionIdleInterval:
         nodeData.mrpRetryIntervalIdle = Internal::GetRetryInterval(value);
         break;
-    case TxtFieldKey::kSleepyActiveInterval:
+    case TxtFieldKey::kSessionActiveInterval:
         nodeData.mrpRetryIntervalActive = Internal::GetRetryInterval(value);
         break;
+    case TxtFieldKey::kSessionActiveThreshold:
+        nodeData.mrpRetryActiveThreshold = Internal::GetRetryActiveThreshold(value);
+        break;
     case TxtFieldKey::kTcpSupported:
         nodeData.supportsTcp = Internal::MakeBoolFromAsciiDecimal(value);
         break;

diff --git a/src/lib/dnssd/TxtFields.h b/src/lib/dnssd/TxtFields.h
@@ -31,8 +31,9 @@ namespace Dnssd {
 using namespace System::Clock::Literals;
 
 // Operational node TXT entries
-static constexpr size_t kKeySleepyIdleIntervalMaxLength          = 7; // [SII] 0-3600000
-static constexpr size_t kKeySleepyActiveIntervalMaxLength        = 7; // [SAI] 0-3600000
+static constexpr size_t kKeySessionIdleIntervalMaxLength         = 7; // [SII] 0-3600000
+static constexpr size_t kKeySessionActiveIntervalMaxLength       = 7; // [SAI] 0-3600000
+static constexpr size_t kKeySessionActiveThresholdMaxLength      = 5; // [SAT] 0-65535
 static constexpr System::Clock::Milliseconds32 kMaxRetryInterval = 3600000_ms32;
 static constexpr size_t kKeyTcpSupportedMaxLength                = 1;
 
@@ -66,8 +67,9 @@ enum class TxtFieldKey : uint8_t
     kRotatingDeviceId,
     kPairingInstruction,
     kPairingHint,
-    kSleepyIdleInterval,
-    kSleepyActiveInterval,
+    kSessionIdleInterval,
+    kSessionActiveInterval,
+    kSessionActiveThreshold,
     kTcpSupported,
     kCount,
 };
@@ -92,8 +94,9 @@ constexpr const TxtFieldInfo txtFieldInfo[static_cast<size_t>(TxtFieldKey::kCoun
     { TxtFieldKey::kRotatingDeviceId, kKeyRotatingDeviceIdMaxLength, "RI", TxtKeyUse::kCommission },
     { TxtFieldKey::kPairingInstruction, kKeyPairingInstructionMaxLength, "PI", TxtKeyUse::kCommission },
     { TxtFieldKey::kPairingHint, kKeyPairingHintMaxLength, "PH", TxtKeyUse::kCommission },
-    { TxtFieldKey::kSleepyIdleInterval, kKeySleepyIdleIntervalMaxLength, "SII", TxtKeyUse::kCommon },
-    { TxtFieldKey::kSleepyActiveInterval, kKeySleepyActiveIntervalMaxLength, "SAI", TxtKeyUse::kCommon },
+    { TxtFieldKey::kSessionIdleInterval, kKeySessionIdleIntervalMaxLength, "SII", TxtKeyUse::kCommon },
+    { TxtFieldKey::kSessionActiveInterval, kKeySessionActiveIntervalMaxLength, "SAI", TxtKeyUse::kCommon },
+    { TxtFieldKey::kSessionActiveThreshold, kKeySessionActiveThresholdMaxLength, "SAT", TxtKeyUse::kCommon },
     { TxtFieldKey::kTcpSupported, kKeyTcpSupportedMaxLength, "T", TxtKeyUse::kCommon },
 };
 #ifdef CHIP_CONFIG_TEST

diff --git a/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h b/src/lib/dnssd/minimal_mdns/tests/CheckOnlyServer.h
@@ -311,7 +311,7 @@ class CheckOnlyServer : private chip::PoolImpl<ServerBase::EndpointInfo, 0, chip
             found = false;
         }
     };
-    static constexpr size_t kMaxExpectedTxt = 11;
+    static constexpr size_t kMaxExpectedTxt = 12;
     KV mExpectedTxt[kMaxExpectedTxt];
     size_t mNumExpectedTxtRecords = 0;
     size_t mNumReceivedTxtRecords = 0;

diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
@@ -80,7 +80,8 @@ OperationalAdvertisingParameters operationalParams1 =
         .SetPort(CHIP_PORT)
         .EnableIpV4(true)
         .SetTcpSupported(chip::Optional<bool>(false))
-        .SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(32_ms32, 30_ms32)); // Match SII, SAI below
+        .SetLocalMRPConfig(chip::Optional<ReliableMessageProtocolConfig>::Value(
+            32_ms32, 30_ms32)); // Match SII, SAI. SAT not provided so it uses default 4000ms
 OperationalAdvertisingParameters operationalParams2 =
     OperationalAdvertisingParameters().SetPeerId(kPeerId2).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true);
 OperationalAdvertisingParameters operationalParams3 =
@@ -91,7 +92,7 @@ OperationalAdvertisingParameters operationalParams5 =
     OperationalAdvertisingParameters().SetPeerId(kPeerId5).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true);
 OperationalAdvertisingParameters operationalParams6 =
     OperationalAdvertisingParameters().SetPeerId(kPeerId6).SetMac(ByteSpan(kMac)).SetPort(CHIP_PORT).EnableIpV4(true);
-const QNamePart txtOperational1Parts[]  = { "SII=32", "SAI=30", "T=0" };
+const QNamePart txtOperational1Parts[]  = { "SII=32", "SAI=30", "SAT=4000", "T=0" };
 PtrResourceRecord ptrOperationalService = PtrResourceRecord(kDnsSdQueryName, kMatterOperationalQueryName);
 PtrResourceRecord ptrOperational1       = PtrResourceRecord(kMatterOperationalQueryName, kInstanceName1);
 SrvResourceRecord srvOperational1       = SrvResourceRecord(kInstanceName1, kHostnameName, CHIP_PORT);
@@ -181,10 +182,10 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
         .SetRotatingDeviceId(chip::Optional<const char *>("id_that_spins"))
         .SetTcpSupported(chip::Optional<bool>(true))
         // 3600005 is more than the max so should be adjusted down
-        .SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32));
+        .SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32, 65535_ms16));
 QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22",          "VP=555+897",       "CM=2",       "DT=70000",
                                                               "DN=testy-test", "RI=id_that_spins", "PI=Pair me", "PH=3",
-                                                              "SAI=3600000",   "SII=3600000",      "T=1" };
+                                                              "SAI=3600000",   "SII=3600000",      "SAT=65535",  "T=1" };
 FullQName txtCommissionableNodeParamsLargeEnhancedName    = FullQName(txtCommissionableNodeParamsLargeEnhancedParts);
 TxtResourceRecord txtCommissionableNodeParamsLargeEnhanced =
     TxtResourceRecord(instanceName, txtCommissionableNodeParamsLargeEnhancedName);