Skip to content

Commit

Permalink
Fix CASE Churn caused by subscriptions on tv (#23616)
Browse files Browse the repository at this point in the history
* Fix CASE Churn caused by subscriptions on tv

* Address comments

* User feature flag for IM fix since it only applies when both server and controller are enabled

* Address feedback

* fix build
  • Loading branch information
chrisdecenzo authored and pull[bot] committed Aug 3, 2023
1 parent 6dc4356 commit 1249503
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 1 deletion.
3 changes: 3 additions & 0 deletions examples/tv-app/tv-common/include/CHIPProjectAppConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
// TVs need to be both commissioners and commissionees
#define CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE 1

// See issue 23625.
#define CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE 1

// TVs that are not commissionees,
// or that don't automatically enter commissioning mode should set this to 0
#define CHIP_DEVICE_CONFIG_ENABLE_PAIRING_AUTOSTART 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class MediaSubscriptionBase : public MediaBase
mTvEndpoint);

return cluster.template SubscribeAttribute<TypeInfo>(context, successFn, failureFn, minInterval, maxInterval,
onSubscriptionEstablished);
onSubscriptionEstablished, nullptr /* resubscribeCb */,
true /* fabricFiltered */, true /* keepPreviousSubscriptions */);
}
};
13 changes: 13 additions & 0 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon
VerifyOrDie(apExchangeContext != nullptr);

mExchangeCtx.Grab(apExchangeContext);
#if CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
// TODO: this should be replaced by a pointer to the InteractionModelEngine that created the ReadHandler
// once InteractionModelEngine is no longer a singleton (see issue 23625)
mExchangeMgr = apExchangeContext->GetExchangeMgr();
#endif // CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE

mInteractionType = aInteractionType;
mLastWrittenEventsBytes = 0;
Expand Down Expand Up @@ -185,7 +190,11 @@ CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aSt
{
VerifyOrReturnLogError(!mExchangeCtx, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
#if CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
auto exchange = mExchangeMgr->NewContext(mSessionHandle.Get().Value(), this);
#else // CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
auto exchange = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get().Value(), this);
#endif // CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
VerifyOrReturnLogError(exchange != nullptr, CHIP_ERROR_INCORRECT_STATE);
mExchangeCtx.Grab(exchange);
}
Expand All @@ -205,7 +214,11 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
{
VerifyOrReturnLogError(!mExchangeCtx, CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnLogError(mSessionHandle, CHIP_ERROR_INCORRECT_STATE);
#if CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
auto exchange = mExchangeMgr->NewContext(mSessionHandle.Get().Value(), this);
#else // CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
auto exchange = InteractionModelEngine::GetInstance()->GetExchangeManager()->NewContext(mSessionHandle.Get().Value(), this);
#endif // CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
VerifyOrReturnLogError(exchange != nullptr, CHIP_ERROR_INCORRECT_STATE);
mExchangeCtx.Grab(exchange);
}
Expand Down
5 changes: 5 additions & 0 deletions src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,11 @@ class ReadHandler : public Messaging::ExchangeDelegate
SessionHolder mSessionHandle;

Messaging::ExchangeHolder mExchangeCtx;
#if CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
// TODO: this should be replaced by a pointer to the InteractionModelEngine that created the ReadHandler
// once InteractionModelEngine is no longer a singleton (see issue 23625)
Messaging::ExchangeManager * mExchangeMgr = nullptr;
#endif // CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE

ObjectList<AttributePathParams> * mpAttributePathList = nullptr;
ObjectList<EventPathParams> * mpEventPathList = nullptr;
Expand Down
12 changes: 12 additions & 0 deletions src/include/platform/CHIPDeviceConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,18 @@
#define CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE 0
#endif

/**
* CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
*
* See issue 23625.
*
* Needs to be 1 when the following is 1:
* CHIP_DEVICE_CONFIG_ENABLE_BOTH_COMMISSIONER_AND_COMMISSIONEE
*/
#ifndef CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE
#define CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE 0
#endif

/**
* CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY_CLIENT
*
Expand Down

0 comments on commit 1249503

Please sign in to comment.