Skip to content

Commit

Permalink
Time sync: Remove ClusterStateCache (#29396)
Browse files Browse the repository at this point in the history 
* Time sync: Remove ClusterStateCache

It's easy to use, but it sure is big. Since we aren't asking for
lists or anything, it's also overkill.

* pack all the things into a class.

* Fiddle with the build file

* Comments on the delegate.

* remove one more app prefix

* Apply suggestions from code review

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

---------

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@cecille @bzbarsky-apple @pull
2 people authored and pull[bot] committed Oct 11, 2023
1 parent 7a10b2a commit 5539882
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 39 deletions.
8 changes: 6 additions & 2 deletions src/app/chip_data_model.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ template("chip_data_model") {
cflags = []
}

if (!defined(defines)) {
defines = []
}

foreach(cluster, _cluster_sources) {
if (cluster == "door-lock-server") {
sources += [
Expand Down Expand Up @@ -262,8 +266,8 @@ template("chip_data_model") {
"${_app_root}/clusters/${cluster}/DefaultTimeSyncDelegate.cpp",
"${_app_root}/clusters/${cluster}/TimeSyncDataProvider.cpp",
]
cflags +=
[ "-DTIME_SYNC_ENABLE_TSC_FEATURE=${time_sync_enable_tsc_feature}" ]
defines +=
[ "TIME_SYNC_ENABLE_TSC_FEATURE=${time_sync_enable_tsc_feature}" ]
} else if (cluster == "scenes-server") {
sources += [
"${_app_root}/clusters/${cluster}/${cluster}.cpp",
Expand Down
6 changes: 5 additions & 1 deletion src/app/clusters/time-synchronization-server/time-synchronization-delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ class Delegate
* local network defined NTP (DHCPv6 -> DHCP -> DNS-SD sources)
* If the delegate is unable to support any source, it may return an error immediately. If the delegate is going
* to attempt to obtain time from any source, it returns CHIP_NO_ERROR and calls the callback on completion.
* If the delegate successfully obtains the time, it sets the time using the platform time API (SetClock_RealTime)
* If the delegate has a time available at the time of this call, it may call the callback synchronously from within
* this function.
* If the delegate needs to reach out asynchronously to obtain a time, it saves this callback to call asynchronously.
* The delegate should track these callbacks in a CallbackDeque to ensure they can be properly cancelled.
* If the delegate is successful in obtaining the time, it sets the time using the platform time API (SetClock_RealTime)
* and calls the callback with the time source and granularity set as appropriate.
* If the delegate is unsuccessful in obtaining the time, it calls the callback with timeSource set to kNone and
* granularity set to kNoTimeGranularity.
Expand Down
72 changes: 41 additions & 31 deletions src/app/clusters/time-synchronization-server/time-synchronization-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,41 +300,30 @@ void TimeSynchronizationServer::AttemptToGetFallbackNTPTimeFromDelegate()
void TimeSynchronizationServer::OnDeviceConnectedFn(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
{
// Connected to our trusted time source, let's read the time.
app::AttributePathParams readPaths[2];
readPaths[0] = app::AttributePathParams(kRootEndpointId, app::Clusters::TimeSynchronization::Id,
app::Clusters::TimeSynchronization::Attributes::UTCTime::Id);
readPaths[1] = app::AttributePathParams(kRootEndpointId, app::Clusters::TimeSynchronization::Id,
app::Clusters::TimeSynchronization::Attributes::Granularity::Id);

app::InteractionModelEngine * engine = app::InteractionModelEngine::GetInstance();
app::ReadPrepareParams readParams(sessionHandle);
AttributePathParams readPaths[2];
readPaths[0] = AttributePathParams(kRootEndpointId, Id, Attributes::UTCTime::Id);
readPaths[1] = AttributePathParams(kRootEndpointId, Id, Attributes::Granularity::Id);

InteractionModelEngine * engine = InteractionModelEngine::GetInstance();
ReadPrepareParams readParams(sessionHandle);
readParams.mpAttributePathParamsList = readPaths;
readParams.mAttributePathParamsListSize = 2;

auto attributeCache = Platform::MakeUnique<app::ClusterStateCache>(*this);
if (attributeCache == nullptr)
auto readInfo = Platform::MakeUnique<TimeReadInfo>(engine, &exchangeMgr, *this, ReadClient::InteractionType::Read);
if (readInfo == nullptr)
{
// This is unlikely to work if we don't have memory, but let's try
OnDeviceConnectionFailureFn();
return;
}
auto readClient = chip::Platform::MakeUnique<app::ReadClient>(engine, &exchangeMgr, attributeCache->GetBufferedCallback(),
app::ReadClient::InteractionType::Read);
if (readClient == nullptr)
{
// This is unlikely to work if we don't have memory, but let's try
OnDeviceConnectionFailureFn();
return;
}
CHIP_ERROR err = readClient->SendRequest(readParams);
CHIP_ERROR err = readInfo->readClient.SendRequest(readParams);
if (err != CHIP_NO_ERROR)
{
ChipLogError(Zcl, "Failed to read UTC time from trusted source");
OnDeviceConnectionFailureFn();
return;
}
mAttributeCache = std::move(attributeCache);
mReadClient = std::move(readClient);
mTimeReadInfo = std::move(readInfo);
}

void TimeSynchronizationServer::OnDeviceConnectionFailureFn()
Expand All @@ -343,20 +332,39 @@ void TimeSynchronizationServer::OnDeviceConnectionFailureFn()
AttemptToGetFallbackNTPTimeFromDelegate();
}

void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
void TimeSynchronizationServer::OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData,
const StatusIB & aStatus)
{
using namespace chip::app::Clusters::TimeSynchronization::Attributes;

Granularity::TypeInfo::Type granularity = GranularityEnum::kNoTimeGranularity;
mAttributeCache->Get<Granularity::TypeInfo>(kRootEndpointId, granularity);
if (aPath.mClusterId != Id || aStatus.IsFailure())
{
return;
}
switch (aPath.mAttributeId)
{
case Attributes::UTCTime::Id:
if (DataModel::Decode(*apData, mTimeReadInfo->utcTime) != CHIP_NO_ERROR)
{
mTimeReadInfo->utcTime.SetNull();
}
break;
case Attributes::Granularity::Id:
if (DataModel::Decode(*apData, mTimeReadInfo->granularity) != CHIP_NO_ERROR)
{
mTimeReadInfo->granularity = GranularityEnum::kNoTimeGranularity;
}
break;
default:
break;
}
}

UTCTime::TypeInfo::Type time;
CHIP_ERROR err = mAttributeCache->Get<UTCTime::TypeInfo>(kRootEndpointId, time);
if (err == CHIP_NO_ERROR && !time.IsNull() && granularity != GranularityEnum::kNoTimeGranularity)
void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
{
if (!mTimeReadInfo->utcTime.IsNull() && mTimeReadInfo->granularity != GranularityEnum::kNoTimeGranularity)
{
GranularityEnum ourGranularity;
// Being conservative with granularity - nothing smaller than seconds because of network delay
switch (granularity)
switch (mTimeReadInfo->granularity)
{
case GranularityEnum::kMinutesGranularity:
case GranularityEnum::kSecondsGranularity:
Expand All @@ -367,7 +375,8 @@ void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
break;
}

err = SetUTCTime(kRootEndpointId, time.Value(), ourGranularity, TimeSourceEnum::kNodeTimeCluster);
CHIP_ERROR err =
SetUTCTime(kRootEndpointId, mTimeReadInfo->utcTime.Value(), ourGranularity, TimeSourceEnum::kNodeTimeCluster);
if (err == CHIP_NO_ERROR)
{
return;
Expand All @@ -377,6 +386,7 @@ void TimeSynchronizationServer::OnDone(ReadClient * apReadClient)
// If we failed to set the UTC time, it doesn't hurt to try the backup - NTP system might have different permissions on the
// system clock
AttemptToGetFallbackNTPTimeFromDelegate();
mTimeReadInfo = nullptr;
}
#endif

Expand Down
21 changes: 16 additions & 5 deletions src/app/clusters/time-synchronization-server/time-synchronization-server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ enum class TimeSyncEventFlag : uint8_t
class TimeSynchronizationServer : public FabricTable::Delegate
#if TIME_SYNC_ENABLE_TSC_FEATURE
,
public ClusterStateCache::Callback
public ReadClient::Callback
#endif
{
public:
Expand Down Expand Up @@ -117,8 +117,8 @@ class TimeSynchronizationServer : public FabricTable::Delegate
void OnDeviceConnectedFn(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle);
void OnDeviceConnectionFailureFn();

// AttributeCache::Callback functions
void OnAttributeChanged(ClusterStateCache * cache, const ConcreteAttributePath & path) override {}
// ReadClient::Callback functions
void OnAttributeData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override;
void OnDone(ReadClient * apReadClient) override;
#endif

Expand All @@ -142,10 +142,21 @@ class TimeSynchronizationServer : public FabricTable::Delegate
static TimeSynchronizationServer sTimeSyncInstance;
TimeSyncEventFlag mEventFlag = TimeSyncEventFlag::kNone;
#if TIME_SYNC_ENABLE_TSC_FEATURE
Platform::UniquePtr<app::ClusterStateCache> mAttributeCache;
Platform::UniquePtr<app::ReadClient> mReadClient;
chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
struct TimeReadInfo
{
TimeReadInfo(InteractionModelEngine * apImEngine, Messaging::ExchangeManager * apExchangeMgr,
ReadClient::Callback & apCallback, ReadClient::InteractionType aInteractionType) :
readClient(apImEngine, apExchangeMgr, apCallback, aInteractionType)
{
utcTime.SetNull();
}
Attributes::UTCTime::TypeInfo::DecodableType utcTime;
Attributes::Granularity::TypeInfo::DecodableType granularity = GranularityEnum::kNoTimeGranularity;
ReadClient readClient;
};
Platform::UniquePtr<TimeReadInfo> mTimeReadInfo;
#endif
chip::Callback::Callback<OnTimeSyncCompletion> mOnTimeSyncCompletion;
chip::Callback::Callback<OnFallbackNTPCompletion> mOnFallbackNTPCompletion;
Expand Down

0 comments on commit 5539882

Please sign in to comment.