From 108ae4f22f8fb5f8b7209905231f5eb6f8e29e62 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 30 Sep 2024 13:55:10 -0400 Subject: [PATCH] Improve DataVersion filter list logging when subscribing. We logged the encoded filters when doing a re-subscribe with a ClusterStateCache, but not when doing an initial subscribe. This moves the logging into the encoding of the filter list, so it's logged consistently. Also logs the DataVersion that we get back in MTRDevice_Concrete. --- src/app/ClusterStateCache.cpp | 10 +--------- src/app/MessageDef/DataVersionFilterIBs.cpp | 15 +++++++++++++++ src/app/MessageDef/DataVersionFilterIBs.h | 11 +++++++++++ src/app/ReadClient.cpp | 15 +-------------- src/app/ReadClient.h | 2 -- src/darwin/Framework/CHIP/MTRDevice_Concrete.mm | 2 ++ 6 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/app/ClusterStateCache.cpp b/src/app/ClusterStateCache.cpp index 65ab12095bfeba..fdb8e40abac0b7 100644 --- a/src/app/ClusterStateCache.cpp +++ b/src/app/ClusterStateCache.cpp @@ -626,15 +626,7 @@ CHIP_ERROR ClusterStateCacheT::OnUpdateDataVersionFilterLi continue; } - DataVersionFilterIB::Builder & filterIB = aDataVersionFilterIBsBuilder.CreateDataVersionFilter(); - SuccessOrExit(err = aDataVersionFilterIBsBuilder.GetError()); - ClusterPathIB::Builder & filterPath = filterIB.CreatePath(); - SuccessOrExit(err = filterIB.GetError()); - SuccessOrExit(err = filterPath.Endpoint(filter.first.mEndpointId).Cluster(filter.first.mClusterId).EndOfClusterPathIB()); - SuccessOrExit(err = filterIB.DataVersion(filter.first.mDataVersion.Value()).EndOfDataVersionFilterIB()); - ChipLogProgress(DataManagement, "Update DataVersionFilter: Endpoint=%u Cluster=" ChipLogFormatMEI " Version=%" PRIu32, - filter.first.mEndpointId, ChipLogValueMEI(filter.first.mClusterId), filter.first.mDataVersion.Value()); - + SuccessOrExit(err = aDataVersionFilterIBsBuilder.EncodeDataVersionFilterIB(filter.first)); aEncodedDataVersionList = true; } diff --git a/src/app/MessageDef/DataVersionFilterIBs.cpp b/src/app/MessageDef/DataVersionFilterIBs.cpp index ca6008975d7992..91b9e63d25fd90 100644 --- a/src/app/MessageDef/DataVersionFilterIBs.cpp +++ b/src/app/MessageDef/DataVersionFilterIBs.cpp @@ -69,6 +69,21 @@ DataVersionFilterIB::Builder & DataVersionFilterIBs::Builder::CreateDataVersionF return mDataVersionFilter; } +CHIP_ERROR DataVersionFilterIBs::Builder::EncodeDataVersionFilterIB(const DataVersionFilter & aFilter) +{ + DataVersionFilterIB::Builder & filterIB = CreateDataVersionFilter(); + ReturnErrorOnFailure(GetError()); + ClusterPathIB::Builder & path = filterIB.CreatePath(); + ReturnErrorOnFailure(filterIB.GetError()); + ReturnErrorOnFailure(path.Endpoint(aFilter.mEndpointId).Cluster(aFilter.mClusterId).EndOfClusterPathIB()); + ReturnErrorOnFailure(filterIB.DataVersion(aFilter.mDataVersion.Value()).EndOfDataVersionFilterIB()); + + ChipLogProgress(DataManagement, "Encoded DataVersionFilter: Endpoint=%u Cluster=" ChipLogFormatMEI " Version=%" PRIu32, + aFilter.mEndpointId, ChipLogValueMEI(aFilter.mClusterId), aFilter.mDataVersion.Value()); + + return CHIP_NO_ERROR; +} + CHIP_ERROR DataVersionFilterIBs::Builder::EndOfDataVersionFilterIBs() { EndOfContainer(); diff --git a/src/app/MessageDef/DataVersionFilterIBs.h b/src/app/MessageDef/DataVersionFilterIBs.h index 199a6686cb011f..bb753006e4dd19 100644 --- a/src/app/MessageDef/DataVersionFilterIBs.h +++ b/src/app/MessageDef/DataVersionFilterIBs.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include #include #include @@ -54,6 +55,16 @@ class Builder : public ArrayBuilder */ DataVersionFilterIB::Builder & GetDataVersionFilter() { return mDataVersionFilter; }; + /** + * Add a DataVersionFilter to the list. This is a convenience method + * that will handle calling CreateDataVersionFilter() and then using the + * result to encode the provided DataVersionFilter. + * + * The passed-in DataVersionFilter is assumed to pass the + * IsValidDataVersionFilter() test. + */ + CHIP_ERROR EncodeDataVersionFilterIB(const DataVersionFilter & aFilter); + /** * @brief Mark the end of this DataVersionFilterIBs * diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 91b8adb0721608..1e9c7e727f409c 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -433,7 +433,7 @@ CHIP_ERROR ReadClient::BuildDataVersionFilterList(DataVersionFilterIBs::Builder TLV::TLVWriter backup; aDataVersionFilterIBsBuilder.Checkpoint(backup); - CHIP_ERROR err = EncodeDataVersionFilter(aDataVersionFilterIBsBuilder, filter); + CHIP_ERROR err = aDataVersionFilterIBsBuilder.EncodeDataVersionFilterIB(filter); if (err == CHIP_NO_ERROR) { #if CHIP_PROGRESS_LOGGING @@ -464,19 +464,6 @@ CHIP_ERROR ReadClient::BuildDataVersionFilterList(DataVersionFilterIBs::Builder return CHIP_NO_ERROR; } -CHIP_ERROR ReadClient::EncodeDataVersionFilter(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder, - DataVersionFilter const & aFilter) -{ - // Caller has checked aFilter.IsValidDataVersionFilter() - DataVersionFilterIB::Builder & filterIB = aDataVersionFilterIBsBuilder.CreateDataVersionFilter(); - ReturnErrorOnFailure(aDataVersionFilterIBsBuilder.GetError()); - ClusterPathIB::Builder & path = filterIB.CreatePath(); - ReturnErrorOnFailure(filterIB.GetError()); - ReturnErrorOnFailure(path.Endpoint(aFilter.mEndpointId).Cluster(aFilter.mClusterId).EndOfClusterPathIB()); - ReturnErrorOnFailure(filterIB.DataVersion(aFilter.mDataVersion.Value()).EndOfDataVersionFilterIB()); - return CHIP_NO_ERROR; -} - CHIP_ERROR ReadClient::GenerateDataVersionFilterList(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder, const Span & aAttributePaths, const Span & aDataVersionFilters, diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 468aba012e07ae..fd27f0afc9e959 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -562,8 +562,6 @@ class ReadClient : public Messaging::ExchangeDelegate CHIP_ERROR BuildDataVersionFilterList(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder, const Span & aAttributePaths, const Span & aDataVersionFilters, bool & aEncodedDataVersionList); - CHIP_ERROR EncodeDataVersionFilter(DataVersionFilterIBs::Builder & aDataVersionFilterIBsBuilder, - DataVersionFilter const & aFilter); CHIP_ERROR ReadICDOperatingModeFromAttributeDataIB(TLV::TLVReader && aReader, PeerType & aType); CHIP_ERROR ProcessAttributeReportIBs(TLV::TLVReader & aAttributeDataIBsReader); CHIP_ERROR ProcessEventReportIBs(TLV::TLVReader & aEventReportIBsReader); diff --git a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm index e69417b1fb23ee..5d6e47c8d9d781 100644 --- a/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm +++ b/src/darwin/Framework/CHIP/MTRDevice_Concrete.mm @@ -3322,6 +3322,8 @@ - (void)_noteDataVersion:(NSNumber *)dataVersion forClusterPath:(MTRClusterPath _clusterDataToPersist = [NSMutableDictionary dictionary]; } _clusterDataToPersist[clusterPath] = clusterData; + + MTR_LOG("%@ updated DataVersion for %@ to %@", self, clusterPath, dataVersion); } }