From 63c470921de6dc6e271bc19718466088a62f5350 Mon Sep 17 00:00:00 2001 From: Philip Gregor <147669098+pgregorr-amazon@users.noreply.github.com> Date: Mon, 22 Apr 2024 14:09:55 -0700 Subject: [PATCH 1/5] v1.3 Commissioner Passcode field for Linux and Android tv-casting-apps (#33038) --- .../com/matter/casting/DiscoveryExampleFragment.java | 4 ++++ .../jni/com/matter/casting/core/CastingPlayer.java | 2 ++ .../com/matter/casting/core/MatterCastingPlayer.java | 11 ++++++++++- .../app/src/main/jni/cpp/support/Converters-JNI.cpp | 5 +++-- examples/tv-casting-app/linux/simple-app-helper.cpp | 3 ++- .../tv-casting-common/core/CastingPlayer.cpp | 3 +++ .../tv-casting-common/core/CastingPlayer.h | 5 ++++- .../tv-casting-common/core/CastingPlayerDiscovery.cpp | 11 ++++++----- .../tv-casting-common/support/CastingStore.cpp | 10 ++++++++++ .../tv-casting-common/support/CastingStore.h | 2 ++ src/lib/dnssd/TxtFields.cpp | 4 ++-- src/lib/dnssd/Types.h | 8 +++----- src/lib/dnssd/tests/TestTxtFields.cpp | 8 ++++---- 13 files changed, 55 insertions(+), 21 deletions(-) diff --git a/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java b/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java index 49076867a9cc75..a706780c5be3b4 100644 --- a/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java +++ b/examples/tv-casting-app/android/App/app/src/main/java/com/matter/casting/DiscoveryExampleFragment.java @@ -350,6 +350,10 @@ private String getCastingPlayerButtonText(CastingPlayer player) { ? (aux.isEmpty() ? "" : ", ") + "Device Type: " + player.getDeviceType() : ""; aux += (aux.isEmpty() ? "" : ", ") + "Resolved IP?: " + (player.getIpAddresses().size() > 0); + aux += + (aux.isEmpty() ? "" : ", ") + + "Supports Commissioner Generated Passcode: " + + (player.getSupportsCommissionerGeneratedPasscode()); aux = aux.isEmpty() ? aux : "\n" + aux; return main + aux; diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/CastingPlayer.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/CastingPlayer.java index 39db6488fa8ed8..3c3a74032bd313 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/CastingPlayer.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/CastingPlayer.java @@ -49,6 +49,8 @@ public interface CastingPlayer { long getDeviceType(); + boolean getSupportsCommissionerGeneratedPasscode(); + List getEndpoints(); @Override diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/MatterCastingPlayer.java b/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/MatterCastingPlayer.java index dd4bd0ba6531c6..a4f03a00e4f5a2 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/MatterCastingPlayer.java +++ b/examples/tv-casting-app/android/App/app/src/main/jni/com/matter/casting/core/MatterCastingPlayer.java @@ -47,6 +47,8 @@ public class MatterCastingPlayer implements CastingPlayer { private int productId; private int vendorId; private long deviceType; + private boolean supportsCommissionerGeneratedPasscode; + protected long _cppCastingPlayer; public MatterCastingPlayer( @@ -59,7 +61,8 @@ public MatterCastingPlayer( int port, int productId, int vendorId, - long deviceType) { + long deviceType, + boolean supportsCommissionerGeneratedPasscode) { this.connected = connected; this.deviceId = deviceId; this.hostName = hostName; @@ -70,6 +73,7 @@ public MatterCastingPlayer( this.productId = productId; this.vendorId = vendorId; this.deviceType = deviceType; + this.supportsCommissionerGeneratedPasscode = supportsCommissionerGeneratedPasscode; } /** @@ -131,6 +135,11 @@ public long getDeviceType() { return this.deviceType; } + @Override + public boolean getSupportsCommissionerGeneratedPasscode() { + return this.supportsCommissionerGeneratedPasscode; + } + @Override public native List getEndpoints(); diff --git a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/support/Converters-JNI.cpp b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/support/Converters-JNI.cpp index aa5ef2b25f0096..00ca47216ae804 100644 --- a/examples/tv-casting-app/android/App/app/src/main/jni/cpp/support/Converters-JNI.cpp +++ b/examples/tv-casting-app/android/App/app/src/main/jni/cpp/support/Converters-JNI.cpp @@ -151,7 +151,7 @@ jobject convertCastingPlayerFromCppToJava(matter::casting::memory::StrongGetMethodID(matterCastingPlayerJavaClass, "", - "(ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;IIIJ)V"); + "(ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;IIIJZ)V"); if (constructor == nullptr) { ChipLogError(AppServer, "convertCastingPlayerFromCppToJava() could not locate MatterCastingPlayer Java class constructor"); @@ -186,7 +186,8 @@ jobject convertCastingPlayerFromCppToJava(matter::casting::memory::StrongNewStringUTF(player->GetId()), env->NewStringUTF(player->GetHostName()), env->NewStringUTF(player->GetDeviceName()), env->NewStringUTF(player->GetInstanceName()), jIpAddressList, (jint) (player->GetPort()), (jint) (player->GetProductId()), - (jint) (player->GetVendorId()), (jlong) (player->GetDeviceType())); + (jint) (player->GetVendorId()), (jlong) (player->GetDeviceType()), + static_cast(player->GetSupportsCommissionerGeneratedPasscode())); if (jMatterCastingPlayer == nullptr) { ChipLogError(AppServer, "convertCastingPlayerFromCppToJava(): Could not create MatterCastingPlayer Java object"); diff --git a/examples/tv-casting-app/linux/simple-app-helper.cpp b/examples/tv-casting-app/linux/simple-app-helper.cpp index d89b5c25234efc..de8c255366cd37 100644 --- a/examples/tv-casting-app/linux/simple-app-helper.cpp +++ b/examples/tv-casting-app/linux/simple-app-helper.cpp @@ -45,6 +45,7 @@ DiscoveryDelegateImpl * DiscoveryDelegateImpl::GetInstance() void DiscoveryDelegateImpl::HandleOnAdded(matter::casting::memory::Strong player) { + ChipLogProgress(AppServer, "DiscoveryDelegateImpl::HandleOnAdded() called"); if (commissionersCount == 0) { ChipLogProgress(AppServer, "Select discovered Casting Player (start index = 0) to request commissioning"); @@ -58,7 +59,7 @@ void DiscoveryDelegateImpl::HandleOnAdded(matter::casting::memory::Strong player) { - ChipLogProgress(AppServer, "Updated CastingPlayer with ID: %s", player->GetId()); + ChipLogProgress(AppServer, "DiscoveryDelegateImpl::HandleOnUpdated() Updated CastingPlayer with ID: %s", player->GetId()); } void InvokeContentLauncherLaunchURL(matter::casting::memory::Strong endpoint) diff --git a/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.cpp b/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.cpp index fe966722ccf902..c9f938dbc695c7 100644 --- a/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.cpp +++ b/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.cpp @@ -236,6 +236,7 @@ bool CastingPlayer::ContainsDesiredEndpoint(core::CastingPlayer * cachedCastingP void CastingPlayer::LogDetail() const { + ChipLogProgress(AppServer, "CastingPlayer::LogDetail() called"); if (strlen(mAttributes.id) != 0) { ChipLogDetail(AppServer, "\tID: %s", mAttributes.id); @@ -281,6 +282,8 @@ void CastingPlayer::LogDetail() const { ChipLogDetail(AppServer, "\tDevice Type: %" PRIu32, mAttributes.deviceType); } + ChipLogDetail(AppServer, "\tSupports Commissioner Generated Passcode: %s", + mAttributes.supportsCommissionerGeneratedPasscode ? "true" : "false"); if (mAttributes.nodeId > 0) { ChipLogDetail(AppServer, "\tNode ID: 0x" ChipLogFormatX64, ChipLogValueX64(mAttributes.nodeId)); diff --git a/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.h b/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.h index 7d383bb6669225..783d50f60af390 100644 --- a/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.h +++ b/examples/tv-casting-app/tv-casting-common/core/CastingPlayer.h @@ -56,13 +56,14 @@ class CastingPlayerAttributes char deviceName[chip::Dnssd::kMaxDeviceNameLen + 1] = {}; char hostName[chip::Dnssd::kHostNameMaxLength + 1] = {}; char instanceName[chip::Dnssd::Commission::kInstanceNameMaxLength + 1] = {}; - unsigned int numIPs; // number of valid IP addresses + unsigned int numIPs; // Number of valid IP addresses chip::Inet::IPAddress ipAddresses[chip::Dnssd::CommonResolutionData::kMaxIPAddresses]; chip::Inet::InterfaceId interfaceId; uint16_t port; uint16_t productId; uint16_t vendorId; uint32_t deviceType; + bool supportsCommissionerGeneratedPasscode; chip::NodeId nodeId = 0; chip::FabricIndex fabricIndex = 0; @@ -182,6 +183,8 @@ class CastingPlayer : public std::enable_shared_from_this uint32_t GetDeviceType() const { return mAttributes.deviceType; } + bool GetSupportsCommissionerGeneratedPasscode() const { return mAttributes.supportsCommissionerGeneratedPasscode; } + chip::NodeId GetNodeId() const { return mAttributes.nodeId; } chip::FabricIndex GetFabricIndex() const { return mAttributes.fabricIndex; } diff --git a/examples/tv-casting-app/tv-casting-common/core/CastingPlayerDiscovery.cpp b/examples/tv-casting-app/tv-casting-common/core/CastingPlayerDiscovery.cpp index 9b26a868efd8c1..9dd365d8db559f 100644 --- a/examples/tv-casting-app/tv-casting-common/core/CastingPlayerDiscovery.cpp +++ b/examples/tv-casting-app/tv-casting-common/core/CastingPlayerDiscovery.cpp @@ -93,11 +93,12 @@ void DeviceDiscoveryDelegateImpl::OnDiscoveredDevice(const chip::Dnssd::Discover { attributes.ipAddresses[j] = nodeData.resolutionData.ipAddress[j]; } - attributes.interfaceId = nodeData.resolutionData.interfaceId; - attributes.port = nodeData.resolutionData.port; - attributes.productId = nodeData.nodeData.productId; - attributes.vendorId = nodeData.nodeData.vendorId; - attributes.deviceType = nodeData.nodeData.deviceType; + attributes.interfaceId = nodeData.resolutionData.interfaceId; + attributes.port = nodeData.resolutionData.port; + attributes.productId = nodeData.nodeData.productId; + attributes.vendorId = nodeData.nodeData.vendorId; + attributes.deviceType = nodeData.nodeData.deviceType; + attributes.supportsCommissionerGeneratedPasscode = nodeData.nodeData.supportsCommissionerGeneratedPasscode; memory::Strong player = std::make_shared(attributes); diff --git a/examples/tv-casting-app/tv-casting-common/support/CastingStore.cpp b/examples/tv-casting-app/tv-casting-common/support/CastingStore.cpp index c9b9c5450b3641..819c0299945ed4 100644 --- a/examples/tv-casting-app/tv-casting-common/support/CastingStore.cpp +++ b/examples/tv-casting-app/tv-casting-common/support/CastingStore.cpp @@ -182,6 +182,14 @@ std::vector CastingStore::ReadAll() continue; } + if (castingPlayerContainerTagNum == kCastingPlayerSupportsCommissionerGeneratedPasscodeTag) + { + err = reader.Get(attributes.supportsCommissionerGeneratedPasscode); + VerifyOrReturnValue(err == CHIP_NO_ERROR, std::vector(), + ChipLogError(AppServer, "TLVReader.Get failed %" CHIP_ERROR_FORMAT, err.Format())); + continue; + } + if (castingPlayerContainerTagNum == kCastingPlayerPortTag) { err = reader.Get(attributes.port); @@ -472,6 +480,8 @@ CHIP_ERROR CastingStore::WriteAll(std::vector castingPlayer ReturnErrorOnFailure(tlvWriter.Put(chip::TLV::ContextTag(kCastingPlayerVendorIdTag), castingPlayer.GetVendorId())); ReturnErrorOnFailure(tlvWriter.Put(chip::TLV::ContextTag(kCastingPlayerProductIdTag), castingPlayer.GetProductId())); ReturnErrorOnFailure(tlvWriter.Put(chip::TLV::ContextTag(kCastingPlayerDeviceTypeIdTag), castingPlayer.GetDeviceType())); + ReturnErrorOnFailure(tlvWriter.Put(chip::TLV::ContextTag(kCastingPlayerSupportsCommissionerGeneratedPasscodeTag), + castingPlayer.GetSupportsCommissionerGeneratedPasscode())); ReturnErrorOnFailure(tlvWriter.Put(chip::TLV::ContextTag(kCastingPlayerPortTag), castingPlayer.GetPort())); ReturnErrorOnFailure(tlvWriter.PutBytes(chip::TLV::ContextTag(kCastingPlayerInstanceNameTag), (const uint8_t *) castingPlayer.GetInstanceName(), diff --git a/examples/tv-casting-app/tv-casting-common/support/CastingStore.h b/examples/tv-casting-app/tv-casting-common/support/CastingStore.h index 9602af892c9909..c60fa128079122 100644 --- a/examples/tv-casting-app/tv-casting-common/support/CastingStore.h +++ b/examples/tv-casting-app/tv-casting-common/support/CastingStore.h @@ -97,6 +97,8 @@ class CastingStore : public chip::FabricTable::Delegate kCastingPlayerEndpointServerListContainerTag, kCastingPlayerEndpointServerClusterIdTag, + kCastingPlayerSupportsCommissionerGeneratedPasscodeTag, + kContextTagMaxNum = UINT8_MAX }; diff --git a/src/lib/dnssd/TxtFields.cpp b/src/lib/dnssd/TxtFields.cpp index b77d991784867d..c4ec70e342d41a 100644 --- a/src/lib/dnssd/TxtFields.cpp +++ b/src/lib/dnssd/TxtFields.cpp @@ -185,7 +185,7 @@ void GetPairingInstruction(const ByteSpan & value, char * pairingInstruction) uint8_t GetCommissionerPasscode(const ByteSpan & value) { - return MakeU8FromAsciiDecimal(value); + return MakeBoolFromAsciiDecimal(value); } Optional GetRetryInterval(const ByteSpan & value) @@ -256,7 +256,7 @@ void FillNodeDataFromTxt(const ByteSpan & key, const ByteSpan & val, DnssdNodeDa nodeData.pairingHint = Internal::GetPairingHint(val); break; case TxtFieldKey::kCommissionerPasscode: - nodeData.commissionerPasscode = Internal::GetCommissionerPasscode(val); + nodeData.supportsCommissionerGeneratedPasscode = Internal::GetCommissionerPasscode(val); break; default: break; diff --git a/src/lib/dnssd/Types.h b/src/lib/dnssd/Types.h index 15dbf90670ac34..43e55bfbc65e0e 100644 --- a/src/lib/dnssd/Types.h +++ b/src/lib/dnssd/Types.h @@ -214,7 +214,7 @@ struct DnssdNodeData uint16_t productId = 0; uint16_t pairingHint = 0; uint8_t commissioningMode = 0; - uint8_t commissionerPasscode = 0; + bool supportsCommissionerGeneratedPasscode = false; uint8_t rotatingId[kMaxRotatingIdLen] = {}; char instanceName[Commission::kInstanceNameMaxLength + 1] = {}; char deviceName[kMaxDeviceNameLen + 1] = {}; @@ -272,10 +272,8 @@ struct DnssdNodeData ChipLogDetail(Discovery, "\tInstance Name: %s", instanceName); } ChipLogDetail(Discovery, "\tCommissioning Mode: %u", commissioningMode); - if (commissionerPasscode > 0) - { - ChipLogDetail(Discovery, "\tCommissioner Passcode: %u", commissionerPasscode); - } + ChipLogDetail(Discovery, "\tSupports Commissioner Generated Passcode: %s", + supportsCommissionerGeneratedPasscode ? "true" : "false"); } }; diff --git a/src/lib/dnssd/tests/TestTxtFields.cpp b/src/lib/dnssd/tests/TestTxtFields.cpp index 4593a4903b1a01..d6e7e6ea240670 100644 --- a/src/lib/dnssd/tests/TestTxtFields.cpp +++ b/src/lib/dnssd/tests/TestTxtFields.cpp @@ -309,7 +309,7 @@ bool NodeDataIsEmpty(const DiscoveredNodeData & node) node.nodeData.pairingHint != 0 || node.resolutionData.mrpRetryIntervalIdle.HasValue() || node.resolutionData.mrpRetryIntervalActive.HasValue() || node.resolutionData.mrpRetryActiveThreshold.HasValue() || node.resolutionData.isICDOperatingAsLIT.HasValue() || node.resolutionData.supportsTcp || - node.nodeData.commissionerPasscode != 0) + node.nodeData.supportsCommissionerGeneratedPasscode != 0) { return false; } @@ -360,12 +360,12 @@ void TestFillDiscoveredNodeDataFromTxt(nlTestSuite * inSuite, void * inContext) filled.nodeData.commissioningMode = 0; NL_TEST_ASSERT(inSuite, NodeDataIsEmpty(filled)); - // Commissioning mode + // Supports Commissioner Generated Passcode strcpy(key, "CP"); strcpy(val, "1"); FillNodeDataFromTxt(GetSpan(key), GetSpan(val), filled.nodeData); - NL_TEST_ASSERT(inSuite, filled.nodeData.commissionerPasscode == 1); - filled.nodeData.commissionerPasscode = 0; + NL_TEST_ASSERT(inSuite, filled.nodeData.supportsCommissionerGeneratedPasscode == true); + filled.nodeData.supportsCommissionerGeneratedPasscode = false; NL_TEST_ASSERT(inSuite, NodeDataIsEmpty(filled)); // Device type From 2d97bb72fb54616085109539956ab276416dfb57 Mon Sep 17 00:00:00 2001 From: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> Date: Mon, 22 Apr 2024 17:34:08 -0400 Subject: [PATCH 2/5] [ICD] Update and Document ICDManager interface (#33081) * Update and Document ICDManager interface * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Update ICDManagementCluster test to cover off the Check-In message randomness * Address comments and update comments * restyle --------- Co-authored-by: Boris Zbarsky --- .../lit-icd-app/nrfconnect/main/AppTask.cpp | 3 +- examples/platform/silabs/BaseApplication.cpp | 4 +- src/app/icd/server/ICDManager.cpp | 23 +- src/app/icd/server/ICDManager.h | 163 ++++++-- src/app/icd/server/ICDStateObserver.h | 34 +- src/app/reporting/ReportSchedulerImpl.h | 6 + src/app/server/Dnssd.h | 20 +- src/app/tests/TestICDManager.cpp | 393 +++++++++++++++++- .../suites/TestIcdManagementCluster.yaml | 5 +- src/messaging/ExchangeContext.cpp | 2 + 10 files changed, 581 insertions(+), 72 deletions(-) diff --git a/examples/lit-icd-app/nrfconnect/main/AppTask.cpp b/examples/lit-icd-app/nrfconnect/main/AppTask.cpp index d7d35bb53fcc04..336024fe289ea3 100644 --- a/examples/lit-icd-app/nrfconnect/main/AppTask.cpp +++ b/examples/lit-icd-app/nrfconnect/main/AppTask.cpp @@ -296,7 +296,8 @@ void AppTask::ButtonEventHandler(uint32_t buttonState, uint32_t hasChanged) void AppTask::IcdUatEventHandler(const AppEvent &) { - Server::GetInstance().GetICDManager().UpdateOperationState(ICDManager::OperationalState::ActiveMode); + // Temporarily claim network activity, until we implement a "user trigger" reason for ICD wakeups. + PlatformMgr().ScheduleWork([](intptr_t) { ICDNotifier::GetInstance().NotifyNetworkActivityNotification(); }); } void AppTask::FunctionTimerTimeoutCallback(k_timer * timer) diff --git a/examples/platform/silabs/BaseApplication.cpp b/examples/platform/silabs/BaseApplication.cpp index a6986db57d2b3e..f02826f1308c32 100644 --- a/examples/platform/silabs/BaseApplication.cpp +++ b/examples/platform/silabs/BaseApplication.cpp @@ -515,9 +515,7 @@ void BaseApplication::ButtonHandler(AppEvent * aEvent) SILABS_LOG("Network is already provisioned, Ble advertisement not enabled"); #if CHIP_CONFIG_ENABLE_ICD_SERVER // Temporarily claim network activity, until we implement a "user trigger" reason for ICD wakeups. - PlatformMgr().LockChipStack(); - ICDNotifier::GetInstance().NotifyNetworkActivityNotification(); - PlatformMgr().UnlockChipStack(); + PlatformMgr().ScheduleWork([](intptr_t) { ICDNotifier::GetInstance().NotifyNetworkActivityNotification(); }); #endif // CHIP_CONFIG_ENABLE_ICD_SERVER } } diff --git a/src/app/icd/server/ICDManager.cpp b/src/app/icd/server/ICDManager.cpp index 5f93946fa8eebf..8691a4d1360c64 100644 --- a/src/app/icd/server/ICDManager.cpp +++ b/src/app/icd/server/ICDManager.cpp @@ -378,12 +378,13 @@ void ICDManager::UpdateICDMode() if (ICDConfigurationData::GetInstance().GetICDMode() != tempMode) { ICDConfigurationData::GetInstance().SetICDMode(tempMode); - postObserverEvent(ObserverEventType::ICDModeChange); // Can't use attribute accessors/Attributes::OperatingMode::Set in unit tests #if !CONFIG_BUILD_FOR_HOST_UNIT_TEST Attributes::OperatingMode::Set(kRootEndpointId, static_cast(tempMode)); #endif + + postObserverEvent(ObserverEventType::ICDModeChange); } // When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec. @@ -433,6 +434,8 @@ void ICDManager::UpdateOperationState(OperationalState state) { ChipLogError(AppServer, "Failed to set Slow Polling Interval: err %" CHIP_ERROR_FORMAT, err.Format()); } + + postObserverEvent(ObserverEventType::EnterIdleMode); } else if (state == OperationalState::ActiveMode) { @@ -447,7 +450,7 @@ void ICDManager::UpdateOperationState(OperationalState state) if (activeModeDuration == kZero && !mKeepActiveFlags.HasAny()) { - // A Network Activity triggered the active mode and activeModeDuration is 0. + // Network Activity triggered the active mode and activeModeDuration is 0. // Stay active for at least Active Mode Threshold. activeModeDuration = ICDConfigurationData::GetInstance().GetActiveModeThreshold(); } @@ -455,9 +458,14 @@ void ICDManager::UpdateOperationState(OperationalState state) DeviceLayer::SystemLayer().StartTimer(activeModeDuration, OnActiveModeDone, this); Milliseconds32 activeModeJitterInterval = Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS); + // TODO(#33074): Edge case when we transition to IdleMode with this condition being true + // (activeModeDuration == kZero && !mKeepActiveFlags.HasAny()) activeModeJitterInterval = (activeModeDuration >= activeModeJitterInterval) ? activeModeDuration - activeModeJitterInterval : kZero; + // Reset this flag when we enter ActiveMode to avoid having a feedback loop that keeps us indefinitly in + // ActiveMode. + mTransitionToIdleCalled = false; DeviceLayer::SystemLayer().StartTimer(activeModeJitterInterval, OnTransitionToIdle, this); CHIP_ERROR err = @@ -504,10 +512,6 @@ void ICDManager::OnIdleModeDone(System::Layer * aLayer, void * appState) { ICDManager * pICDManager = reinterpret_cast(appState); pICDManager->UpdateOperationState(OperationalState::ActiveMode); - - // We only reset this flag when idle mode is complete to avoid re-triggering the check when an event brings us back to active, - // which could cause a loop. - pICDManager->mTransitionToIdleCalled = false; } void ICDManager::OnActiveModeDone(System::Layer * aLayer, void * appState) @@ -532,10 +536,10 @@ void ICDManager::OnTransitionToIdle(System::Layer * aLayer, void * appState) } /* ICDListener functions. */ + void ICDManager::OnKeepActiveRequest(KeepActiveFlags request) { assertChipStackLockedByCurrentThread(); - VerifyOrReturn(request < KeepActiveFlagsValues::kInvalidFlag); if (request.Has(KeepActiveFlag::kExchangeContextOpen)) @@ -560,7 +564,6 @@ void ICDManager::OnKeepActiveRequest(KeepActiveFlags request) void ICDManager::OnActiveRequestWithdrawal(KeepActiveFlags request) { assertChipStackLockedByCurrentThread(); - VerifyOrReturn(request < KeepActiveFlagsValues::kInvalidFlag); if (request.Has(KeepActiveFlag::kExchangeContextOpen)) @@ -697,6 +700,10 @@ void ICDManager::postObserverEvent(ObserverEventType event) obs->mObserver->OnEnterActiveMode(); return Loop::Continue; } + case ObserverEventType::EnterIdleMode: { + obs->mObserver->OnEnterIdleMode(); + return Loop::Continue; + } case ObserverEventType::TransitionToIdle: { obs->mObserver->OnTransitionToIdle(); return Loop::Continue; diff --git a/src/app/icd/server/ICDManager.h b/src/app/icd/server/ICDManager.h index 69dfdbf6a88863..bf6e439c62ef52 100644 --- a/src/app/icd/server/ICDManager.h +++ b/src/app/icd/server/ICDManager.h @@ -57,7 +57,9 @@ class TestICDManager; class ICDManager : public ICDListener, public TestEventTriggerHandler { public: - // This structure is used for the creation an ObjectPool of ICDStateObserver pointers + /** + * @brief This structure is used for the creation an ObjectPool of ICDStateObserver pointers + */ struct ObserverPointer { ObserverPointer(ICDStateObserver * obs) : mObserver(obs) {} @@ -71,11 +73,31 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler ActiveMode, }; - // This enum class represents to all ICDStateObserver callbacks available from the - // mStateObserverPool for the ICDManager. + /** + * @brief This enum class represents to all ICDStateObserver callbacks available from the + * mStateObserverPool for the ICDManager. + * + * EnterActiveMode, TransitionToIdle and EnterIdleMode will always be called as a trio in the same order. + * Each event will only be called once per cycle. + * EnterActiveMode will always be called first, when the ICD has transitioned to ActiveMode. + * TransitionToIdle will always be second. This event will only be called the first time there is + * `ICD_ACTIVE_TIME_JITTER_MS` remaining to the ActiveMode timer. + * When this event is called, the ICD is still in ActiveMode. + * If the ActiveMode timer is increased due to the TransitionToIdle event, the event will not be called a second time in + * a given cycle. + * OnEnterIdleMode will always the third when the ICD has transitioned to IdleMode. + * + * The ICDModeChange event can occur independently from the EnterActiveMode, TransitionToIdle and EnterIdleMode. + * It will typpically hapen at the ICDManager init when a client is already registered with the ICD before the + * OnEnterIdleMode event or when a client send a register command after the OnEnterActiveMode event. Nothing prevents the + * ICDModeChange event to happen multiple times per cycle or while the ICD is in IdleMode. + * + * See src/app/icd/server/ICDStateObserver.h for more information on the APIs each event triggers + */ enum class ObserverEventType : uint8_t { EnterActiveMode, + EnterIdleMode, TransitionToIdle, ICDModeChange, }; @@ -85,22 +107,31 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler * This type can be used to implement specific verifiers that can be used in the CheckInMessagesWouldBeSent function. * The goal is to avoid having multiple functions that implement the iterator loop with only the check changing. * - * @return true if at least one Check-In message would be sent - * false No Check-In messages would be sent + * @return true: if at least one Check-In message would be sent + * false: No Check-In messages would be sent */ - using ShouldCheckInMsgsBeSentFunction = bool(FabricIndex aFabricIndex, NodeId subjectID); - ICDManager() {} + ICDManager() = default; + ~ICDManager() = default; + void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, Crypto::SymmetricKeystore * symmetricKeyStore, - Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * manager); + Messaging::ExchangeManager * exchangeManager, SubscriptionsInfoProvider * subInfoProvider); void Shutdown(); - void UpdateICDMode(); - void UpdateOperationState(OperationalState state); - void SetKeepActiveModeRequirements(KeepActiveFlags flag, bool state); - bool IsKeepActive() { return mKeepActiveFlags.HasAny(); } + + /** + * @brief SupportsFeature verifies if a given FeatureMap bit is enabled + * + * @param[in] feature FeatureMap bit to verify + * + * @return true: if the FeatureMap bit is enabled in the ICDM cluster attribute. + * false: if the FeatureMap bit is not enabled in the ICDM cluster attribute. + * if we failed to read the FeatureMap attribute. + */ bool SupportsFeature(Clusters::IcdManagement::Feature feature); + ICDConfigurationData::ICDMode GetICDMode() { return ICDConfigurationData::GetInstance().GetICDMode(); }; + /** * @brief Adds the referenced observer in parameters to the mStateObserverPool * A maximum of CHIP_CONFIG_ICD_OBSERVERS_POOL_SIZE observers can be concurrently registered @@ -111,20 +142,14 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler /** * @brief Remove the referenced observer in parameters from the mStateObserverPool + * If the observer is not present in the object pool, we do nothing */ void ReleaseObserver(ICDStateObserver * observer); - /** - * @brief Associates the ObserverEventType parameters to the correct - * ICDStateObservers function and calls it for all observers in the mStateObserverPool - */ - void postObserverEvent(ObserverEventType event); - OperationalState GetOperationalState() { return mOperationalState; } - /** * @brief Ensures that the remaining Active Mode duration is at least the smaller of 30000 milliseconds and stayActiveDuration. * - * @param stayActiveDuration The duration (in milliseconds) requested by the client to stay in Active Mode + * @param[in] stayActiveDuration The duration (in milliseconds) requested by the client to stay in Active Mode * @return The duration (in milliseconds) the device will stay in Active Mode */ uint32_t StayActiveRequest(uint32_t stayActiveDuration); @@ -132,15 +157,14 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler /** * @brief TestEventTriggerHandler for the ICD feature set * - * @param eventTrigger Event trigger to handle. + * @param[in] eventTrigger Event trigger to handle. + * * @return CHIP_ERROR CHIP_NO_ERROR - No erros during the processing * CHIP_ERROR_INVALID_ARGUMENT - eventTrigger isn't a valid value */ CHIP_ERROR HandleEventTrigger(uint64_t eventTrigger) override; #if CHIP_CONFIG_ENABLE_ICD_CIP - void SendCheckInMsgs(); - /** * @brief Trigger the ICDManager to send Check-In message if necessary * @@ -160,47 +184,108 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler #if CONFIG_BUILD_FOR_HOST_UNIT_TEST void SetTestFeatureMapValue(uint32_t featureMap) { mFeatureMap = featureMap; }; -#if !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS +#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS && !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION bool GetIsBootUpResumeSubscriptionExecuted() { return mIsBootUpResumeSubscriptionExecuted; }; #endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS #endif // Implementation of ICDListener functions. // Callers must origin from the chip task context or hold the ChipStack lock. + void OnNetworkActivity() override; void OnKeepActiveRequest(KeepActiveFlags request) override; void OnActiveRequestWithdrawal(KeepActiveFlags request) override; void OnICDManagementServerEvent(ICDManagementEvents event) override; void OnSubscriptionReport() override; -protected: +private: friend class TestICDManager; + /** + * @brief UpdateICDMode evaluates in which mode the ICD can be in; SIT or LIT mode. + * If the current operating mode does not match the evaluated operating mode, function updates the ICDMode and triggers + * all necessary operations. + * For a SIT ICD, this function does nothing. + * For a LIT ICD, the function checks if the ICD has a registration in the ICDMonitoringTable to determine which ICDMode + * the ICD must be in. + */ + void UpdateICDMode(); /** - * @brief Hepler function that extends the Active Mode duration as well as the Active Mode Jitter timer for the transition to - * iddle mode. + * @brief UpdateOperationState updates the OperationState of the ICD to the requested one. + * IdleMode -> IdleMode : No actions are necessary, do nothing. + * IdleMode -> ActiveMode : Transition the device to ActiveMode, start the ActiveMode timer and trigger all necessary + * operations. These operations could be : Send Check-In messages + * Send subscription reports + * Process user actions + * ActiveMode -> ActiveMode : Increase remaining ActiveMode timer to one ActiveModeThreshold. + * If ActiveModeThreshold is 0, do nothing. + * ActiveMode -> IdleMode : Transition ICD to IdleMode and start the IdleMode timer. + * + * @param state requested OperationalState for the ICD to transition to + */ + void UpdateOperationState(OperationalState state); + + /** + * @brief Set or Remove a keep ActiveMode requirement for the given flag + * If state is true and the ICD is in IdleMode, transition the ICD to ActiveMode + * If state is false and the ICD is in ActiveMode, check whether we can transition the ICD to IdleMode. + * If we can, transition the ICD to IdleMode. + * + * @param flag KeepActiveFlag to remove or add + * @param state true: adding a flag requirement + * false: removing a flag requirement + */ + void SetKeepActiveModeRequirements(KeepActiveFlags flag, bool state); + + /** + * @brief Associates the ObserverEventType parameters to the correct + * ICDStateObservers function and calls it for all observers in the mStateObserverPool + */ + void postObserverEvent(ObserverEventType event); + + /** + * @brief Hepler function that extends the ActiveMode timer as well as the Active Mode Jitter timer for the transition to + * idle mode event. */ void ExtendActiveMode(System::Clock::Milliseconds16 extendDuration); + /** + * @brief Timer callback function for when the IdleMode timer expires + * + * @param appState pointer to the ICDManager + */ static void OnIdleModeDone(System::Layer * aLayer, void * appState); + + /** + * @brief Timer callback function for when the ActiveMode timer expires + * + * @param appState pointer to the ICDManager + */ static void OnActiveModeDone(System::Layer * aLayer, void * appState); /** - * @brief Callback function called shortly before the device enters idle mode to allow checks to be made. This is currently only - * called once to prevent entering in a loop if some events re-trigger this check (for instance if a check for subscription - * before entering idle mode leads to emiting a report, we will re-enter UpdateOperationState and check again for subscription, - * etc.) + * @brief Timer Callback function called shortly before the device enters idle mode to allow checks to be made. + * This is currently only called once to prevent entering in a loop if some events re-trigger this check (for instance if + * a check for subscriptions before entering idle mode leads to emiting a report, we will re-enter UpdateOperationState + * and check again for subscription, etc.) + * + * @param appState pointer to the ICDManager */ static void OnTransitionToIdle(System::Layer * aLayer, void * appState); #if CHIP_CONFIG_ENABLE_ICD_CIP - uint8_t mCheckInRequestCount = 0; -#endif // CHIP_CONFIG_ENABLE_ICD_CIP - - uint8_t mOpenExchangeContextCount = 0; + /** + * @brief Function triggers all necessary Check-In messages to be sent. + * + * @note For each ICDMonitoring entry, we check if should send a Check-In message with + * ShouldCheckInMsgsBeSentAtActiveModeFunction. If we should, we allocate an ICDCheckInSender which tries to send a + * Check-In message to the registered client. + */ + void SendCheckInMsgs(); -private: -#if CHIP_CONFIG_ENABLE_ICD_CIP + /** + * @brief See function implementation in .cpp for details on this function. + */ bool ShouldCheckInMsgsBeSentAtActiveModeFunction(FabricIndex aFabricIndex, NodeId subjectID); /** @@ -221,11 +306,15 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler OperationalState mOperationalState = OperationalState::ActiveMode; bool mTransitionToIdleCalled = false; ObjectPool mStateObserverPool; + uint8_t mOpenExchangeContextCount = 0; #if CHIP_CONFIG_ENABLE_ICD_CIP + uint8_t mCheckInRequestCount = 0; + #if !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS bool mIsBootUpResumeSubscriptionExecuted = false; #endif // !CHIP_CONFIG_SUBSCRIPTION_TIMEOUT_RESUMPTION && CHIP_CONFIG_PERSIST_SUBSCRIPTIONS + PersistentStorageDelegate * mStorage = nullptr; FabricTable * mFabricTable = nullptr; Messaging::ExchangeManager * mExchangeManager = nullptr; diff --git a/src/app/icd/server/ICDStateObserver.h b/src/app/icd/server/ICDStateObserver.h index 996dad0ec6608b..34fc309b466449 100644 --- a/src/app/icd/server/ICDStateObserver.h +++ b/src/app/icd/server/ICDStateObserver.h @@ -27,13 +27,43 @@ namespace chip { namespace app { +/** + * @brief Public API used by the ICDManager to expose when different events occur. + * ICDManager::RegisterObserver can be used to register as an Observer to be notified when these events occur. + * These functions are called synchronously. + */ class ICDStateObserver { public: virtual ~ICDStateObserver() {} - virtual void OnEnterActiveMode() = 0; + + /** + * @brief API called when the ICD enters ActiveMode. API isn't called if we need to extend the remaining active mode timer + * duration. API is called after the ICDManager has finished executing its internal actions. + */ + virtual void OnEnterActiveMode() = 0; + + /** + * @brief API called when the ICD enters IdleMode. + * API is called after the ICDManager has finished executing its internal actions. + */ + virtual void OnEnterIdleMode() = 0; + + /** + * @brief API is called when the ICD is about to enter IdleMode. API is called when there is `ICD_ACTIVE_TIME_JITTER_MS` of time + * remaining to the active mode timer. + * This API is only called once per transition from ActiveMode to IdleMode. + * If OnTransitionToIdle triggers the active mode timer to increase, the next time we are about to enter IdleMode, + * this API will not be called. + */ virtual void OnTransitionToIdle() = 0; - virtual void OnICDModeChange() = 0; + + /** + * @brief API is called when the ICD changes operating mode. This API is only called if the ICD changes state, not when it + * remains in the same state. + * API is called after the ICDManager has finished executing its internal actions. + */ + virtual void OnICDModeChange() = 0; }; } // namespace app diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index 38fcc2ae1bf941..f7889a5ac3e6c7 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -89,6 +89,12 @@ class ReportSchedulerImpl : public ReportScheduler */ void OnICDModeChange() override{}; + /** + * @brief This implementation does not attempt any synchronization on this ICD event, therefore no action is needed on + * ICDEnterIdleMode() + */ + void OnEnterIdleMode() override{}; + // ReadHandlerObserver /** diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h index e669f4d0860a50..105318ca5a08be 100644 --- a/src/app/server/Dnssd.h +++ b/src/app/server/Dnssd.h @@ -126,11 +126,25 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver */ CHIP_ERROR SetEphemeralDiscriminator(Optional discriminator); - // ICDStateObserver - // No action is needed by the DnssdServer on active or idle state entries + /** + * @brief When the ICD changes operating mode, the dnssd server needs to restart its DNS-SD advertising to update the TXT keys. + */ + void OnICDModeChange() override; + + /** + * @brief dnssd server has no action to do on this ICD event. Do nothing. + */ void OnEnterActiveMode() override{}; + + /** + * @brief dnssd server has no action to do on this ICD event. Do nothing. + */ void OnTransitionToIdle() override{}; - void OnICDModeChange() override; + + /** + * @brief dnssd server has no action to do on this ICD event. Do nothing. + */ + void OnEnterIdleMode() override{}; private: /// Overloaded utility method for commissioner and commissionable advertisement diff --git a/src/app/tests/TestICDManager.cpp b/src/app/tests/TestICDManager.cpp index fd93a146cf2755..6e814ebb01f579 100644 --- a/src/app/tests/TestICDManager.cpp +++ b/src/app/tests/TestICDManager.cpp @@ -77,9 +77,27 @@ enum class ICDTestEventTriggerEvent : uint64_t class TestICDStateObserver : public app::ICDStateObserver { public: - void OnEnterActiveMode() {} - void OnTransitionToIdle() {} - void OnICDModeChange() {} + void OnEnterActiveMode() { mOnEnterActiveModeCalled = true; } + void OnEnterIdleMode() { mOnEnterIdleModeCalled = true; } + void OnTransitionToIdle() { mOnTransitionToIdleCalled = true; } + void OnICDModeChange() { mOnICDModeChangeCalled = true; } + + void ResetOnEnterActiveMode() { mOnEnterActiveModeCalled = false; } + void ResetOnEnterIdleMode() { mOnEnterIdleModeCalled = false; } + void ResetOnTransitionToIdle() { mOnTransitionToIdleCalled = false; } + void ResetOnICDModeChange() { mOnICDModeChangeCalled = false; } + void ResetAll() + { + ResetOnEnterActiveMode(); + ResetOnEnterIdleMode(); + ResetOnTransitionToIdle(); + ResetOnICDModeChange(); + } + + bool mOnEnterActiveModeCalled = false; + bool mOnEnterIdleModeCalled = false; + bool mOnICDModeChangeCalled = false; + bool mOnTransitionToIdleCalled = false; }; class TestSubscriptionsInfoProvider : public SubscriptionsInfoProvider @@ -123,10 +141,10 @@ class TestContext : public chip::Test::AppContext // Performs setup for each individual test in the test suite CHIP_ERROR SetUp() override { - ReturnErrorOnFailure(chip::Test::AppContext::SetUp()); - mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider); + mICDStateObserver.ResetAll(); mICDManager.RegisterObserver(&mICDStateObserver); + mICDManager.Init(&testStorage, &GetFabricTable(), &mKeystore, &GetExchangeManager(), &mSubInfoProvider); return CHIP_NO_ERROR; } @@ -196,7 +214,7 @@ class TestICDManager /** * @brief Test verifies that the ICDManager starts its timers correctly based on if it will have any messages to send - * when the IdleModeDuration expires + * when the IdleMode timer expires */ static void TestICDModeDurationsWith0ActiveModeDurationWithoutActiveSub(nlTestSuite * aSuite, void * aContext) { @@ -254,7 +272,7 @@ class TestICDManager AdvanceClockAndRunEventLoop(ctx, icdConfigData.GetActiveModeThreshold() + 1_ms16); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode); - // Expire IdleModeDuration - Device should be in ActiveMode since it has an ICDM registration + // Expire IdleMode timer - Device should be in ActiveMode since it has an ICDM registration AdvanceClockAndRunEventLoop(ctx, icdConfigData.GetIdleModeDuration() + 1_s); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode); @@ -276,7 +294,7 @@ class TestICDManager /** * @brief Test verifies that the ICDManager remains in IdleMode since it will not have any messages to send - * when the IdleModeDuration expires + * when the IdleMode timer expires */ static void TestICDModeDurationsWith0ActiveModeDurationWithActiveSub(nlTestSuite * aSuite, void * aContext) { @@ -334,7 +352,7 @@ class TestICDManager AdvanceClockAndRunEventLoop(ctx, icdConfigData.GetActiveModeThreshold() + 1_ms16); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode); - // Expire IdleModeDuration - Device stay in IdleMode since it has an active subscription for the ICDM entry + // Expire IdleMode timer - Device stay in IdleMode since it has an active subscription for the ICDM entry AdvanceClockAndRunEventLoop(ctx, icdConfigData.GetIdleModeDuration() + 1_s); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode); @@ -414,9 +432,9 @@ class TestICDManager NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode); } - /* - * Test that verifies that the ICDManager is the correct operating mode based on entries - * in the ICDMonitoringTable + /** + * @brief Test that verifies that the ICDManager is in the correct operating mode based on entries + * in the ICDMonitoringTable */ static void TestICDMRegisterUnregisterEvents(nlTestSuite * aSuite, void * aContext) { @@ -554,7 +572,9 @@ class TestICDManager NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode); } - /* Test that verifies the logic of the ICDManager when it receives a StayActiveRequest*/ + /** + * @brief Test verifies the logic of the ICDManager when it receives a StayActiveRequest + */ static void TestICDMStayActive(nlTestSuite * aSuite, void * aContext) { TestContext * ctx = static_cast(aContext); @@ -568,7 +588,7 @@ class TestICDManager notifier.NotifySubscriptionReport(); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode); - // Advance time by the ActiveModeDuration - 1 + // Advance time just before ActiveMode timer expires AdvanceClockAndRunEventLoop(ctx, icdConfigData.GetActiveModeDuration() - 1_ms32); // Confirm ICD manager is in active mode NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode); @@ -580,7 +600,7 @@ class TestICDManager NL_TEST_ASSERT(aSuite, stayActivePromisedMs == stayActiveRequestedMs); // Advance time by the duration of the stay stayActiveRequestedMs - 1 ms - AdvanceClockAndRunEventLoop(ctx, System::Clock::Milliseconds32(stayActiveRequestedMs) - 1_ms32); + AdvanceClockAndRunEventLoop(ctx, Milliseconds32(stayActiveRequestedMs) - 1_ms32); // Confirm ICD manager is in active mode NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode); @@ -601,7 +621,7 @@ class TestICDManager NL_TEST_ASSERT(aSuite, stayActivePromisedMs == 30000); // Advance time by the duration of the max stay active duration - 1 ms - AdvanceClockAndRunEventLoop(ctx, System::Clock::Milliseconds32(30000) - 1_ms32); + AdvanceClockAndRunEventLoop(ctx, Milliseconds32(30000) - 1_ms32); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode); // Advance time by 1ms and Confirm ICD manager is in idle mode @@ -621,7 +641,7 @@ class TestICDManager NL_TEST_ASSERT(aSuite, stayActivePromisedMs == 30000); // Advance time by the duration of the stay active request - 20000 ms - AdvanceClockAndRunEventLoop(ctx, System::Clock::Milliseconds32(stayActiveRequestedMs) - 20000_ms32); + AdvanceClockAndRunEventLoop(ctx, Milliseconds32(stayActiveRequestedMs) - 20000_ms32); // Confirm ICD manager is in active mode, we should have 20000 seconds left at that point NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::ActiveMode); @@ -721,6 +741,329 @@ class TestICDManager ctx->mICDManager.HandleEventTrigger(static_cast(ICDTestEventTriggerEvent::kRemoveActiveModeReq)); NL_TEST_ASSERT(aSuite, ctx->mICDManager.mOperationalState == ICDManager::OperationalState::IdleMode); } + + /** + * @brief Test verifies when OnEnterIdleMode is called during normal operations. + * Without the ActiveMode timer being extended + */ + static void TestICDStateObserverOnEnterIdleModeActiveModeDuration(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Verify that ICDManager starts in IdleMode and calls OnEnterIdleMode + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnEnterIdleModeCalled); + ctx->mICDStateObserver.ResetOnEnterIdleMode(); + + // Advance clock just before IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Expire IdleModeInterval + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Advance clock Just before ActiveMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeDuration() - 1_ms32); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Expire ActiveMode timer + AdvanceClockAndRunEventLoop(ctx, 1_ms32); + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnEnterIdleModeCalled); + } + + /** + * @brief Test verifies when OnEnterIdleMode is called with the ActiveMode timer gets extended + */ + static void TestICDStateObserverOnEnterIdleModeActiveModeThreshold(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Verify that ICDManager starts in IdleMode and calls OnEnterIdleMode + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnEnterIdleModeCalled); + ctx->mICDStateObserver.ResetOnEnterIdleMode(); + + // Advance clock just before the IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Expire IdleMode timer + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Advance clock Just before ActiveMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeDuration() - 1_ms32); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Increase ActiveMode timer by one ActiveModeThreshold + ICDNotifier::GetInstance().NotifyNetworkActivityNotification(); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Advance clock Just before ActiveMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeThreshold() - 1_ms32); + NL_TEST_ASSERT(aSuite, !ctx->mICDStateObserver.mOnEnterIdleModeCalled); + + // Expire ActiveMode timer + AdvanceClockAndRunEventLoop(ctx, 1_ms32); + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnEnterIdleModeCalled); + } + + static void TestICDStateObserverOnEnterActiveMode(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Verify OnEnterActiveMode wasn't called at Init + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Advance clock just before IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Expire IdleMode timer and check wether OnEnterActiveMode was called + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnEnterActiveModeCalled); + ctx->mICDStateObserver.ResetOnEnterActiveMode(); + + // Advance clock just before the ActiveMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeDuration() - 1_ms32); + + // Verify OnEnterActiveMde wasn't called + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Increase ActiveMode timer by one ActiveModeThreshold + ICDNotifier::GetInstance().NotifyNetworkActivityNotification(); + + // Verify OnEnterActiveMde wasn't called + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Advance clock just before ActiveMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeThreshold() - 1_ms32); + + // Verify OnEnterActiveMde wasn't called + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Expire ActiveMode timer + AdvanceClockAndRunEventLoop(ctx, 1_ms32); + + // Verify OnEnterActiveMde wasn't called + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Advance clock just before IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnEnterActiveModeCalled)); + + // Expire IdleMode timer and check OnEnterActiveMode was called + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnEnterActiveModeCalled); + } + + static void TestICDStateObserverOnICDModeChange(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + typedef ICDListener::ICDManagementEvents ICDMEvent; + + // Since we don't have a registration, we stay in SIT mode. No changes + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnICDModeChangeCalled)); + + // Trigger register event to force ICDManager to re-evaluate OperatingMode + ICDNotifier::GetInstance().NotifyICDManagementEvent(ICDMEvent::kTableUpdated); + + // Since we don't have a registration, we stay in SIT mode. No changes + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnICDModeChangeCalled)); + + // Add an entry to the ICDMonitoringTable + ICDMonitoringTable table(ctx->testStorage, kTestFabricIndex1, kMaxTestClients, &(ctx->mKeystore)); + + ICDMonitoringEntry entry(&(ctx->mKeystore)); + entry.checkInNodeID = kClientNodeId11; + entry.monitoredSubject = kClientNodeId11; + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == entry.SetKey(ByteSpan(kKeyBuffer1a))); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == table.Set(0, entry)); + + // Trigger register event after first entry was added + ICDNotifier::GetInstance().NotifyICDManagementEvent(ICDMEvent::kTableUpdated); + + // We have a registration. Transition to LIT mode + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnICDModeChangeCalled); + ctx->mICDStateObserver.ResetOnICDModeChange(); + + // Trigger register event to force ICDManager to re-evaluate OperatingMode + ICDNotifier::GetInstance().NotifyICDManagementEvent(ICDMEvent::kTableUpdated); + + // We have a registration. We stay in LIT mode. No changes. + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnICDModeChangeCalled)); + + // Remove entry from the ICDMonitoringTable + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == table.Remove(0)); + ICDNotifier::GetInstance().NotifyICDManagementEvent(ICDMEvent::kTableUpdated); + + // Since we don't have a registration anymore. Transition to SIT mode. + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnICDModeChangeCalled); + ctx->mICDStateObserver.ResetOnICDModeChange(); + } + + static void TestICDStateObserverOnICDModeChangeOnInit(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + ICDMonitoringTable table(ctx->testStorage, kTestFabricIndex1, kMaxTestClients, &(ctx->mKeystore)); + + // Since we don't have a registration, we stay in SIT mode. No changes + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnICDModeChangeCalled)); + + // Add an entry to the ICDMonitoringTable + ICDMonitoringEntry entry(&(ctx->mKeystore)); + entry.checkInNodeID = kClientNodeId11; + entry.monitoredSubject = kClientNodeId11; + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == entry.SetKey(ByteSpan(kKeyBuffer1a))); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == table.Set(0, entry)); + + // Shut down and reinit ICDManager - We should go to LIT mode since we have a registration + ctx->mICDManager.Shutdown(); + ctx->mICDManager.RegisterObserver(&(ctx->mICDStateObserver)); + ctx->mICDManager.Init(&(ctx->testStorage), &(ctx->GetFabricTable()), &(ctx->mKeystore), &(ctx->GetExchangeManager()), + &(ctx->mSubInfoProvider)); + + // We have a registration, transition to LIT mode + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnICDModeChangeCalled); + ctx->mICDStateObserver.ResetOnICDModeChange(); + + // Remove entry from the ICDMonitoringTable + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == table.Remove(0)); + } + + /** + * @brief Test verifies the OnTransitionToIdleMode event when the ActiveModeDuration is greater than the + * ICD_ACTIVE_TIME_JITTER_MS + */ + static void TestICDStateObserverOnTransitionToIdleModeGreaterActiveModeDuration(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Set New durations for test case - ActiveModeDuration must be longuer than ICD_ACTIVE_TIME_JITTER_MS + Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration(); + ICDConfigurationData::GetInstance().SetModeDurations( + MakeOptional(Milliseconds32(200) + Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)), NullOptional); + + // Advance clock just before IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Expire IdleMode timer + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Advance time just before OnTransitionToIdleMode is called + AdvanceClockAndRunEventLoop( + ctx, ICDConfigurationData::GetInstance().GetActiveModeDuration() - Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS) - 1_ms32); + + // Check mOnTransitionToIdleCalled has not been called + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Increase ActiveMode timer by one ActiveModeThreshold + ICDNotifier::GetInstance().NotifyNetworkActivityNotification(); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Advance time just before OnTransitionToIdleMode is called + AdvanceClockAndRunEventLoop( + ctx, ICDConfigurationData::GetInstance().GetActiveModeThreshold() - Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS) - 1_ms32); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Expire OnTransitionToIdleMode + AdvanceClockAndRunEventLoop(ctx, 1_ms32); + // Check mOnTransitionToIdleCalled has been called + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnTransitionToIdleCalled); + ctx->mICDStateObserver.ResetOnTransitionToIdle(); + + // Expire ActiveMode timer + AdvanceClockAndRunEventLoop(ctx, Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Reset Old durations + ICDConfigurationData::GetInstance().SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional); + } + + /** + * @brief Test verifies the OnTransitionToIdleMode event when the ActiveModeDuration is equal to the + * ICD_ACTIVE_TIME_JITTER_MS. + */ + static void TestICDStateObserverOnTransitionToIdleModeEqualActiveModeDuration(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Set New durations for test case - ActiveModeDuration must be equal to ICD_ACTIVE_TIME_JITTER_MS + Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration(); + ICDConfigurationData::GetInstance().SetModeDurations( + MakeOptional(Milliseconds32(ICD_ACTIVE_TIME_JITTER_MS)), NullOptional); + + // Advance clock just before IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Expire IdleMode timer + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Expire OnTransitionToIdleMode + AdvanceClockAndRunEventLoop(ctx, 1_ms32); + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnTransitionToIdleCalled); + + // Reset Old durations + ICDConfigurationData::GetInstance().SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional); + } + + /** + * @brief Test verifies the OnTransitionToIdleMode event when the ActiveModeDuration is 0 and without an ActiveMode req + */ + static void TestICDStateObserverOnTransitionToIdleMode0ActiveModeDurationWithoutReq(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Set New durations for test case - ActiveModeDuration equal 0 + Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration(); + ICDConfigurationData::GetInstance().SetModeDurations(MakeOptional(0), NullOptional); + + // Advance clock just before IdleMode timer expires + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration() - 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Expire IdleMode timer + AdvanceClockAndRunEventLoop(ctx, 1_s); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Increase time by 1 - Should not trigger OnTransitionToIdle. + // Timer length is one ActiveModeThreshold + AdvanceClockAndRunEventLoop(ctx, 1_ms32); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Expire ActiveModeThreshold + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetActiveModeThreshold()); + NL_TEST_ASSERT(aSuite, ctx->mICDStateObserver.mOnTransitionToIdleCalled); + + // Reset Old durations + ICDConfigurationData::GetInstance().SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional); + } + + /** + * @brief Test verifies the OnTransitionToIdleMode event when the ActiveModeDuration is 0 with an ActiveMode req + */ + static void TestICDStateObserverOnTransitionToIdleMode0ActiveModeDurationWittReq(nlTestSuite * aSuite, void * aContext) + { + TestContext * ctx = static_cast(aContext); + + // Set New durations for test case - ActiveModeDuration equal 0 + Milliseconds32 oldActiveModeDuration = ICDConfigurationData::GetInstance().GetActiveModeDuration(); + ICDConfigurationData::GetInstance().SetModeDurations(MakeOptional(0), NullOptional); + + // Add ActiveMode req for the Test event trigger event + ctx->mICDManager.HandleEventTrigger(static_cast(ICDTestEventTriggerEvent::kAddActiveModeReq)); + + // Expire IdleMode timer + AdvanceClockAndRunEventLoop(ctx, ICDConfigurationData::GetInstance().GetIdleModeDuration()); + NL_TEST_ASSERT(aSuite, !(ctx->mICDStateObserver.mOnTransitionToIdleCalled)); + + // Reset Old durations + ICDConfigurationData::GetInstance().SetModeDurations(MakeOptional(oldActiveModeDuration), NullOptional); + } }; } // namespace app @@ -740,6 +1083,22 @@ static const nlTest sTests[] = { NL_TEST_DEF("TestICDStayActive", TestICDManager::TestICDMStayActive), NL_TEST_DEF("TestShouldCheckInMsgsBeSentAtActiveModeFunction", TestICDManager::TestShouldCheckInMsgsBeSentAtActiveModeFunction), NL_TEST_DEF("TestHandleTestEventTriggerActiveModeReq", TestICDManager::TestHandleTestEventTriggerActiveModeReq), + NL_TEST_DEF("TestICDStateObserverOnEnterIdleModeActiveModeDuration", + TestICDManager::TestICDStateObserverOnEnterIdleModeActiveModeDuration), + NL_TEST_DEF("TestICDStateObserverOnEnterIdleModeActiveModeThreshold", + TestICDManager::TestICDStateObserverOnEnterIdleModeActiveModeThreshold), + NL_TEST_DEF("TestICDStateObserverOnEnterActiveMode", TestICDManager::TestICDStateObserverOnEnterActiveMode), + NL_TEST_DEF("TestICDStateObserverOnICDModeChange", TestICDManager::TestICDStateObserverOnICDModeChange), + NL_TEST_DEF("TestICDStateObserverOnICDModeChangeOnInit", TestICDManager::TestICDStateObserverOnICDModeChangeOnInit), + NL_TEST_DEF("TestICDStateObserverOnTransitionToIdleModeGreaterActiveModeDuration", + TestICDManager::TestICDStateObserverOnTransitionToIdleModeGreaterActiveModeDuration), + NL_TEST_DEF("TestICDStateObserverOnTransitionToIdleModeEqualActiveModeDuration", + TestICDManager::TestICDStateObserverOnTransitionToIdleModeEqualActiveModeDuration), + NL_TEST_DEF("TestICDStateObserverOnTransitionToIdleMode0ActiveModeDurationWithoutReq", + TestICDManager::TestICDStateObserverOnTransitionToIdleMode0ActiveModeDurationWithoutReq), + // TODO(#33074): When the OnTransitionToIdle edge is fixed, we can enable this test + // NL_TEST_DEF("TestICDStateObserverOnTransitionToIdleMode0ActiveModeDurationWittReq", + // TestICDManager::TestICDStateObserverOnTransitionToIdleMode0ActiveModeDurationWittReq), NL_TEST_SENTINEL(), }; diff --git a/src/app/tests/suites/TestIcdManagementCluster.yaml b/src/app/tests/suites/TestIcdManagementCluster.yaml index 3d0081504c19bf..f7f207ab8df310 100644 --- a/src/app/tests/suites/TestIcdManagementCluster.yaml +++ b/src/app/tests/suites/TestIcdManagementCluster.yaml @@ -87,7 +87,10 @@ tests: command: "readAttribute" attribute: "ICDCounter" response: - value: beforeRebootICDCounter + 101 + constraints: + # It is possible ICD hasn't had time to send a Check-In message after reboot + minValue: beforeRebootICDCounter + 100 + maxValue: beforeRebootICDCounter + 101 - label: "Verify the ICD is operating as a LIT ICD" command: "readAttribute" diff --git a/src/messaging/ExchangeContext.cpp b/src/messaging/ExchangeContext.cpp index 74a861c1170b7f..3869ab4c48c921 100644 --- a/src/messaging/ExchangeContext.cpp +++ b/src/messaging/ExchangeContext.cpp @@ -322,6 +322,7 @@ ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, cons SetAutoRequestAck(session->AllowsMRP()); #if CHIP_CONFIG_ENABLE_ICD_SERVER + // TODO(#33075) : Add check for group context to not a req since it serves no purpose app::ICDNotifier::GetInstance().NotifyActiveRequestNotification(app::ICDListener::KeepActiveFlag::kExchangeContextOpen); #endif @@ -341,6 +342,7 @@ ExchangeContext::~ExchangeContext() VerifyOrDie(mFlags.Has(Flags::kFlagClosed)); #if CHIP_CONFIG_ENABLE_ICD_SERVER + // TODO(#33075) : Add check for group context to not a req since it serves no purpose app::ICDNotifier::GetInstance().NotifyActiveRequestWithdrawal(app::ICDListener::KeepActiveFlag::kExchangeContextOpen); #endif // CHIP_CONFIG_ENABLE_ICD_SERVER From 40761d1a9bae658585cce73561bdffdbb92b8fe5 Mon Sep 17 00:00:00 2001 From: Sid Hsu Date: Tue, 23 Apr 2024 05:57:16 +0800 Subject: [PATCH 3/5] Update the checkout_submodules.py script to support the recursive configuration in the .gitmodules file. (#33089) --- scripts/checkout_submodules.py | 51 ++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/scripts/checkout_submodules.py b/scripts/checkout_submodules.py index aa220f7ff3b139..81dafa1fb91a29 100755 --- a/scripts/checkout_submodules.py +++ b/scripts/checkout_submodules.py @@ -52,7 +52,7 @@ 'silabs_docker', ]) -Module = namedtuple('Module', 'name path platforms') +Module = namedtuple('Module', 'name path platforms recursive') def load_module_info() -> None: @@ -63,9 +63,11 @@ def load_module_info() -> None: if name != 'DEFAULT': platforms = module.get('platforms', '').split(',') platforms = set(filter(None, platforms)) - assert not (platforms - ALL_PLATFORMS), "Submodule's platform not contained in ALL_PLATFORMS" + assert not ( + platforms - ALL_PLATFORMS), "Submodule's platform not contained in ALL_PLATFORMS" + recursive = module.getboolean('recursive', False) name = name.replace('submodule "', '').replace('"', '') - yield Module(name=name, path=module['path'], platforms=platforms) + yield Module(name=name, path=module['path'], platforms=platforms, recursive=recursive) def module_matches_platforms(module: Module, platforms: set) -> bool: @@ -88,8 +90,10 @@ def make_chip_root_safe_directory() -> None: config.check_returncode() existing = config.stdout.split('\0') if CHIP_ROOT not in existing: - logging.info("Adding CHIP_ROOT to global git safe.directory configuration") - subprocess.check_call(['git', 'config', '--global', '--add', 'safe.directory', CHIP_ROOT]) + logging.info( + "Adding CHIP_ROOT to global git safe.directory configuration") + subprocess.check_call( + ['git', 'config', '--global', '--add', 'safe.directory', CHIP_ROOT]) def checkout_modules(modules: list, shallow: bool, force: bool, recursive: bool, jobs: int) -> None: @@ -101,9 +105,21 @@ def checkout_modules(modules: list, shallow: bool, force: bool, recursive: bool, cmd += ['--force'] if force else [] cmd += ['--recursive'] if recursive else [] cmd += ['--jobs', f'{jobs}'] if jobs else [] - cmd += [module.path for module in modules] + module_paths = [module.path for module in modules] - subprocess.check_call(cmd) + subprocess.check_call(cmd + module_paths) + + if recursive: + # We've recursively checkouted all submodules. + pass + else: + # We've checkouted all top-level submodules. + # We're going to recursively checkout submodules whose recursive configuration is true. + cmd += ['--recursive'] + module_paths = [module.path for module in modules if module.recursive] + + if module_paths: + subprocess.check_call(cmd + module_paths) def deinit_modules(modules: list, force: bool) -> None: @@ -120,28 +136,35 @@ def deinit_modules(modules: list, force: bool) -> None: def main(): logging.basicConfig(format='%(message)s', level=logging.INFO) - parser = argparse.ArgumentParser(description='Checkout or update relevant git submodules') + parser = argparse.ArgumentParser( + description='Checkout or update relevant git submodules') parser.add_argument('--allow-changing-global-git-config', action='store_true', help='Allow global git options to be modified if necessary, e.g. safe.directory') - parser.add_argument('--shallow', action='store_true', help='Fetch submodules without history') + parser.add_argument('--shallow', action='store_true', + help='Fetch submodules without history') parser.add_argument('--platform', nargs='+', choices=ALL_PLATFORMS, default=[], help='Process submodules for specific platforms only') - parser.add_argument('--force', action='store_true', help='Perform action despite of warnings') + parser.add_argument('--force', action='store_true', + help='Perform action despite of warnings') parser.add_argument('--deinit-unmatched', action='store_true', help='Deinitialize submodules for non-matching platforms') - parser.add_argument('--recursive', action='store_true', help='Recursive init of the listed submodules') - parser.add_argument('--jobs', type=int, metavar='N', help='Clone new submodules in parallel with N jobs') + parser.add_argument('--recursive', action='store_true', + help='Recursive init of the listed submodules') + parser.add_argument('--jobs', type=int, metavar='N', + help='Clone new submodules in parallel with N jobs') args = parser.parse_args() modules = list(load_module_info()) selected_platforms = set(args.platform) - selected_modules = [m for m in modules if module_matches_platforms(m, selected_platforms)] + selected_modules = [ + m for m in modules if module_matches_platforms(m, selected_platforms)] unmatched_modules = [m for m in modules if not module_matches_platforms( m, selected_platforms) and module_initialized(m)] if args.allow_changing_global_git_config: make_chip_root_safe_directory() # ignore directory ownership issues for sub-modules - checkout_modules(selected_modules, args.shallow, args.force, args.recursive, args.jobs) + checkout_modules(selected_modules, args.shallow, + args.force, args.recursive, args.jobs) if args.deinit_unmatched and unmatched_modules: deinit_modules(unmatched_modules, args.force) From 8dd2de7c0201c2803bb6de31152f2d4013c66a42 Mon Sep 17 00:00:00 2001 From: Mikhail Burshteyn Date: Mon, 22 Apr 2024 16:16:20 -0600 Subject: [PATCH 4/5] Update README.md to fix a spelling mistake (#33097) --- scripts/py_matter_idl/matter_idl/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/py_matter_idl/matter_idl/README.md b/scripts/py_matter_idl/matter_idl/README.md index bd4d3ef468188d..602e8565fac829 100644 --- a/scripts/py_matter_idl/matter_idl/README.md +++ b/scripts/py_matter_idl/matter_idl/README.md @@ -182,7 +182,7 @@ directory of this README). Most of the heavy lifting is done by the lark using content - [matter_idl_parser.py](./matter_idl_parser.py) has a transformer that converts the text given by lark into a more type-safe (and type-rich) AST as - defined ing [matter_idl_types.py](./matter_idl_types.py) + defined in [matter_idl_types.py](./matter_idl_types.py) ## Code generation From 9ff29ed04029f14275cb661fb197aa71754e2d4d Mon Sep 17 00:00:00 2001 From: joonhaengHeo <85541460+joonhaengHeo@users.noreply.github.com> Date: Tue, 23 Apr 2024 13:33:43 +0900 Subject: [PATCH 5/5] Fix Android Bluetooth connect issue (#33087) --- .../chip/chiptool/bluetooth/BluetoothManager.kt | 16 ++++++++++++++++ .../java/chip/platform/AndroidBleManager.java | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/bluetooth/BluetoothManager.kt b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/bluetooth/BluetoothManager.kt index 16d0f745b5f70f..61e95838b1cc61 100644 --- a/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/bluetooth/BluetoothManager.kt +++ b/examples/android/CHIPTool/app/src/main/java/com/google/chip/chiptool/bluetooth/BluetoothManager.kt @@ -138,6 +138,12 @@ class BluetoothManager : BleCallback { private val coroutineContinuation = continuation + private val STATE_INIT = 1 + private val STATE_DISCOVER_SERVICE = 2 + private val STATE_REQUEST_MTU = 3 + + private var mState = STATE_INIT + override fun onConnectionStateChange(gatt: BluetoothGatt?, status: Int, newState: Int) { super.onConnectionStateChange(gatt, status, newState) Log.i( @@ -148,21 +154,31 @@ class BluetoothManager : BleCallback { if (newState == BluetoothProfile.STATE_CONNECTED && status == BluetoothGatt.GATT_SUCCESS) { Log.i("$TAG|onConnectionStateChange", "Discovering Services...") + mState = STATE_DISCOVER_SERVICE gatt?.discoverServices() } } override fun onServicesDiscovered(gatt: BluetoothGatt?, status: Int) { Log.d(TAG, "${gatt?.device?.name}.onServicesDiscovered status = $status") + if (mState != STATE_DISCOVER_SERVICE) { + Log.d(TAG, "Invalid state : $mState") + return + } wrappedCallback.onServicesDiscovered(gatt, status) Log.i("$TAG|onServicesDiscovered", "Services Discovered") + mState = STATE_REQUEST_MTU gatt?.requestMtu(247) } override fun onMtuChanged(gatt: BluetoothGatt?, mtu: Int, status: Int) { Log.d(TAG, "${gatt?.device?.name}.onMtuChanged: connecting to CHIP device") super.onMtuChanged(gatt, mtu, status) + if (mState != STATE_REQUEST_MTU) { + Log.d(TAG, "Invalid state : $mState") + return + } wrappedCallback.onMtuChanged(gatt, mtu, status) if (coroutineContinuation.isActive) { coroutineContinuation.resume(gatt) diff --git a/src/platform/android/java/chip/platform/AndroidBleManager.java b/src/platform/android/java/chip/platform/AndroidBleManager.java index 1d440b67b18da3..52b0052d409ef8 100644 --- a/src/platform/android/java/chip/platform/AndroidBleManager.java +++ b/src/platform/android/java/chip/platform/AndroidBleManager.java @@ -584,12 +584,19 @@ private void connectBLE(Object bluetoothDeviceObj) { } class ConnectionGattCallback extends AndroidBluetoothGattCallback { + private static final int STATE_INIT = 1; + private static final int STATE_DISCOVER_SERVICE = 2; + private static final int STATE_REQUEST_MTU = 3; + + private int mState = STATE_INIT; + @Override public void onConnectionStateChange(BluetoothGatt gatt, int status, int newState) { Log.i(TAG, "onConnectionStateChange status = " + status + ", newState= + " + newState); super.onConnectionStateChange(gatt, status, newState); if (newState == BluetoothProfile.STATE_CONNECTED && status == BluetoothGatt.GATT_SUCCESS) { Log.i(TAG, "Discovering Services..."); + mState = STATE_DISCOVER_SERVICE; gatt.discoverServices(); return; } else if (newState == BluetoothProfile.STATE_DISCONNECTED) { @@ -602,14 +609,23 @@ public void onConnectionStateChange(BluetoothGatt gatt, int status, int newState public void onServicesDiscovered(BluetoothGatt gatt, int status) { Log.d(TAG, "onServicesDiscovered status = " + status); super.onServicesDiscovered(gatt, status); + if (mState != STATE_DISCOVER_SERVICE) { + Log.d(TAG, "Invalid state : " + mState); + return; + } Log.i(TAG, "Services Discovered"); + mState = STATE_REQUEST_MTU; gatt.requestMtu(247); } @Override public void onMtuChanged(BluetoothGatt gatt, int mtu, int status) { super.onMtuChanged(gatt, mtu, status); + if (mState != STATE_REQUEST_MTU) { + Log.d(TAG, "Invalid state : " + mState); + return; + } String deviceName = ""; if (gatt != null && gatt.getDevice() != null) { deviceName = gatt.getDevice().getName();