Skip to content

Commit

Permalink
[ICD]Add optional common ICD key to indicate ICD operating mode (LIT/…
Browse files Browse the repository at this point in the history
…SIT) (#30135)

* Add optional common ICD key to indicate ICD operating mode (LIT/SIT)

* Update api to check for featuremap bit of the ICD cluster. Check for LongIdleTimeSupport to advertise ICD common key

* replace the use of PRIu16

* Rephrase text in some traces. comment out some code in DiscoveryCommands.cpp as it requires soe changes in zap first

* Addres comments: Renaming, Add feature checks for LIT, helper function to add ICD key to DNS-SD adv

* Convert static var to an alloc in test PtrSrvTxtMultipleRespondersToServiceListing to reduce stack usage due to NRF unit test failure

* address comment

* apply suggestions
  • Loading branch information
jmartinez-silabs authored and pull[bot] committed Apr 23, 2024
1 parent 53a3d7e commit f7aa46f
Show file tree
Hide file tree
Showing 30 changed files with 279 additions and 60 deletions.
5 changes: 5 additions & 0 deletions examples/chip-tool/commands/common/RemoteDataModelLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,11 @@ CHIP_ERROR LogDiscoveredNodeData(const chip::Dnssd::DiscoveredNodeData & nodeDat
value["mrpRetryActiveThreshold"] = resolutionData.mrpRetryActiveThreshold.Value().count();
}

if (resolutionData.isICDOperatingAsLIT.HasValue())
{
value["isICDOperatingAsLIT"] = resolutionData.isICDOperatingAsLIT.Value();
}

Json::Value rootValue;
rootValue[kValueKey] = value;

Expand Down
1 change: 1 addition & 0 deletions examples/chip-tool/commands/discover/Commands.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class Resolve : public DiscoverCommand, public chip::AddressResolve::NodeListene
ChipLogProgress(chipTool, " MRP retry interval (active): %" PRIu32 "ms",
result.mrpRemoteConfig.mActiveRetransTimeout.count());
ChipLogProgress(chipTool, " Supports TCP: %s", result.supportsTcp ? "yes" : "no");
ChipLogProgress(chipTool, " ICD is operating as: %s", result.isICDOperatingAsLIT ? "LIT" : "SIT");
SetCommandExitStatus(CHIP_NO_ERROR);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
<arg name="mrpRetryIntervalIdle" type="int32u" optional="true"/>
<arg name="mrpRetryIntervalActive" type="int32u" optional="true"/>
<arg name="mrpRetryActiveThreshold" type="int16u" optional="true"/>
<arg name="isICDOperatingAsLIT" type="boolean" optional="true"/>
</command>
</cluster>
</configurator>
Expand Down
12 changes: 9 additions & 3 deletions src/app/icd/ICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricT
VerifyOrDie(stateObserver != nullptr);
VerifyOrDie(symmetricKeystore != nullptr);

bool supportLIT = SupportsFeature(Feature::kLongIdleTimeSupport);
VerifyOrDieWithMsg((supportLIT == false) || SupportsFeature(Feature::kCheckInProtocolSupport), AppServer,
"The CheckIn protocol feature is required for LIT support");
VerifyOrDieWithMsg((supportLIT == false) || SupportsFeature(Feature::kUserActiveModeTrigger), AppServer,
"The user ActiveMode trigger feature is required for LIT support");

mStorage = storage;
mFabricTable = fabricTable;
mStateObserver = stateObserver;
Expand Down Expand Up @@ -82,15 +88,15 @@ void ICDManager::Shutdown()
mFabricTable = nullptr;
}

bool ICDManager::SupportsCheckInProtocol()
bool ICDManager::SupportsFeature(Feature feature)
{
bool success = false;
uint32_t featureMap = 0;
// Can't use attribute accessors/Attributes::FeatureMap::Get in unit tests
#ifndef CONFIG_BUILD_FOR_HOST_UNIT_TEST
success = (Attributes::FeatureMap::Get(kRootEndpointId, &featureMap) == EMBER_ZCL_STATUS_SUCCESS);
#endif
return success ? ((featureMap & to_underlying(Feature::kCheckInProtocolSupport)) != 0) : false;
return success ? ((featureMap & to_underlying(feature)) != 0) : false;
}

void ICDManager::UpdateICDMode()
Expand All @@ -101,7 +107,7 @@ void ICDManager::UpdateICDMode()

// The Check In Protocol Feature is required and the slow polling interval shall also be greater than 15 seconds
// to run an ICD in LIT mode.
if (GetSlowPollingInterval() > GetSITPollingThreshold() && SupportsCheckInProtocol())
if (GetSlowPollingInterval() > GetSITPollingThreshold() && SupportsFeature(Feature::kLongIdleTimeSupport))
{
VerifyOrDie(mStorage != nullptr);
VerifyOrDie(mFabricTable != nullptr);
Expand Down
4 changes: 2 additions & 2 deletions src/app/icd/ICDManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
#pragma once

#include <app-common/zap-generated/cluster-enums.h>
#include <app/icd/ICDMonitoringTable.h>
#include <app/icd/ICDNotifier.h>
#include <app/icd/ICDStateObserver.h>
Expand Down Expand Up @@ -58,6 +59,7 @@ class ICDManager : public ICDListener
void UpdateOperationState(OperationalState state);
void SetKeepActiveModeRequirements(KeepActiveFlags flag, bool state);
bool IsKeepActive() { return mKeepActiveFlags.HasAny(); }
bool SupportsFeature(Clusters::IcdManagement::Feature feature);
ICDMode GetICDMode() { return mICDMode; }
OperationalState GetOperationalState() { return mOperationalState; }

Expand Down Expand Up @@ -97,8 +99,6 @@ class ICDManager : public ICDListener
static constexpr uint32_t kMinActiveModeDuration = 300;
static constexpr uint16_t kMinActiveModeThreshold = 300;

bool SupportsCheckInProtocol();

BitFlags<KeepActiveFlags> mKeepActiveFlags{ 0 };

OperationalState mOperationalState = OperationalState::IdleMode;
Expand Down
39 changes: 30 additions & 9 deletions src/app/server/Dnssd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

#include <app/server/Dnssd.h>

#include <app-common/zap-generated/cluster-enums.h>
#include <inttypes.h>

#include <lib/core/Optional.h>
#include <lib/dnssd/Advertiser.h>
#include <lib/dnssd/ServiceNaming.h>
Expand Down Expand Up @@ -145,6 +145,19 @@ CHIP_ERROR DnssdServer::SetEphemeralDiscriminator(Optional<uint16_t> discriminat
return CHIP_NO_ERROR;
}

#if CHIP_CONFIG_ENABLE_ICD_SERVER
template <class AdvertisingParams>
void DnssdServer::AddICDKeyToAdvertisement(AdvertisingParams & advParams)
{
// Only advertise the ICD key if the device can operate as a LIT
if (Server::GetInstance().GetICDManager().SupportsFeature(Clusters::IcdManagement::Feature::kLongIdleTimeSupport))
{
advParams.SetICDOperatingAsLIT(
Optional<bool>(Server::GetInstance().GetICDManager().GetICDMode() == ICDManager::ICDMode::LIT));
}
}
#endif

/// Set MDNS operational advertisement
CHIP_ERROR DnssdServer::AdvertiseOperational()
{
Expand All @@ -165,14 +178,18 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
Crypto::DRBG_get_bytes(macBuffer, sizeof(macBuffer));
}

const auto advertiseParameters = chip::Dnssd::OperationalAdvertisingParameters()
.SetPeerId(fabricInfo.GetPeerId())
.SetMac(mac)
.SetPort(GetSecuredPort())
.SetInterfaceId(GetInterfaceId())
.SetLocalMRPConfig(GetLocalMRPConfig())
.SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT))
.EnableIpV4(true);
auto advertiseParameters = chip::Dnssd::OperationalAdvertisingParameters()
.SetPeerId(fabricInfo.GetPeerId())
.SetMac(mac)
.SetPort(GetSecuredPort())
.SetInterfaceId(GetInterfaceId())
.SetLocalMRPConfig(GetLocalMRPConfig())
.SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT))
.EnableIpV4(true);

#if CHIP_CONFIG_ENABLE_ICD_SERVER
AddICDKeyToAdvertisement(advertiseParameters);
#endif

auto & mdnsAdvertiser = chip::Dnssd::ServiceAdvertiser::Instance();

Expand Down Expand Up @@ -243,6 +260,10 @@ CHIP_ERROR DnssdServer::Advertise(bool commissionableNode, chip::Dnssd::Commissi

advertiseParameters.SetLocalMRPConfig(GetLocalMRPConfig()).SetTcpSupported(Optional<bool>(INET_CONFIG_ENABLE_TCP_ENDPOINT));

#if CHIP_CONFIG_ENABLE_ICD_SERVER
AddICDKeyToAdvertisement(advertiseParameters);
#endif

if (commissionableNode)
{
uint16_t discriminator = 0;
Expand Down
4 changes: 4 additions & 0 deletions src/app/server/Dnssd.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class DLL_EXPORT DnssdServer
void OnExtendedDiscoveryExpiration(System::Layer * aSystemLayer, void * aAppState);
#endif // CHIP_DEVICE_CONFIG_ENABLE_EXTENDED_DISCOVERY

#if CHIP_CONFIG_ENABLE_ICD_SERVER
template <class AdvertisingParams>
void AddICDKeyToAdvertisement(AdvertisingParams & advParams);
#endif
/// Start operational advertising
CHIP_ERROR AdvertiseOperational();

Expand Down
4 changes: 4 additions & 0 deletions src/app/server/Server.h
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,10 @@ class Server

app::reporting::ReportScheduler * GetReportScheduler() { return mReportScheduler; }

#if CHIP_CONFIG_ENABLE_ICD_SERVER
app::ICDManager & GetICDManager() { return mICDManager; }
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER

/**
* This function causes the ShutDown event to be generated async on the
* Matter event loop. Should be called before stopping the event loop.
Expand Down
11 changes: 11 additions & 0 deletions src/app/tests/suites/commands/discovery/DiscoveryCommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,17 @@ void DiscoveryCommands::OnNodeDiscovered(const chip::Dnssd::DiscoveredNodeData &
data.mrpRetryIntervalActive.SetValue(nodeData.resolutionData.mrpRetryIntervalActive.Value().count());
}

// TODO need to add new entries for kDefaultResponse in DiscoveryCommands.js (project-chip/zap) before adding this
// if (nodeData.resolutionData.mrpRetryActiveThreshold.HasValue())
// {
// data.mrpRetryActiveThreshold.SetValue(nodeData.resolutionData.mrpRetryActiveThreshold.Value().count());
// }

// if (nodeData.resolutionData.isICDOperatingAsLIT.HasValue())
// {
// data.isICDOperatingAsLIT.SetValue(resolutionData.isICDOperatingAsLIT.Value());
// }

chip::app::StatusIB status;
status.mStatus = chip::Protocols::InteractionModel::Status::Success;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ void pychip_CommissionableNodeController_PrintDiscoveredCommissioners(
ChipLogProgress(Discovery, "\tMrp Interval active\tNot present");
}
ChipLogProgress(Discovery, "\tSupports TCP\t\t%d", dnsSdInfo->resolutionData.supportsTcp);

if (dnsSdInfo->resolutionData.isICDOperatingAsLIT.HasValue())
{
ChipLogProgress(Discovery, "\tICD is operating as a\t%s",
dnsSdInfo->resolutionData.isICDOperatingAsLIT.Value() ? "LIT" : "SIT");
}

for (unsigned j = 0; j < dnsSdInfo->resolutionData.numIPs; ++j)
{
char buf[chip::Inet::IPAddress::kMaxStringLength];
Expand Down
13 changes: 13 additions & 0 deletions src/controller/python/ChipDeviceController-Discovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ void pychip_DeviceController_IterateDiscoveredCommissionableNodes(Controller::De
{
jsonVal["mrpRetryIntervalActive"] = dnsSdInfo->resolutionData.GetMrpRetryIntervalActive().Value().count();
}
if (dnsSdInfo->resolutionData.GetMrpRetryActiveThreshold().HasValue())
{
jsonVal["mrpRetryActiveThreshold"] = dnsSdInfo->resolutionData.GetMrpRetryActiveThreshold().Value().count();
}
jsonVal["supportsTcp"] = dnsSdInfo->resolutionData.supportsTcp;
{
Json::Value addresses;
Expand All @@ -139,6 +143,10 @@ void pychip_DeviceController_IterateDiscoveredCommissionableNodes(Controller::De
}
jsonVal["addresses"] = addresses;
}
if (dnsSdInfo->resolutionData.isICDOperatingAsLIT.HasValue())
{
jsonVal["isICDOperatingAsLIT"] = dnsSdInfo->resolutionData.isICDOperatingAsLIT.Value();
}
if (dnsSdInfo->commissionData.rotatingIdLen > 0)
{
jsonVal["rotatingId"] = rotatingId;
Expand Down Expand Up @@ -196,6 +204,11 @@ void pychip_DeviceController_PrintDiscoveredDevices(Controller::DeviceCommission
ChipLogProgress(Discovery, "\tMrp Interval active\tNot present");
}
ChipLogProgress(Discovery, "\tSupports TCP\t\t%d", dnsSdInfo->resolutionData.supportsTcp);
if (dnsSdInfo->resolutionData.isICDOperatingAsLIT.HasValue())
{
ChipLogProgress(Discovery, "\tICD is operating as a\t%s",
dnsSdInfo->resolutionData.isICDOperatingAsLIT.Value() ? "LIT" : "SIT");
}
for (unsigned j = 0; j < dnsSdInfo->resolutionData.numIPs; ++j)
{
char buf[Inet::IPAddress::kMaxStringLength];
Expand Down
2 changes: 2 additions & 0 deletions src/controller/python/chip/discovery/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ class CommissionableNode():
pairingHint: int = None
mrpRetryIntervalIdle: int = None
mrpRetryIntervalActive: int = None
mrpRetryActiveThreshold: int = None
supportsTcp: bool = None
isICDOperatingAsLIT: bool = None
addresses: List[str] = None
rotatingId: Optional[str] = None

Expand Down
2 changes: 2 additions & 0 deletions src/controller/python/chip/yaml/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,9 @@ def decode(self, result: _ActionResult):
'pairingHint': response.pairingHint,
'mrpRetryIntervalIdle': response.mrpRetryIntervalIdle,
'mrpRetryIntervalActive': response.mrpRetryIntervalActive,
'mrpRetryActiveThreshold': response.mrpRetryActiveThreshold,
'supportsTcp': response.supportsTcp,
'isICDOperatingAsLIT': response.isICDOperatingAsLIT,
'addresses': response.addresses,
'rotatingId': response.rotatingId,

Expand Down
3 changes: 2 additions & 1 deletion src/lib/address_resolve/AddressResolve.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ struct ResolveResult
{
Transport::PeerAddress address;
ReliableMessageProtocolConfig mrpRemoteConfig;
bool supportsTcp = false;
bool supportsTcp = false;
bool isICDOperatingAsLIT = false;

ResolveResult() : address(Transport::Type::kUdp), mrpRemoteConfig(GetDefaultMRPConfig()) {}
};
Expand Down
5 changes: 5 additions & 0 deletions src/lib/address_resolve/AddressResolve_DefaultImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,11 @@ void Resolver::OnOperationalNodeResolved(const Dnssd::ResolvedNodeData & nodeDat
result.mrpRemoteConfig = nodeData.resolutionData.GetRemoteMRPConfig();
result.supportsTcp = nodeData.resolutionData.supportsTcp;

if (nodeData.resolutionData.isICDOperatingAsLIT.HasValue())
{
result.isICDOperatingAsLIT = nodeData.resolutionData.isICDOperatingAsLIT.Value();
}

for (size_t i = 0; i < nodeData.resolutionData.numIPs; i++)
{
#if !INET_CONFIG_ENABLE_IPV4
Expand Down
4 changes: 3 additions & 1 deletion src/lib/address_resolve/tool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ 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());
ChipLogProgress(Discovery, " MRP ACTIVE Threshold time: %u ms", result.mrpRemoteConfig.mActiveThresholdTime.count());
ChipLogProgress(Discovery, " ICD is operating as a: %s", result.isICDOperatingAsLIT ? "LIT" : "SIT");

NotifyDone();
}

Expand Down
8 changes: 8 additions & 0 deletions src/lib/dnssd/Advertiser.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ class BaseAdvertisingParams
}
Optional<bool> GetTcpSupported() const { return mTcpSupported; }

Derived & SetICDOperatingAsLIT(Optional<bool> operatesAsLIT)
{
mICDOperatesAsLIT = operatesAsLIT;
return *reinterpret_cast<Derived *>(this);
}
Optional<bool> GetICDOperatingAsLIT() const { return mICDOperatesAsLIT; }

private:
uint16_t mPort = CHIP_PORT;
Inet::InterfaceId mInterfaceId = Inet::InterfaceId::Null();
Expand All @@ -109,6 +116,7 @@ class BaseAdvertisingParams
size_t mMacLength = 0;
Optional<ReliableMessageProtocolConfig> mLocalMRPConfig;
Optional<bool> mTcpSupported;
Optional<bool> mICDOperatesAsLIT;
};

/// Defines parameters required for advertising a CHIP node
Expand Down
10 changes: 10 additions & 0 deletions src/lib/dnssd/Advertiser_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
char sessionActiveThresholdBuf[KeySize(TxtFieldKey::kSessionActiveThreshold) +
ValSize(TxtFieldKey::kSessionActiveThreshold) + 2];
char tcpSupportedBuf[KeySize(TxtFieldKey::kTcpSupported) + ValSize(TxtFieldKey::kTcpSupported) + 2];
char operatingICDAsLITBuf[KeySize(TxtFieldKey::kLongIdleTimeICD) + ValSize(TxtFieldKey::kLongIdleTimeICD) + 2];
};
template <class Derived>
CHIP_ERROR AddCommonTxtEntries(const BaseAdvertisingParams<Derived> & params, CommonTxtEntryStorage & storage,
Expand Down Expand Up @@ -280,6 +281,15 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
CHIP_ERROR_INVALID_STRING_LENGTH);
txtFields[numTxtFields++] = storage.tcpSupportedBuf;
}
if (params.GetICDOperatingAsLIT().HasValue())
{
size_t writtenCharactersNumber =
static_cast<size_t>(snprintf(storage.operatingICDAsLITBuf, sizeof(storage.operatingICDAsLITBuf), "ICD=%d",
params.GetICDOperatingAsLIT().Value()));
VerifyOrReturnError((writtenCharactersNumber > 0) && (writtenCharactersNumber < sizeof(storage.operatingICDAsLITBuf)),
CHIP_ERROR_INVALID_STRING_LENGTH);
txtFields[numTxtFields++] = storage.operatingICDAsLITBuf;
}
return CHIP_NO_ERROR;
}

Expand Down
4 changes: 4 additions & 0 deletions src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ CHIP_ERROR CopyTxtRecord(TxtFieldKey key, char * buffer, size_t bufferLen, const
case TxtFieldKey::kSessionActiveInterval:
case TxtFieldKey::kSessionActiveThreshold:
return CopyTextRecordValue(buffer, bufferLen, params.GetLocalMRPConfig(), key);
case TxtFieldKey::kLongIdleTimeICD:
return CopyTextRecordValue(buffer, bufferLen, params.GetICDOperatingAsLIT());
default:
return CHIP_ERROR_INVALID_ARGUMENT;
}
Expand Down Expand Up @@ -569,6 +571,7 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const OperationalAdvertisingParamete
ADD_TXT_RECORD(SessionActiveInterval);
ADD_TXT_RECORD(SessionActiveThreshold);
ADD_TXT_RECORD(TcpSupported);
ADD_TXT_RECORD(LongIdleTimeICD); // Optional, will not be added if related 'params' doesn't have a value

ADD_PTR_RECORD(CompressedFabricId);

Expand All @@ -588,6 +591,7 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter
ADD_TXT_RECORD(SessionActiveInterval);
ADD_TXT_RECORD(SessionActiveThreshold);
ADD_TXT_RECORD(TcpSupported);
ADD_TXT_RECORD(LongIdleTimeICD); // Optional, will not be added if related 'params' doesn't have a value

ADD_PTR_RECORD(VendorId);
ADD_PTR_RECORD(DeviceType);
Expand Down
Loading

0 comments on commit f7aa46f

Please sign in to comment.