From 1249503adb05d777df7f47eba3863d1caea8c932 Mon Sep 17 00:00:00 2001 From: chrisdecenzo <61757564+chrisdecenzo@users.noreply.github.com> Date: Thu, 24 Nov 2022 06:24:57 -0800 Subject: [PATCH] Fix CASE Churn caused by subscriptions on tv (#23616) * 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 --- .../tv-app/tv-common/include/CHIPProjectAppConfig.h | 3 +++ .../include/MediaSubscriptionBase.h | 3 ++- src/app/ReadHandler.cpp | 13 +++++++++++++ src/app/ReadHandler.h | 5 +++++ src/include/platform/CHIPDeviceConfig.h | 12 ++++++++++++ 5 files changed, 35 insertions(+), 1 deletion(-) diff --git a/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h b/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h index c9ddf7451b869f..395be25eebba1a 100644 --- a/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h +++ b/examples/tv-app/tv-common/include/CHIPProjectAppConfig.h @@ -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 diff --git a/examples/tv-casting-app/tv-casting-common/include/MediaSubscriptionBase.h b/examples/tv-casting-app/tv-casting-common/include/MediaSubscriptionBase.h index e2a065ef13cdc4..73e4eb49c43921 100644 --- a/examples/tv-casting-app/tv-casting-common/include/MediaSubscriptionBase.h +++ b/examples/tv-casting-app/tv-casting-common/include/MediaSubscriptionBase.h @@ -39,6 +39,7 @@ class MediaSubscriptionBase : public MediaBase mTvEndpoint); return cluster.template SubscribeAttribute(context, successFn, failureFn, minInterval, maxInterval, - onSubscriptionEstablished); + onSubscriptionEstablished, nullptr /* resubscribeCb */, + true /* fabricFiltered */, true /* keepPreviousSubscriptions */); } }; diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 5b9a2921d1b12f..bc7cfacdd3a458 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -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; @@ -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); } @@ -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); } diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index cf0ee7ab50b706..c4f1472f617263 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -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 * mpAttributePathList = nullptr; ObjectList * mpEventPathList = nullptr; diff --git a/src/include/platform/CHIPDeviceConfig.h b/src/include/platform/CHIPDeviceConfig.h index cd9d78b48922ab..5dfc6fcf9ba258 100644 --- a/src/include/platform/CHIPDeviceConfig.h +++ b/src/include/platform/CHIPDeviceConfig.h @@ -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 *