From 4e7c8e46264b497787fcafdf3bacd65ef2056fa7 Mon Sep 17 00:00:00 2001 From: Yunhan Wang Date: Tue, 7 Jun 2022 01:08:24 -0700 Subject: [PATCH 1/3] Fix computation of min/max interval for subscriptions to be spec compliant --- .../MessageDef/SubscribeResponseMessage.cpp | 32 +------- src/app/MessageDef/SubscribeResponseMessage.h | 14 ---- src/app/ReadClient.cpp | 2 +- src/app/ReadHandler.cpp | 5 +- src/app/ReadHandler.h | 15 ++-- src/app/tests/TestMessageDef.cpp | 7 -- src/controller/tests/data_model/TestRead.cpp | 81 ++++++++++++++++++- 7 files changed, 92 insertions(+), 64 deletions(-) diff --git a/src/app/MessageDef/SubscribeResponseMessage.cpp b/src/app/MessageDef/SubscribeResponseMessage.cpp index f88ecd58a9c806..57cdf385b0ab06 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.cpp +++ b/src/app/MessageDef/SubscribeResponseMessage.cpp @@ -50,19 +50,6 @@ CHIP_ERROR SubscribeResponseMessage::Parser::CheckSchemaValidity() const ReturnErrorOnFailure(reader.Get(subscriptionId)); PRETTY_PRINT("\tSubscriptionId = 0x%" PRIx32 ",", subscriptionId); } -#endif // CHIP_DETAIL_LOGGING - break; - case to_underlying(Tag::kMinIntervalFloorSeconds): - VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kMinIntervalFloorSeconds))), - CHIP_ERROR_INVALID_TLV_TAG); - tagPresenceMask |= (1 << to_underlying(Tag::kMinIntervalFloorSeconds)); - VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE); -#if CHIP_DETAIL_LOGGING - { - uint16_t minIntervalFloorSeconds; - ReturnErrorOnFailure(reader.Get(minIntervalFloorSeconds)); - PRETTY_PRINT("\tMinIntervalFloorSeconds = 0x%x,", minIntervalFloorSeconds); - } #endif // CHIP_DETAIL_LOGGING break; case to_underlying(Tag::kMaxIntervalCeilingSeconds): @@ -91,8 +78,8 @@ CHIP_ERROR SubscribeResponseMessage::Parser::CheckSchemaValidity() const if (CHIP_END_OF_TLV == err) { - const uint16_t requiredFields = (1 << to_underlying(Tag::kSubscriptionId)) | - (1 << to_underlying(Tag::kMinIntervalFloorSeconds)) | (1 << to_underlying(Tag::kMaxIntervalCeilingSeconds)); + const uint16_t requiredFields = + (1 << to_underlying(Tag::kSubscriptionId)) | (1 << to_underlying(Tag::kMaxIntervalCeilingSeconds)); err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_SUBSCRIBE_RESPONSE_MESSAGE; } @@ -106,11 +93,6 @@ CHIP_ERROR SubscribeResponseMessage::Parser::GetSubscriptionId(SubscriptionId * return GetUnsignedInteger(to_underlying(Tag::kSubscriptionId), apSubscribeId); } -CHIP_ERROR SubscribeResponseMessage::Parser::GetMinIntervalFloorSeconds(uint16_t * const apMinIntervalFloorSeconds) const -{ - return GetUnsignedInteger(to_underlying(Tag::kMinIntervalFloorSeconds), apMinIntervalFloorSeconds); -} - CHIP_ERROR SubscribeResponseMessage::Parser::GetMaxIntervalCeilingSeconds(uint16_t * const apMaxIntervalCeilingSeconds) const { return GetUnsignedInteger(to_underlying(Tag::kMaxIntervalCeilingSeconds), apMaxIntervalCeilingSeconds); @@ -125,16 +107,6 @@ SubscribeResponseMessage::Builder & SubscribeResponseMessage::Builder::Subscript return *this; } -SubscribeResponseMessage::Builder & -SubscribeResponseMessage::Builder::MinIntervalFloorSeconds(const uint16_t aMinIntervalFloorSeconds) -{ - if (mError == CHIP_NO_ERROR) - { - mError = mpWriter->Put(TLV::ContextTag(to_underlying(Tag::kMinIntervalFloorSeconds)), aMinIntervalFloorSeconds); - } - return *this; -} - SubscribeResponseMessage::Builder & SubscribeResponseMessage::Builder::MaxIntervalCeilingSeconds(const uint16_t aMaxIntervalCeilingSeconds) { diff --git a/src/app/MessageDef/SubscribeResponseMessage.h b/src/app/MessageDef/SubscribeResponseMessage.h index 7c4b3a0e5dd5b1..b712e6bf25081c 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.h +++ b/src/app/MessageDef/SubscribeResponseMessage.h @@ -32,7 +32,6 @@ namespace SubscribeResponseMessage { enum class Tag : uint8_t { kSubscriptionId = 0, - kMinIntervalFloorSeconds = 1, kMaxIntervalCeilingSeconds = 2, }; @@ -62,14 +61,6 @@ class Parser : public MessageParser */ CHIP_ERROR GetSubscriptionId(SubscriptionId * const apSubscriptionId) const; - /** - * @brief Get Final MinIntervalFloorSeconds. Next() must be called before accessing them. - * - * @return #CHIP_NO_ERROR on success - * #CHIP_END_OF_TLV if there is no such element - */ - CHIP_ERROR GetMinIntervalFloorSeconds(uint16_t * const apMinIntervalFloorSeconds) const; - /** * @brief Get Final MaxIntervalCeilingSeconds. Next() must be called before accessing them. * @@ -87,11 +78,6 @@ class Builder : public MessageBuilder */ SubscribeResponseMessage::Builder & SubscriptionId(const chip::SubscriptionId SubscriptionId); - /** - * @brief Final Min Interval for the subscription back to the clients. - */ - SubscribeResponseMessage::Builder & MinIntervalFloorSeconds(const uint16_t aMinIntervalFloorSeconds); - /** * @brief Final Max Interval for the subscription back to the clients. */ diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index a3475134076486..b3e11cf1588783 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -819,7 +819,6 @@ CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aP SubscriptionId subscriptionId = 0; ReturnErrorOnFailure(subscribeResponse.GetSubscriptionId(&subscriptionId)); VerifyOrReturnError(IsMatchingClient(subscriptionId), CHIP_ERROR_INVALID_ARGUMENT); - ReturnErrorOnFailure(subscribeResponse.GetMinIntervalFloorSeconds(&mMinIntervalFloorSeconds)); ReturnErrorOnFailure(subscribeResponse.GetMaxIntervalCeilingSeconds(&mMaxIntervalCeilingSeconds)); ChipLogProgress(DataManagement, @@ -864,6 +863,7 @@ CHIP_ERROR ReadClient::SendSubscribeRequest(const ReadPrepareParams & aReadPrepa { VerifyOrReturnError(aReadPrepareParams.mMinIntervalFloorSeconds <= aReadPrepareParams.mMaxIntervalCeilingSeconds, CHIP_ERROR_INVALID_ARGUMENT); + mMinIntervalFloorSeconds = aReadPrepareParams.mMinIntervalFloorSeconds; return SendSubscribeRequestImpl(aReadPrepareParams); } diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 47877edff568c5..aff3f18563d94e 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -676,10 +676,7 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() SubscribeResponseMessage::Builder response; ReturnErrorOnFailure(response.Init(&writer)); - response.SubscriptionId(mSubscriptionId) - .MinIntervalFloorSeconds(mMinIntervalFloorSeconds) - .MaxIntervalCeilingSeconds(mMaxIntervalCeilingSeconds) - .EndOfSubscribeResponseMessage(); + response.SubscriptionId(mSubscriptionId).MaxIntervalCeilingSeconds(mMaxIntervalCeilingSeconds).EndOfSubscribeResponseMessage(); ReturnErrorOnFailure(response.GetError()); ReturnErrorOnFailure(writer.Finalize(&packet)); diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index f339aba1c3b552..30f16635cd24bf 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -45,6 +45,8 @@ #include #include +constexpr uint16_t kSubscriptionMaxIntervalPublisherLimit = 3600; // 3600 seconds + namespace chip { namespace app { @@ -172,15 +174,18 @@ class ReadHandler : public Messaging::ExchangeDelegate /* * Set the reporting intervals for the subscription. This SHALL only be called - * from the OnSubscriptionRequested callback above. + * from the OnSubscriptionRequested callback above. The restriction is as below + * MinIntervalFloor ≤ MaxInterval ≤ MAX(SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT, MaxIntervalCeiling) + * Where SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT is set to 60m in the spec. */ - CHIP_ERROR SetReportingIntervals(uint16_t aMinInterval, uint16_t aMaxInterval) + CHIP_ERROR SetReportingIntervals(uint16_t aMaxInterval) { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(aMinInterval <= aMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); - - mMinIntervalFloorSeconds = aMinInterval; mMaxIntervalCeilingSeconds = aMaxInterval; + VerifyOrReturnError(mMinIntervalFloorSeconds <= mMaxIntervalCeilingSeconds, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(mMaxIntervalCeilingSeconds <= + std::max(kSubscriptionMaxIntervalPublisherLimit, mMaxIntervalCeilingSeconds), + CHIP_ERROR_INVALID_ARGUMENT); return CHIP_NO_ERROR; } diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 4235e5356b4ae0..f709f7d5a16e38 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -1330,9 +1330,6 @@ void BuildSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVWriter & subscribeResponseBuilder.SubscriptionId(1); NL_TEST_ASSERT(apSuite, subscribeResponseBuilder.GetError() == CHIP_NO_ERROR); - subscribeResponseBuilder.MinIntervalFloorSeconds(1); - NL_TEST_ASSERT(apSuite, subscribeResponseBuilder.GetError() == CHIP_NO_ERROR); - subscribeResponseBuilder.MaxIntervalCeilingSeconds(2); NL_TEST_ASSERT(apSuite, subscribeResponseBuilder.GetError() == CHIP_NO_ERROR); @@ -1346,7 +1343,6 @@ void ParseSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & SubscribeResponseMessage::Parser subscribeResponseParser; chip::SubscriptionId subscriptionId = 0; - uint16_t minIntervalFloorSeconds = 0; uint16_t maxIntervalCeilingSeconds = 0; err = subscribeResponseParser.Init(aReader); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); @@ -1357,9 +1353,6 @@ void ParseSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & err = subscribeResponseParser.GetSubscriptionId(&subscriptionId); NL_TEST_ASSERT(apSuite, subscriptionId == 1 && err == CHIP_NO_ERROR); - err = subscribeResponseParser.GetMinIntervalFloorSeconds(&minIntervalFloorSeconds); - NL_TEST_ASSERT(apSuite, minIntervalFloorSeconds == 1 && err == CHIP_NO_ERROR); - err = subscribeResponseParser.GetMaxIntervalCeilingSeconds(&maxIntervalCeilingSeconds); NL_TEST_ASSERT(apSuite, maxIntervalCeilingSeconds == 2 && err == CHIP_NO_ERROR); diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 6ce3d52353f266..3032d321d0faf8 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -219,6 +219,7 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback static void TestReadHandler_TwoSubscribesMultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_MultipleSubscriptionsWithDataVersionFilter(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_SubscriptionAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionLargeAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext); static void TestReadHandlerResourceExhaustion_MultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadSubscribeAttributeResponseWithCache(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite * apSuite, void * apContext); @@ -228,16 +229,17 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback static void TestReadHandler_TwoParallelReadsSecondTooManyPaths(nlTestSuite * apSuite, void * apContext); private: - static constexpr uint16_t kTestMinInterval = 33; static constexpr uint16_t kTestMaxInterval = 66; + static uint16_t mMaxInterval; + CHIP_ERROR OnSubscriptionRequested(app::ReadHandler & aReadHandler, Transport::SecureSession & aSecureSession) { VerifyOrReturnError(!mEmitSubscriptionError, CHIP_ERROR_INVALID_ARGUMENT); if (mAlterSubscriptionIntervals) { - ReturnErrorOnFailure(aReadHandler.SetReportingIntervals(kTestMinInterval, kTestMaxInterval)); + ReturnErrorOnFailure(aReadHandler.SetReportingIntervals(mMaxInterval)); } return CHIP_NO_ERROR; } @@ -259,6 +261,8 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback bool mAlterSubscriptionIntervals = false; }; +uint16_t TestReadInteraction::mMaxInterval = 66; + TestReadInteraction gTestReadInteraction; class MockInteractionModelApp : public chip::app::ClusterStateCache::Callback @@ -1644,7 +1648,7 @@ void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals( NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(apSuite, minInterval == kTestMinInterval); + NL_TEST_ASSERT(apSuite, minInterval == 0); NL_TEST_ASSERT(apSuite, maxInterval == kTestMaxInterval); numSubscriptionEstablishedCalls++; @@ -1687,6 +1691,76 @@ void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals( gTestReadInteraction.mAlterSubscriptionIntervals = false; } +void TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + uint16_t largeMaxInterval = 6000; + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite, + largeMaxInterval](const app::ReadClient & readClient) { + uint16_t minInterval = 0, maxInterval = 0; + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(apSuite, minInterval == 0); + NL_TEST_ASSERT(apSuite, maxInterval == largeMaxInterval); + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = true; + gTestReadInteraction.mMaxInterval = largeMaxInterval; + + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 10, + onSubscriptionEstablishedCb, false, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls == 1); + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + void TestReadInteraction::TestReadHandler_MultipleReads(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); @@ -2617,6 +2691,7 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadAttributeTimeout", TestReadInteraction::TestReadAttributeTimeout), NL_TEST_DEF("TestSubscribeAttributeTimeout", TestReadInteraction::TestSubscribeAttributeTimeout), NL_TEST_DEF("TestReadHandler_SubscriptionAlteredReportingIntervals", TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals), + NL_TEST_DEF("TestReadHandler_SubscriptionLargeAlteredReportingIntervals", TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingIntervals), NL_TEST_DEF("TestReadSubscribeAttributeResponseWithCache", TestReadInteraction::TestReadSubscribeAttributeResponseWithCache), NL_TEST_DEF("TestReadHandler_KillOverQuotaSubscriptions", TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions), NL_TEST_DEF("TestReadHandler_KillOldestSubscriptions", TestReadInteraction::TestReadHandler_KillOldestSubscriptions), From 8a9cf8333a72df92701f259109851b0906759fe5 Mon Sep 17 00:00:00 2001 From: Yunhan Wang Date: Tue, 7 Jun 2022 10:33:44 -0700 Subject: [PATCH 2/3] Fix logic in SetReportingIntervals --- .../MessageDef/SubscribeResponseMessage.cpp | 22 +++++++++---------- src/app/MessageDef/SubscribeResponseMessage.h | 6 ++--- src/app/ReadClient.cpp | 8 +++---- src/app/ReadClient.h | 4 ++-- src/app/ReadHandler.cpp | 14 ++++++------ src/app/ReadHandler.h | 13 ++++++----- src/app/tests/TestMessageDef.cpp | 20 ++++++++--------- 7 files changed, 44 insertions(+), 43 deletions(-) diff --git a/src/app/MessageDef/SubscribeResponseMessage.cpp b/src/app/MessageDef/SubscribeResponseMessage.cpp index 57cdf385b0ab06..309ebfe5f6f3d7 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.cpp +++ b/src/app/MessageDef/SubscribeResponseMessage.cpp @@ -52,16 +52,16 @@ CHIP_ERROR SubscribeResponseMessage::Parser::CheckSchemaValidity() const } #endif // CHIP_DETAIL_LOGGING break; - case to_underlying(Tag::kMaxIntervalCeilingSeconds): - VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kMaxIntervalCeilingSeconds))), + case to_underlying(Tag::kMaxInterval): + VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kMaxInterval))), CHIP_ERROR_INVALID_TLV_TAG); - tagPresenceMask |= (1 << to_underlying(Tag::kMaxIntervalCeilingSeconds)); + tagPresenceMask |= (1 << to_underlying(Tag::kMaxInterval)); VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE); #if CHIP_DETAIL_LOGGING { - uint16_t maxIntervalCeilingSeconds; - ReturnErrorOnFailure(reader.Get(maxIntervalCeilingSeconds)); - PRETTY_PRINT("\tMaxIntervalCeilingSeconds = 0x%x,", maxIntervalCeilingSeconds); + uint16_t maxInterval; + ReturnErrorOnFailure(reader.Get(maxInterval)); + PRETTY_PRINT("\tMaxInterval = 0x%x,", maxInterval); } #endif // CHIP_DETAIL_LOGGING break; @@ -79,7 +79,7 @@ CHIP_ERROR SubscribeResponseMessage::Parser::CheckSchemaValidity() const if (CHIP_END_OF_TLV == err) { const uint16_t requiredFields = - (1 << to_underlying(Tag::kSubscriptionId)) | (1 << to_underlying(Tag::kMaxIntervalCeilingSeconds)); + (1 << to_underlying(Tag::kSubscriptionId)) | (1 << to_underlying(Tag::kMaxInterval)); err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_SUBSCRIBE_RESPONSE_MESSAGE; } @@ -93,9 +93,9 @@ CHIP_ERROR SubscribeResponseMessage::Parser::GetSubscriptionId(SubscriptionId * return GetUnsignedInteger(to_underlying(Tag::kSubscriptionId), apSubscribeId); } -CHIP_ERROR SubscribeResponseMessage::Parser::GetMaxIntervalCeilingSeconds(uint16_t * const apMaxIntervalCeilingSeconds) const +CHIP_ERROR SubscribeResponseMessage::Parser::GetMaxInterval(uint16_t * const apMaxInterval) const { - return GetUnsignedInteger(to_underlying(Tag::kMaxIntervalCeilingSeconds), apMaxIntervalCeilingSeconds); + return GetUnsignedInteger(to_underlying(Tag::kMaxInterval), apMaxInterval); } SubscribeResponseMessage::Builder & SubscribeResponseMessage::Builder::SubscriptionId(const chip::SubscriptionId aSubscribeId) @@ -108,11 +108,11 @@ SubscribeResponseMessage::Builder & SubscribeResponseMessage::Builder::Subscript } SubscribeResponseMessage::Builder & -SubscribeResponseMessage::Builder::MaxIntervalCeilingSeconds(const uint16_t aMaxIntervalCeilingSeconds) +SubscribeResponseMessage::Builder::MaxInterval(const uint16_t aMaxInterval) { if (mError == CHIP_NO_ERROR) { - mError = mpWriter->Put(TLV::ContextTag(to_underlying(Tag::kMaxIntervalCeilingSeconds)), aMaxIntervalCeilingSeconds); + mError = mpWriter->Put(TLV::ContextTag(to_underlying(Tag::kMaxInterval)), aMaxInterval); } return *this; } diff --git a/src/app/MessageDef/SubscribeResponseMessage.h b/src/app/MessageDef/SubscribeResponseMessage.h index b712e6bf25081c..c51160b5ea628c 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.h +++ b/src/app/MessageDef/SubscribeResponseMessage.h @@ -32,7 +32,7 @@ namespace SubscribeResponseMessage { enum class Tag : uint8_t { kSubscriptionId = 0, - kMaxIntervalCeilingSeconds = 2, + kMaxInterval = 2, }; class Parser : public MessageParser @@ -67,7 +67,7 @@ class Parser : public MessageParser * @return #CHIP_NO_ERROR on success * #CHIP_END_OF_TLV if there is no such element */ - CHIP_ERROR GetMaxIntervalCeilingSeconds(uint16_t * const apMaxIntervalCeilingSeconds) const; + CHIP_ERROR GetMaxInterval(uint16_t * const apMaxInterval) const; }; class Builder : public MessageBuilder @@ -81,7 +81,7 @@ class Builder : public MessageBuilder /** * @brief Final Max Interval for the subscription back to the clients. */ - SubscribeResponseMessage::Builder & MaxIntervalCeilingSeconds(const uint16_t aMaxIntervalCeilingSeconds); + SubscribeResponseMessage::Builder & MaxInterval(const uint16_t aMaxInterval); /** * @brief Mark the end of this SubscribeResponseMessage diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index b3e11cf1588783..96aaf01f921e95 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -95,7 +95,7 @@ void ReadClient::ClearActiveSubscriptionState() mIsPrimingReports = true; mPendingMoreChunks = false; mMinIntervalFloorSeconds = 0; - mMaxIntervalCeilingSeconds = 0; + mMaxInterval = 0; mSubscriptionId = 0; MoveToState(ClientState::Idle); } @@ -755,7 +755,7 @@ CHIP_ERROR ReadClient::RefreshLivenessCheckTimer() VerifyOrReturnError(mpExchangeCtx->HasSessionHandle(), err = CHIP_ERROR_INCORRECT_STATE); System::Clock::Timeout timeout = - System::Clock::Seconds16(mMaxIntervalCeilingSeconds) + mpExchangeCtx->GetSessionHandle()->GetAckTimeout(); + System::Clock::Seconds16(mMaxInterval) + mpExchangeCtx->GetSessionHandle()->GetAckTimeout(); // EFR32/MBED/INFINION/K32W's chrono count return long unsinged, but other platform returns unsigned ChipLogProgress(DataManagement, "Refresh LivenessCheckTime for %lu milliseconds with SubscriptionId = 0x%08" PRIx32 @@ -819,12 +819,12 @@ CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aP SubscriptionId subscriptionId = 0; ReturnErrorOnFailure(subscribeResponse.GetSubscriptionId(&subscriptionId)); VerifyOrReturnError(IsMatchingClient(subscriptionId), CHIP_ERROR_INVALID_ARGUMENT); - ReturnErrorOnFailure(subscribeResponse.GetMaxIntervalCeilingSeconds(&mMaxIntervalCeilingSeconds)); + ReturnErrorOnFailure(subscribeResponse.GetMaxInterval(&mMaxInterval)); ChipLogProgress(DataManagement, "Subscription established with SubscriptionID = 0x%08" PRIx32 " MinInterval = %u" "s MaxInterval = %us Peer = %02x:" ChipLogFormatX64, - mSubscriptionId, mMinIntervalFloorSeconds, mMaxIntervalCeilingSeconds, mFabricIndex, + mSubscriptionId, mMinIntervalFloorSeconds, mMaxInterval, mFabricIndex, ChipLogValueX64(mPeerNodeId)); ReturnErrorOnFailure(subscribeResponse.ExitContainer()); diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 1d006b5c6ed95d..49600731b2cd00 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -290,7 +290,7 @@ class ReadClient : public Messaging::ExchangeDelegate VerifyOrReturnError(IsSubscriptionActive(), CHIP_ERROR_INCORRECT_STATE); aMinIntervalFloorSeconds = mMinIntervalFloorSeconds; - aMaxIntervalCeilingSeconds = mMaxIntervalCeilingSeconds; + aMaxIntervalCeilingSeconds = mMaxInterval; return CHIP_NO_ERROR; } @@ -418,7 +418,7 @@ class ReadClient : public Messaging::ExchangeDelegate bool mIsPrimingReports = true; bool mPendingMoreChunks = false; uint16_t mMinIntervalFloorSeconds = 0; - uint16_t mMaxIntervalCeilingSeconds = 0; + uint16_t mMaxInterval = 0; SubscriptionId mSubscriptionId = 0; NodeId mPeerNodeId = kUndefinedNodeId; FabricIndex mFabricIndex = kUndefinedFabricIndex; diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index aff3f18563d94e..2459e4e12f7008 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -676,7 +676,7 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() SubscribeResponseMessage::Builder response; ReturnErrorOnFailure(response.Init(&writer)); - response.SubscriptionId(mSubscriptionId).MaxIntervalCeilingSeconds(mMaxIntervalCeilingSeconds).EndOfSubscribeResponseMessage(); + response.SubscriptionId(mSubscriptionId).MaxInterval(mMaxInterval).EndOfSubscribeResponseMessage(); ReturnErrorOnFailure(response.GetError()); ReturnErrorOnFailure(writer.Finalize(&packet)); @@ -744,8 +744,8 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP ReturnErrorOnFailure(err); ReturnErrorOnFailure(subscribeRequestParser.GetMinIntervalFloorSeconds(&mMinIntervalFloorSeconds)); - ReturnErrorOnFailure(subscribeRequestParser.GetMaxIntervalCeilingSeconds(&mMaxIntervalCeilingSeconds)); - VerifyOrReturnError(mMinIntervalFloorSeconds <= mMaxIntervalCeilingSeconds, CHIP_ERROR_INVALID_ARGUMENT); + ReturnErrorOnFailure(subscribeRequestParser.GetMaxIntervalCeilingSeconds(&mMaxInterval)); + VerifyOrReturnError(mMinIntervalFloorSeconds <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); // // Notify the application (if requested) of the impending subscription and check whether we should still proceed to set it up. @@ -761,7 +761,7 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP } ChipLogProgress(DataManagement, "Final negotiated min/max parameters: Min = %ds, Max = %ds", mMinIntervalFloorSeconds, - mMaxIntervalCeilingSeconds); + mMaxInterval); bool isFabricFiltered; ReturnErrorOnFailure(subscribeRequestParser.GetIsFabricFiltered(&isFabricFiltered)); @@ -786,7 +786,7 @@ void ReadHandler::OnUnblockHoldReportCallback(System::Layer * apSystemLayer, voi InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( - System::Clock::Seconds16(readHandler->mMaxIntervalCeilingSeconds - readHandler->mMinIntervalFloorSeconds), + System::Clock::Seconds16(readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds), OnRefreshSubscribeTimerSyncCallback, readHandler); } @@ -796,13 +796,13 @@ void ReadHandler::OnRefreshSubscribeTimerSyncCallback(System::Layer * apSystemLa ReadHandler * readHandler = static_cast(apAppState); readHandler->mFlags.Set(ReadHandlerFlags::HoldSync, false); ChipLogDetail(DataManagement, "Refresh subscribe timer sync after %d seconds", - readHandler->mMaxIntervalCeilingSeconds - readHandler->mMinIntervalFloorSeconds); + readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds); InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } CHIP_ERROR ReadHandler::RefreshSubscribeSyncTimer() { - ChipLogDetail(DataManagement, "Refresh Subscribe Sync Timer with max %d seconds", mMaxIntervalCeilingSeconds); + ChipLogDetail(DataManagement, "Refresh Subscribe Sync Timer with max %d seconds", mMaxInterval); InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( OnUnblockHoldReportCallback, this); InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 30f16635cd24bf..3613a85c1e4d10 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -45,6 +45,7 @@ #include #include +//https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/61a9d19e6af12fdfb0872bcff26d19de6c680a1a/src/Ch02_Architecture.adoc#1122-subscribe-interaction-limits constexpr uint16_t kSubscriptionMaxIntervalPublisherLimit = 3600; // 3600 seconds namespace chip { @@ -169,7 +170,7 @@ class ReadHandler : public Messaging::ExchangeDelegate void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { aMinInterval = mMinIntervalFloorSeconds; - aMaxInterval = mMaxIntervalCeilingSeconds; + aMaxInterval = mMaxInterval; } /* @@ -181,11 +182,11 @@ class ReadHandler : public Messaging::ExchangeDelegate CHIP_ERROR SetReportingIntervals(uint16_t aMaxInterval) { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); - mMaxIntervalCeilingSeconds = aMaxInterval; - VerifyOrReturnError(mMinIntervalFloorSeconds <= mMaxIntervalCeilingSeconds, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(mMaxIntervalCeilingSeconds <= - std::max(kSubscriptionMaxIntervalPublisherLimit, mMaxIntervalCeilingSeconds), + VerifyOrReturnError(mMinIntervalFloorSeconds <= aMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(aMaxInterval <= + std::max(kSubscriptionMaxIntervalPublisherLimit, mMaxInterval), CHIP_ERROR_INVALID_ARGUMENT); + mMaxInterval = aMaxInterval; return CHIP_NO_ERROR; } @@ -435,7 +436,7 @@ class ReadHandler : public Messaging::ExchangeDelegate SubscriptionId mSubscriptionId = 0; uint16_t mMinIntervalFloorSeconds = 0; - uint16_t mMaxIntervalCeilingSeconds = 0; + uint16_t mMaxInterval = 0; EventNumber mEventMin = 0; diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index f709f7d5a16e38..524b632d83452a 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -1282,8 +1282,8 @@ void ParseSubscribeRequestMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & DataVersionFilterIBs::Parser dataVersionFilterIBsParser; EventPathIBs::Parser eventPathListParser; EventFilterIBs::Parser eventFiltersParser; - uint16_t MinIntervalFloorSeconds = 0; - uint16_t MaxIntervalCeilingSeconds = 0; + uint16_t minIntervalFloorSeconds = 0; + uint16_t maxIntervalCeilingSeconds = 0; bool keepExistingSubscription = false; bool isFabricFiltered = false; @@ -1305,11 +1305,11 @@ void ParseSubscribeRequestMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & err = subscribeRequestParser.GetEventFilters(&eventFiltersParser); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - err = subscribeRequestParser.GetMinIntervalFloorSeconds(&MinIntervalFloorSeconds); - NL_TEST_ASSERT(apSuite, MinIntervalFloorSeconds == 2 && err == CHIP_NO_ERROR); + err = subscribeRequestParser.GetMinIntervalFloorSeconds(&minIntervalFloorSeconds); + NL_TEST_ASSERT(apSuite, minIntervalFloorSeconds == 2 && err == CHIP_NO_ERROR); - err = subscribeRequestParser.GetMaxIntervalCeilingSeconds(&MaxIntervalCeilingSeconds); - NL_TEST_ASSERT(apSuite, MaxIntervalCeilingSeconds == 3 && err == CHIP_NO_ERROR); + err = subscribeRequestParser.GetMaxIntervalCeilingSeconds(&maxIntervalCeilingSeconds); + NL_TEST_ASSERT(apSuite, maxIntervalCeilingSeconds == 3 && err == CHIP_NO_ERROR); err = subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscription); NL_TEST_ASSERT(apSuite, keepExistingSubscription && err == CHIP_NO_ERROR); @@ -1330,7 +1330,7 @@ void BuildSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVWriter & subscribeResponseBuilder.SubscriptionId(1); NL_TEST_ASSERT(apSuite, subscribeResponseBuilder.GetError() == CHIP_NO_ERROR); - subscribeResponseBuilder.MaxIntervalCeilingSeconds(2); + subscribeResponseBuilder.MaxInterval(2); NL_TEST_ASSERT(apSuite, subscribeResponseBuilder.GetError() == CHIP_NO_ERROR); subscribeResponseBuilder.EndOfSubscribeResponseMessage(); @@ -1343,7 +1343,7 @@ void ParseSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & SubscribeResponseMessage::Parser subscribeResponseParser; chip::SubscriptionId subscriptionId = 0; - uint16_t maxIntervalCeilingSeconds = 0; + uint16_t maxInterval = 0; err = subscribeResponseParser.Init(aReader); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); #if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK @@ -1353,8 +1353,8 @@ void ParseSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & err = subscribeResponseParser.GetSubscriptionId(&subscriptionId); NL_TEST_ASSERT(apSuite, subscriptionId == 1 && err == CHIP_NO_ERROR); - err = subscribeResponseParser.GetMaxIntervalCeilingSeconds(&maxIntervalCeilingSeconds); - NL_TEST_ASSERT(apSuite, maxIntervalCeilingSeconds == 2 && err == CHIP_NO_ERROR); + err = subscribeResponseParser.GetMaxInterval(&maxInterval); + NL_TEST_ASSERT(apSuite, maxInterval == 2 && err == CHIP_NO_ERROR); NL_TEST_ASSERT(apSuite, subscribeResponseParser.ExitContainer() == CHIP_NO_ERROR); } From 5ecf97a488ecc5147c40d3c29bca32602727c0ec Mon Sep 17 00:00:00 2001 From: Yunhan Wang Date: Fri, 10 Jun 2022 15:07:27 -0700 Subject: [PATCH 3/3] address comments --- .../MessageDef/SubscribeResponseMessage.cpp | 11 +- src/app/MessageDef/SubscribeResponseMessage.h | 4 +- src/app/ReadClient.cpp | 18 +- src/app/ReadClient.h | 20 +- src/app/ReadHandler.h | 11 +- src/app/tests/TestMessageDef.cpp | 2 +- src/controller/tests/data_model/TestRead.cpp | 545 +++++++++++++++++- 7 files changed, 557 insertions(+), 54 deletions(-) diff --git a/src/app/MessageDef/SubscribeResponseMessage.cpp b/src/app/MessageDef/SubscribeResponseMessage.cpp index 309ebfe5f6f3d7..97e7100c4e5670 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.cpp +++ b/src/app/MessageDef/SubscribeResponseMessage.cpp @@ -53,8 +53,7 @@ CHIP_ERROR SubscribeResponseMessage::Parser::CheckSchemaValidity() const #endif // CHIP_DETAIL_LOGGING break; case to_underlying(Tag::kMaxInterval): - VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kMaxInterval))), - CHIP_ERROR_INVALID_TLV_TAG); + VerifyOrReturnError(!(tagPresenceMask & (1 << to_underlying(Tag::kMaxInterval))), CHIP_ERROR_INVALID_TLV_TAG); tagPresenceMask |= (1 << to_underlying(Tag::kMaxInterval)); VerifyOrReturnError(TLV::kTLVType_UnsignedInteger == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE); #if CHIP_DETAIL_LOGGING @@ -78,9 +77,8 @@ CHIP_ERROR SubscribeResponseMessage::Parser::CheckSchemaValidity() const if (CHIP_END_OF_TLV == err) { - const uint16_t requiredFields = - (1 << to_underlying(Tag::kSubscriptionId)) | (1 << to_underlying(Tag::kMaxInterval)); - err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR + const uint16_t requiredFields = (1 << to_underlying(Tag::kSubscriptionId)) | (1 << to_underlying(Tag::kMaxInterval)); + err = (tagPresenceMask & requiredFields) == requiredFields ? CHIP_NO_ERROR : CHIP_ERROR_IM_MALFORMED_SUBSCRIBE_RESPONSE_MESSAGE; } ReturnErrorOnFailure(err); @@ -107,8 +105,7 @@ SubscribeResponseMessage::Builder & SubscribeResponseMessage::Builder::Subscript return *this; } -SubscribeResponseMessage::Builder & -SubscribeResponseMessage::Builder::MaxInterval(const uint16_t aMaxInterval) +SubscribeResponseMessage::Builder & SubscribeResponseMessage::Builder::MaxInterval(const uint16_t aMaxInterval) { if (mError == CHIP_NO_ERROR) { diff --git a/src/app/MessageDef/SubscribeResponseMessage.h b/src/app/MessageDef/SubscribeResponseMessage.h index c51160b5ea628c..19a44e6c0f2f74 100644 --- a/src/app/MessageDef/SubscribeResponseMessage.h +++ b/src/app/MessageDef/SubscribeResponseMessage.h @@ -31,8 +31,8 @@ namespace app { namespace SubscribeResponseMessage { enum class Tag : uint8_t { - kSubscriptionId = 0, - kMaxInterval = 2, + kSubscriptionId = 0, + kMaxInterval = 2, }; class Parser : public MessageParser diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 96aaf01f921e95..01db106995905b 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -91,12 +91,12 @@ ReadClient::ReadClient(InteractionModelEngine * apImEngine, Messaging::ExchangeM void ReadClient::ClearActiveSubscriptionState() { - mIsInitialReport = true; - mIsPrimingReports = true; - mPendingMoreChunks = false; - mMinIntervalFloorSeconds = 0; - mMaxInterval = 0; - mSubscriptionId = 0; + mIsInitialReport = true; + mIsPrimingReports = true; + mPendingMoreChunks = false; + mMinIntervalFloorSeconds = 0; + mMaxInterval = 0; + mSubscriptionId = 0; MoveToState(ClientState::Idle); } @@ -754,8 +754,7 @@ CHIP_ERROR ReadClient::RefreshLivenessCheckTimer() VerifyOrReturnError(mpExchangeCtx != nullptr, err = CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mpExchangeCtx->HasSessionHandle(), err = CHIP_ERROR_INCORRECT_STATE); - System::Clock::Timeout timeout = - System::Clock::Seconds16(mMaxInterval) + mpExchangeCtx->GetSessionHandle()->GetAckTimeout(); + System::Clock::Timeout timeout = System::Clock::Seconds16(mMaxInterval) + mpExchangeCtx->GetSessionHandle()->GetAckTimeout(); // EFR32/MBED/INFINION/K32W's chrono count return long unsinged, but other platform returns unsigned ChipLogProgress(DataManagement, "Refresh LivenessCheckTime for %lu milliseconds with SubscriptionId = 0x%08" PRIx32 @@ -824,8 +823,7 @@ CHIP_ERROR ReadClient::ProcessSubscribeResponse(System::PacketBufferHandle && aP ChipLogProgress(DataManagement, "Subscription established with SubscriptionID = 0x%08" PRIx32 " MinInterval = %u" "s MaxInterval = %us Peer = %02x:" ChipLogFormatX64, - mSubscriptionId, mMinIntervalFloorSeconds, mMaxInterval, mFabricIndex, - ChipLogValueX64(mPeerNodeId)); + mSubscriptionId, mMinIntervalFloorSeconds, mMaxInterval, mFabricIndex, ChipLogValueX64(mPeerNodeId)); ReturnErrorOnFailure(subscribeResponse.ExitContainer()); diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 49600731b2cd00..e79bab6e867dd0 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -413,16 +413,16 @@ class ReadClient : public Messaging::ExchangeDelegate Messaging::ExchangeManager * mpExchangeMgr = nullptr; Messaging::ExchangeContext * mpExchangeCtx = nullptr; Callback & mpCallback; - ClientState mState = ClientState::Idle; - bool mIsInitialReport = true; - bool mIsPrimingReports = true; - bool mPendingMoreChunks = false; - uint16_t mMinIntervalFloorSeconds = 0; - uint16_t mMaxInterval = 0; - SubscriptionId mSubscriptionId = 0; - NodeId mPeerNodeId = kUndefinedNodeId; - FabricIndex mFabricIndex = kUndefinedFabricIndex; - InteractionType mInteractionType = InteractionType::Read; + ClientState mState = ClientState::Idle; + bool mIsInitialReport = true; + bool mIsPrimingReports = true; + bool mPendingMoreChunks = false; + uint16_t mMinIntervalFloorSeconds = 0; + uint16_t mMaxInterval = 0; + SubscriptionId mSubscriptionId = 0; + NodeId mPeerNodeId = kUndefinedNodeId; + FabricIndex mFabricIndex = kUndefinedFabricIndex; + InteractionType mInteractionType = InteractionType::Read; Timestamp mEventTimestamp; bool mSawAttributeReportsInCurrentReport = false; diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 3613a85c1e4d10..eab32872245732 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -45,7 +45,7 @@ #include #include -//https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/61a9d19e6af12fdfb0872bcff26d19de6c680a1a/src/Ch02_Architecture.adoc#1122-subscribe-interaction-limits +// https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/61a9d19e6af12fdfb0872bcff26d19de6c680a1a/src/Ch02_Architecture.adoc#1122-subscribe-interaction-limits constexpr uint16_t kSubscriptionMaxIntervalPublisherLimit = 3600; // 3600 seconds namespace chip { @@ -183,8 +183,7 @@ class ReadHandler : public Messaging::ExchangeDelegate { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mMinIntervalFloorSeconds <= aMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); - VerifyOrReturnError(aMaxInterval <= - std::max(kSubscriptionMaxIntervalPublisherLimit, mMaxInterval), + VerifyOrReturnError(aMaxInterval <= std::max(kSubscriptionMaxIntervalPublisherLimit, mMaxInterval), CHIP_ERROR_INVALID_ARGUMENT); mMaxInterval = aMaxInterval; return CHIP_NO_ERROR; @@ -434,9 +433,9 @@ class ReadHandler : public Messaging::ExchangeDelegate // engine, the "oldest" subscription is the subscription with the smallest generation. uint64_t mSubscriptionStartGeneration = 0; - SubscriptionId mSubscriptionId = 0; - uint16_t mMinIntervalFloorSeconds = 0; - uint16_t mMaxInterval = 0; + SubscriptionId mSubscriptionId = 0; + uint16_t mMinIntervalFloorSeconds = 0; + uint16_t mMaxInterval = 0; EventNumber mEventMin = 0; diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 524b632d83452a..fbd40af24d4ee1 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -1343,7 +1343,7 @@ void ParseSubscribeResponseMessage(nlTestSuite * apSuite, chip::TLV::TLVReader & SubscribeResponseMessage::Parser subscribeResponseParser; chip::SubscriptionId subscriptionId = 0; - uint16_t maxInterval = 0; + uint16_t maxInterval = 0; err = subscribeResponseParser.Init(aReader); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); #if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 3032d321d0faf8..8741685ed3cdc5 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -218,8 +218,15 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback static void TestReadHandler_OneSubscribeMultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_TwoSubscribesMultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_MultipleSubscriptionsWithDataVersionFilter(nlTestSuite * apSuite, void * apContext); - static void TestReadHandler_SubscriptionAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext); - static void TestReadHandler_SubscriptionLargeAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest1(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest2(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest3(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest4(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest5(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest6(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest7(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest8(nlTestSuite * apSuite, void * apContext); + static void TestReadHandler_SubscriptionReportingIntervalsTest9(nlTestSuite * apSuite, void * apContext); static void TestReadHandlerResourceExhaustion_MultipleReads(nlTestSuite * apSuite, void * apContext); static void TestReadSubscribeAttributeResponseWithCache(nlTestSuite * apSuite, void * apContext); static void TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite * apSuite, void * apContext); @@ -229,8 +236,6 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback static void TestReadHandler_TwoParallelReadsSecondTooManyPaths(nlTestSuite * apSuite, void * apContext); private: - static constexpr uint16_t kTestMaxInterval = 66; - static uint16_t mMaxInterval; CHIP_ERROR OnSubscriptionRequested(app::ReadHandler & aReadHandler, Transport::SecureSession & aSecureSession) @@ -1619,7 +1624,84 @@ void TestReadInteraction::TestReadHandler_SubscriptionAppRejection(nlTestSuite * gTestReadInteraction.mEmitSubscriptionError = false; } -void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext) +// Subscriber sends the request with particular max-interval value: +// Max interval equal to client-requested min-interval. +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest1(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite](const app::ReadClient & readClient) { + uint16_t minInterval = 0, maxInterval = 0; + + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(apSuite, minInterval == 5); + NL_TEST_ASSERT(apSuite, maxInterval == 5); + + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = false; + + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 5, 5, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls != 0); + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but lower than 60m: +// With no server adjustment. +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest2(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); auto sessionHandle = ctx.GetSessionBobToAlice(); @@ -1649,7 +1731,7 @@ void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals( NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(apSuite, minInterval == 0); - NL_TEST_ASSERT(apSuite, maxInterval == kTestMaxInterval); + NL_TEST_ASSERT(apSuite, maxInterval == 10); numSubscriptionEstablishedCalls++; }; @@ -1663,7 +1745,7 @@ void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals( // // Test the server-side application altering the subscription intervals. // - gTestReadInteraction.mAlterSubscriptionIntervals = true; + gTestReadInteraction.mAlterSubscriptionIntervals = false; NL_TEST_ASSERT(apSuite, Controller::SubscribeAttribute( @@ -1691,15 +1773,18 @@ void TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals( gTestReadInteraction.mAlterSubscriptionIntervals = false; } -void TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingIntervals(nlTestSuite * apSuite, void * apContext) +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but lower than 60m: +// With server adjustment to a value greater than client-requested, but less than 60m (allowed). +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest3(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); auto sessionHandle = ctx.GetSessionBobToAlice(); uint32_t numSuccessCalls = 0; uint32_t numFailureCalls = 0; uint32_t numSubscriptionEstablishedCalls = 0; - uint16_t largeMaxInterval = 6000; - responseDirective = kSendDataResponse; + + responseDirective = kSendDataResponse; // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's // not safe to do so. @@ -1713,13 +1798,16 @@ void TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingInter numFailureCalls++; }; - auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite, - largeMaxInterval](const app::ReadClient & readClient) { + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite](const app::ReadClient & readClient) { uint16_t minInterval = 0, maxInterval = 0; + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(apSuite, minInterval == 0); - NL_TEST_ASSERT(apSuite, maxInterval == largeMaxInterval); + NL_TEST_ASSERT(apSuite, maxInterval == 3000); + numSubscriptionEstablishedCalls++; }; @@ -1733,12 +1821,226 @@ void TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingInter // Test the server-side application altering the subscription intervals. // gTestReadInteraction.mAlterSubscriptionIntervals = true; - gTestReadInteraction.mMaxInterval = largeMaxInterval; + gTestReadInteraction.mMaxInterval = 3000; + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 10, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls != 0); + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but lower than 60m: +// server adjustment to a value greater than client-requested, but greater than 60 (not allowed). +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest4(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = true; + gTestReadInteraction.mMaxInterval = 3700; NL_TEST_ASSERT(apSuite, Controller::SubscribeAttribute( &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 10, - onSubscriptionEstablishedCb, false, true) == CHIP_NO_ERROR); + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 0); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but greater than 60m: +// With no server adjustment. +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest5(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite](const app::ReadClient & readClient) { + uint16_t minInterval = 0, maxInterval = 0; + + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(apSuite, minInterval == 0); + NL_TEST_ASSERT(apSuite, maxInterval == 4000); + + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = false; + + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 4000, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls != 0); + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but greater than 60m: +// With server adjustment to a value lower than 60m. Allowed +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest6(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite](const app::ReadClient & readClient) { + uint16_t minInterval = 0, maxInterval = 0; + + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(apSuite, minInterval == 0); + NL_TEST_ASSERT(apSuite, maxInterval == 3000); + + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = true; + gTestReadInteraction.mMaxInterval = 3000; + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 4000, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); ctx.DrainAndServiceIO(); @@ -1746,7 +2048,7 @@ void TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingInter // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute // implementation. // - NL_TEST_ASSERT(apSuite, numSuccessCalls == 1); + NL_TEST_ASSERT(apSuite, numSuccessCalls != 0); NL_TEST_ASSERT(apSuite, numFailureCalls == 0); NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); @@ -1761,6 +2063,206 @@ void TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingInter gTestReadInteraction.mAlterSubscriptionIntervals = false; } +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but greater than 60m: +// With server adjustment to a value larger than 60m, but less than max interval. Allowed +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest7(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls, &apSuite](const app::ReadClient & readClient) { + uint16_t minInterval = 0, maxInterval = 0; + + CHIP_ERROR err = readClient.GetReportingIntervals(minInterval, maxInterval); + + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + NL_TEST_ASSERT(apSuite, minInterval == 0); + NL_TEST_ASSERT(apSuite, maxInterval == 3700); + + numSubscriptionEstablishedCalls++; + }; + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = true; + gTestReadInteraction.mMaxInterval = 3700; + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 4000, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls != 0); + NL_TEST_ASSERT(apSuite, numFailureCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 1); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 1); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + +// Subscriber sends the request with particular max-interval value: +// Max interval greater than client-requested min-interval but greater than 60m: +// With server adjustment to a value larger than 60m, but larger than max interval. Disallowed +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest8(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = true; + gTestReadInteraction.mMaxInterval = 4100; + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 0, 4000, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 0); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + +// Subscriber sends the request with particular max-interval value: +// Validate client is not requesting max-interval < min-interval. +void TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest9(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + auto sessionHandle = ctx.GetSessionBobToAlice(); + uint32_t numSuccessCalls = 0; + uint32_t numFailureCalls = 0; + uint32_t numSubscriptionEstablishedCalls = 0; + + responseDirective = kSendDataResponse; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onSuccessCb = [&numSuccessCalls](const app::ConcreteDataAttributePath & attributePath, const auto & dataResponse) { + numSuccessCalls++; + }; + + // Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's + // not safe to do so. + auto onFailureCb = [&numFailureCalls](const app::ConcreteDataAttributePath * attributePath, CHIP_ERROR aError) { + numFailureCalls++; + }; + + auto onSubscriptionEstablishedCb = [&numSubscriptionEstablishedCalls](const app::ReadClient & readClient) { + numSubscriptionEstablishedCalls++; + }; + + // + // Test the application callback as well to ensure we get the right number of SubscriptionEstablishment/Termination + // callbacks. + // + app::InteractionModelEngine::GetInstance()->RegisterReadHandlerAppCallback(&gTestReadInteraction); + + // + // Test the server-side application altering the subscription intervals. + // + gTestReadInteraction.mAlterSubscriptionIntervals = false; + + NL_TEST_ASSERT(apSuite, + Controller::SubscribeAttribute( + &ctx.GetExchangeManager(), sessionHandle, kTestEndpointId, onSuccessCb, onFailureCb, 5, 4, + onSubscriptionEstablishedCb, nullptr, true) == CHIP_ERROR_INVALID_ARGUMENT); + + // + // Failures won't get routed to us here since re-subscriptions are enabled by default in the Controller::SubscribeAttribute + // implementation. + // + NL_TEST_ASSERT(apSuite, numSuccessCalls == 0); + NL_TEST_ASSERT(apSuite, numSubscriptionEstablishedCalls == 0); + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + app::InteractionModelEngine::GetInstance()->ShutdownActiveReads(); + + NL_TEST_ASSERT(apSuite, gTestReadInteraction.mNumActiveSubscriptions == 0); + + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + + app::InteractionModelEngine::GetInstance()->UnregisterReadHandlerAppCallback(); + gTestReadInteraction.mAlterSubscriptionIntervals = false; +} + void TestReadInteraction::TestReadHandler_MultipleReads(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); @@ -2690,8 +3192,15 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadHandlerResourceExhaustion_MultipleReads", TestReadInteraction::TestReadHandlerResourceExhaustion_MultipleReads), NL_TEST_DEF("TestReadAttributeTimeout", TestReadInteraction::TestReadAttributeTimeout), NL_TEST_DEF("TestSubscribeAttributeTimeout", TestReadInteraction::TestSubscribeAttributeTimeout), - NL_TEST_DEF("TestReadHandler_SubscriptionAlteredReportingIntervals", TestReadInteraction::TestReadHandler_SubscriptionAlteredReportingIntervals), - NL_TEST_DEF("TestReadHandler_SubscriptionLargeAlteredReportingIntervals", TestReadInteraction::TestReadHandler_SubscriptionLargeAlteredReportingIntervals), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest1", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest1), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest2", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest2), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest3", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest3), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest4", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest4), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest5", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest5), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest6", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest6), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest7", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest7), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest8", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest8), + NL_TEST_DEF("TestReadHandler_SubscriptionReportingIntervalsTest9", TestReadInteraction::TestReadHandler_SubscriptionReportingIntervalsTest9), NL_TEST_DEF("TestReadSubscribeAttributeResponseWithCache", TestReadInteraction::TestReadSubscribeAttributeResponseWithCache), NL_TEST_DEF("TestReadHandler_KillOverQuotaSubscriptions", TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions), NL_TEST_DEF("TestReadHandler_KillOldestSubscriptions", TestReadInteraction::TestReadHandler_KillOldestSubscriptions),