Skip to content

Commit

Permalink
[ICD]Convert the ICD DNS advertiser variable from optional bool to an…
Browse files Browse the repository at this point in the history
… 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>
  • Loading branch information
2 people authored and pull[bot] committed Jul 5, 2024
1 parent a1018ca commit 1788110
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 25 deletions.
12 changes: 11 additions & 1 deletion src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,21 @@ void DnssdServer::AddICDKeyToAdvertisement(AdvertisingParams & advParams)
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

Expand Down
15 changes: 11 additions & 4 deletions src/lib/dnssd/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ enum class CommissioningMode
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
{
Expand Down Expand Up @@ -103,12 +110,12 @@ class BaseAdvertisingParams
}
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;
Expand All @@ -118,7 +125,7 @@ class BaseAdvertisingParams
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
Expand Down
14 changes: 5 additions & 9 deletions src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,9 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
{
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)
{
Expand Down Expand Up @@ -290,11 +286,11 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
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;
Expand Down
15 changes: 8 additions & 7 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,19 +213,20 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const
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;
}
Expand Down
4 changes: 2 additions & 2 deletions src/lib/dnssd/minimal_mdns/tests/TestAdvertiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeEnhanced =
.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",
Expand All @@ -204,7 +204,7 @@ CommissionAdvertisingParameters commissionableNodeParamsEnhancedAsICDLIT =
.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",
Expand Down
4 changes: 2 additions & 2 deletions src/lib/dnssd/platform/tests/TestPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ OperationalAdvertisingParameters operationalParams2 =
.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")
Expand Down Expand Up @@ -95,7 +95,7 @@ CommissionAdvertisingParameters commissionableNodeParamsLargeBasic =
.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));

Expand Down

0 comments on commit 1788110

Please sign in to comment.