[ICD]Convert the ICD DNS advertiser variable from optional bool to an enum class (#32080)
* Convert the ICD DNS advertiser variable from optional bool to an enum class
* Apply suggestions from code review
Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com>
* default mICDModeAdvertise to kNone
---------
Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com>
diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp
index a804056..1ec2fa0 100644
--- a/src/app/server/Dnssd.cpp
+++ b/src/app/server/Dnssd.cpp
@@ -147,11 +147,21 @@
VerifyOrDieWithMsg(mICDManager != nullptr, Discovery,
"Invalid pointer to the ICDManager which is required for the LIT operating mode");
+ Dnssd::ICDModeAdvertise ICDModeToAdvertise = Dnssd::ICDModeAdvertise::kNone;
// Only advertise the ICD key if the device can operate as a LIT
if (mICDManager->SupportsFeature(Clusters::IcdManagement::Feature::kLongIdleTimeSupport))
{
- advParams.SetICDOperatingAsLIT(Optional<bool>(mICDManager->GetICDMode() == ICDConfigurationData::ICDMode::LIT));
+ if (mICDManager->GetICDMode() == ICDConfigurationData::ICDMode::LIT)
+ {
+ ICDModeToAdvertise = Dnssd::ICDModeAdvertise::kLIT;
+ }
+ else
+ {
+ ICDModeToAdvertise = Dnssd::ICDModeAdvertise::kSIT;
+ }
}
+
+ advParams.SetICDModeToAdvertise(ICDModeToAdvertise);
}
#endif
diff --git a/src/lib/dnssd/Advertiser.h b/src/lib/dnssd/Advertiser.h
index 7c6e295..ddd8df2 100644
--- a/src/lib/dnssd/Advertiser.h
+++ b/src/lib/dnssd/Advertiser.h
@@ -48,6 +48,13 @@
kEnabledEnhanced // Enhanced Commissioning Mode, CM=2 in DNS-SD key/value pairs
};
+enum class ICDModeAdvertise : uint8_t
+{
+ kNone, // The device does not support the LIT feature-set. No ICD= key is advertised in DNS-SD.
+ kSIT, // The ICD supports the LIT feature-set, but is currently operating as a SIT. ICD=0 in DNS-SD key/value pairs.
+ kLIT, // The ICD is currently operating as a LIT. ICD=1 in DNS-SD key/value pairs.
+};
+
template <class Derived>
class BaseAdvertisingParams
{
@@ -103,12 +110,12 @@
}
Optional<bool> GetTcpSupported() const { return mTcpSupported; }
- Derived & SetICDOperatingAsLIT(Optional<bool> operatesAsLIT)
+ Derived & SetICDModeToAdvertise(ICDModeAdvertise operatingMode)
{
- mICDOperatesAsLIT = operatesAsLIT;
+ mICDModeAdvertise = operatingMode;
return *reinterpret_cast<Derived *>(this);
}
- Optional<bool> GetICDOperatingAsLIT() const { return mICDOperatesAsLIT; }
+ ICDModeAdvertise GetICDModeToAdvertise() const { return mICDModeAdvertise; }
private:
uint16_t mPort = CHIP_PORT;
@@ -118,7 +125,7 @@
size_t mMacLength = 0;
Optional<ReliableMessageProtocolConfig> mLocalMRPConfig;
Optional<bool> mTcpSupported;
- Optional<bool> mICDOperatesAsLIT;
+ ICDModeAdvertise mICDModeAdvertise = ICDModeAdvertise::kNone;
};
/// Defines parameters required for advertising a CHIP node
diff --git a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
index b3b1f8e..4c80723 100644
--- a/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
+++ b/src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
@@ -232,13 +232,9 @@
{
auto mrp = optionalMrp.Value();
-#if CHIP_CONFIG_ENABLE_ICD_SERVER
- // An ICD operating as a LIT should not advertise its slow polling interval.
- // When the ICD doesn't support the LIT feature, it doesn't set nor advertise the GetICDOperatingAsLIT entry.
- // Therefore when GetICDOperatingAsLIT has no value or a value of 0, we advertise the slow polling interval
- // otherwise we don't include the SII key in the advertisement.
- if (!params.GetICDOperatingAsLIT().ValueOr(false))
-#endif
+ // An ICD operating as a LIT shall not advertise its slow polling interval.
+ // Don't include the SII key in the advertisement when operating as so.
+ if (params.GetICDModeToAdvertise() != ICDModeAdvertise::kLIT)
{
if (mrp.mIdleRetransTimeout > kMaxRetryInterval)
{
@@ -290,11 +286,11 @@
CHIP_ERROR_INVALID_STRING_LENGTH);
txtFields[numTxtFields++] = storage.tcpSupportedBuf;
}
- if (params.GetICDOperatingAsLIT().HasValue())
+ if (params.GetICDModeToAdvertise() != ICDModeAdvertise::kNone)
{
size_t writtenCharactersNumber =
static_cast<size_t>(snprintf(storage.operatingICDAsLITBuf, sizeof(storage.operatingICDAsLITBuf), "ICD=%d",
- params.GetICDOperatingAsLIT().Value()));
+ (params.GetICDModeToAdvertise() == ICDModeAdvertise::kLIT)));
VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.operatingICDAsLITBuf)),
CHIP_ERROR_INVALID_STRING_LENGTH);
txtFields[numTxtFields++] = storage.operatingICDAsLITBuf;
diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp
index 8b1bbbf..abbc464 100644
--- a/src/lib/dnssd/Discovery_ImplPlatform.cpp
+++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp
@@ -213,19 +213,20 @@
case TxtFieldKey::kSessionIdleInterval:
#if CHIP_CONFIG_ENABLE_ICD_SERVER
// A ICD operating as a LIT should not advertise its slow polling interval
- if (params.GetICDOperatingAsLIT().HasValue() && params.GetICDOperatingAsLIT().Value())
- {
- // Returning UNINITIALIZED ensures that the SII string isn't added by the AddTxtRecord
- // without erroring out the action.
- return CHIP_ERROR_UNINITIALIZED;
- }
+ // Returning UNINITIALIZED ensures that the SII string isn't added by the AddTxtRecord
+ // without erroring out the action.
+ VerifyOrReturnError(params.GetICDModeToAdvertise() != ICDModeAdvertise::kLIT, CHIP_ERROR_UNINITIALIZED);
FALLTHROUGH;
#endif
case TxtFieldKey::kSessionActiveInterval:
case TxtFieldKey::kSessionActiveThreshold:
return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key);
case TxtFieldKey::kLongIdleTimeICD:
- return CopyTextRecordValue(buffer, bufferLen, params.GetICDOperatingAsLIT());
+ // The ICD key is only added to the advertissment when the device supports the ICD LIT feature-set.
+ // Return UNINITIALIZED when the operating mode is kNone to ensure that the ICD string isn't added
+ // by the AddTxtRecord without erroring out the action.
+ VerifyOrReturnError(params.GetICDModeToAdvertise() != ICDModeAdvertise::kNone, CHIP_ERROR_UNINITIALIZED);
+ return CopyTextRecordValue(buffer, bufferLen, (params.GetICDModeToAdvertise() == ICDModeAdvertise::kLIT));
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}
diff --git a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
index aaf7fc6..547d4ee 100644
--- a/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
+++ b/src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
@@ -180,7 +180,7 @@
.SetPairingInstruction(chip::Optional<const char *>("Pair me"))
.SetProductId(chip::Optional<uint16_t>(897))
.SetRotatingDeviceId(chip::Optional<const char *>("id_that_spins"))
- .SetICDOperatingAsLIT(chip::Optional<bool>(false))
+ .SetICDModeToAdvertise(ICDModeAdvertise::kSIT)
// 3600005 is more than the max so should be adjusted down
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32, 65535_ms16));
QNamePart txtCommissionableNodeParamsLargeEnhancedParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000",
@@ -204,7 +204,7 @@
.SetPairingHint(chip::Optional<uint16_t>(3))
.SetPairingInstruction(chip::Optional<const char *>("Pair me"))
.SetProductId(chip::Optional<uint16_t>(897))
- .SetICDOperatingAsLIT(chip::Optional<bool>(true))
+ .SetICDModeToAdvertise(ICDModeAdvertise::kLIT)
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600000_ms32, 65535_ms16));
// With ICD Operation as LIT, SII key will not be added to the advertisement
QNamePart txtCommissionableNodeParamsEnhancedAsICDLITParts[] = { "D=22", "VP=555+897", "CM=2", "DT=70000",
diff --git a/src/lib/dnssd/platform/tests/TestPlatform.cpp b/src/lib/dnssd/platform/tests/TestPlatform.cpp
index f0628d0..91a79f1 100644
--- a/src/lib/dnssd/platform/tests/TestPlatform.cpp
+++ b/src/lib/dnssd/platform/tests/TestPlatform.cpp
@@ -53,7 +53,7 @@
.SetPort(CHIP_PORT)
.EnableIpV4(true)
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(32_ms32, 30_ms32, 10_ms16)) // SII and SAI to match below
- .SetICDOperatingAsLIT(Optional<bool>(false));
+ .SetICDModeToAdvertise(ICDModeAdvertise::kSIT);
test::ExpectedCall operationalCall2 = test::ExpectedCall()
.SetProtocol(DnssdServiceProtocol::kDnssdProtocolTcp)
.SetServiceName("_matter")
@@ -95,7 +95,7 @@
.SetPairingInstruction(Optional<const char *>("Pair me"))
.SetProductId(Optional<uint16_t>(897))
.SetRotatingDeviceId(Optional<const char *>("id_that_spins"))
- .SetICDOperatingAsLIT(Optional<bool>(false))
+ .SetICDModeToAdvertise(ICDModeAdvertise::kSIT)
// 3600005 is over the max, so this should be adjusted by the platform
.SetLocalMRPConfig(Optional<ReliableMessageProtocolConfig>::Value(3600000_ms32, 3600005_ms32, 65535_ms16));
