Skip to content

Commit

Permalink
[ICD] Update and Document ICDManager interface (#33081)
Browse files Browse the repository at this point in the history 
* Update and Document  ICDManager interface

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky <[email protected]>

* Update ICDManagementCluster test to cover off the Check-In message randomness

* Address comments and update comments

* restyle

---------

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@mkardous-silabs @bzbarsky-apple
mkardous-silabs and bzbarsky-apple authored Apr 22, 2024
1 parent 63c4709 commit 2d97bb7
Show file tree
Hide file tree
Showing 10 changed files with 581 additions and 72 deletions.
3 changes: 2 additions & 1 deletion examples/lit-icd-app/nrfconnect/main/AppTask.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 1 addition & 3 deletions examples/platform/silabs/BaseApplication.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
23 changes: 15 additions & 8 deletions src/app/icd/server/ICDManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<OperatingModeEnum>(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.
Expand Down Expand Up @@ -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)
{
Expand All @@ -447,17 +450,22 @@ 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();
}

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 =
Expand Down Expand Up @@ -504,10 +512,6 @@ void ICDManager::OnIdleModeDone(System::Layer * aLayer, void * appState)
{
ICDManager * pICDManager = reinterpret_cast<ICDManager *>(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)
Expand All @@ -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))
Expand All @@ -560,7 +564,6 @@ void ICDManager::OnKeepActiveRequest(KeepActiveFlags request)
void ICDManager::OnActiveRequestWithdrawal(KeepActiveFlags request)
{
assertChipStackLockedByCurrentThread();

VerifyOrReturn(request < KeepActiveFlagsValues::kInvalidFlag);

if (request.Has(KeepActiveFlag::kExchangeContextOpen))
Expand Down Expand Up @@ -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;
Expand Down
163 changes: 126 additions & 37 deletions src/app/icd/server/ICDManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand All @@ -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,
};
Expand All @@ -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
Expand All @@ -111,36 +142,29 @@ 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);

/**
* @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
*
Expand All @@ -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);

/**
Expand All @@ -221,11 +306,15 @@ class ICDManager : public ICDListener, public TestEventTriggerHandler
OperationalState mOperationalState = OperationalState::ActiveMode;
bool mTransitionToIdleCalled = false;
ObjectPool<ObserverPointer, CHIP_CONFIG_ICD_OBSERVERS_POOL_SIZE> 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;
Expand Down
Loading

0 comments on commit 2d97bb7

Please sign in to comment.