Skip to content

Commit

Permalink
address PR comments
Browse files Browse the repository at this point in the history 
Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@fessehaeve @bzbarsky-apple
fessehaeve and bzbarsky-apple committed Jun 2, 2023
1 parent df139ef commit e2a2f10
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 41 deletions.
56 changes: 34 additions & 22 deletions src/app/clusters/time-synchronization-server/TimeSyncDataProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace chip {
constexpr size_t kTrustedTimeSourceMaxSerializedSize =
TLV::EstimateStructOverhead(sizeof(chip::FabricIndex), sizeof(chip::NodeId), sizeof(chip::EndpointId));
constexpr size_t kTimeZoneMaxSerializedSize =
TLV::EstimateStructOverhead(sizeof(int32_t), sizeof(uint64_t), 64); // 64 for name field
TLV::EstimateStructOverhead(sizeof(int32_t), sizeof(uint64_t), static_cast<size_t>(64)); // 64 for name field
constexpr size_t kDSTOffsetMaxSerializedSize = TLV::EstimateStructOverhead(sizeof(int32_t), sizeof(uint64_t), sizeof(uint64_t));

// Multiply the serialized size by the maximum number of list size and add 2 bytes for the array start and end.
Expand Down Expand Up @@ -54,7 +54,7 @@ CHIP_ERROR TimeSyncDataProvider::LoadTrustedTimeSource(TrustedTimeSource & timeS

TLV::TLVReader reader;

reader.Init(bufferSpan.data(), bufferSpan.size());
reader.Init(bufferSpan);
ReturnErrorOnFailure(reader.Next(TLV::AnonymousTag()));
ReturnErrorOnFailure(timeSource.Decode(reader));

Expand All @@ -72,9 +72,12 @@ CHIP_ERROR TimeSyncDataProvider::StoreDefaultNtp(const CharSpan & defaultNtp)
static_cast<uint16_t>(defaultNtp.size()));
}

CHIP_ERROR TimeSyncDataProvider::LoadDefaultNtp(MutableByteSpan & defaultNtp)
CHIP_ERROR TimeSyncDataProvider::LoadDefaultNtp(CharSpan & defaultNtp)
{
return Load(DefaultStorageKeyAllocator::TSDefaultNTP().KeyName(), defaultNtp);
MutableByteSpan dntpSpan(const_cast<uint8_t *>(Uint8::from_const_char(defaultNtp.data())), defaultNtp.size());
ReturnErrorOnFailure(Load(DefaultStorageKeyAllocator::TSDefaultNTP().KeyName(), dntpSpan));
defaultNtp.reduce_size(dntpSpan.size());
return CHIP_NO_ERROR;
}

CHIP_ERROR TimeSyncDataProvider::ClearDefaultNtp()
Expand Down Expand Up @@ -105,51 +108,56 @@ CHIP_ERROR TimeSyncDataProvider::LoadTimeZone(TimeZone & timeZoneList, uint8_t &
{
uint8_t buffer[kTimeZoneListMaxSerializedSize];
MutableByteSpan bufferSpan(buffer);
size = 0;
CHIP_ERROR err = CHIP_NO_ERROR;
CHIP_ERROR err;
size = 0;

ReturnErrorOnFailure(Load(DefaultStorageKeyAllocator::TSTimeZone().KeyName(), bufferSpan));

TLV::TLVReader reader;
TLV::TLVType outerType;

reader.Init(bufferSpan.data(), bufferSpan.size());
reader.Init(bufferSpan);
ReturnErrorOnFailure(reader.Next(TLV::TLVType::kTLVType_Array, TLV::AnonymousTag()));
ReturnErrorOnFailure(reader.EnterContainer(outerType));
auto tz = timeZoneList.begin();
uint8_t i = 0;

while (reader.Next() != CHIP_ERROR_END_OF_TLV && i < timeZoneList.size())
while ((err = reader.Next()) == CHIP_NO_ERROR && i < timeZoneList.size())
{
auto & tz = timeZoneList[i];
app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type timeZone;
ReturnErrorOnFailure(timeZone.Decode(reader));
tz[i].offset = timeZone.offset;
tz[i].validAt = timeZone.validAt;
tz.offset = timeZone.offset;
tz.validAt = timeZone.validAt;
if (timeZone.name.HasValue())
{
if (timeZone.name.Value().size() <= tz[i].name.Value().size())
if (tz.name.HasValue() && timeZone.name.Value().size() <= tz.name.Value().size())
{
char * dest = const_cast<char *>(tz[i].name.Value().data());
char * dest = const_cast<char *>(tz.name.Value().data());
size_t len = timeZone.name.Value().size();
memcpy(dest, timeZone.name.Value().data(), len);
tz[i].name.SetValue(chip::CharSpan(tz[i].name.Value().data(), len));
tz.name.SetValue(chip::CharSpan(tz.name.Value().data(), len));
}
else
{
err = CHIP_ERROR_BUFFER_TOO_SMALL;
return CHIP_ERROR_BUFFER_TOO_SMALL;
}
}
else
{
tz[i].name.ClearValue();
tz.name.ClearValue();
}
i++;
}
VerifyOrReturnError(err == CHIP_END_OF_TLV, CHIP_IM_GLOBAL_STATUS(ConstraintError));

ReturnErrorOnFailure(reader.ExitContainer(outerType));

err = reader.Next();
VerifyOrReturnError(err == CHIP_END_OF_TLV, CHIP_IM_GLOBAL_STATUS(ConstraintError));

size = i;

return err;
return CHIP_NO_ERROR;
}

CHIP_ERROR TimeSyncDataProvider::ClearTimeZone()
Expand Down Expand Up @@ -182,28 +190,32 @@ CHIP_ERROR TimeSyncDataProvider::LoadDSTOffset(DSTOffset & dstOffsetList, uint8_
{
uint8_t buffer[kDSTOffsetListMaxSerializedSize];
MutableByteSpan bufferSpan(buffer);
CHIP_ERROR err;
size = 0;

ReturnErrorOnFailure(Load(DefaultStorageKeyAllocator::TSDSTOffset().KeyName(), bufferSpan));

TLV::TLVReader reader;
TLV::TLVType outerType;

reader.Init(bufferSpan.data(), bufferSpan.size());
reader.Init(bufferSpan);
ReturnErrorOnFailure(reader.Next(TLV::TLVType::kTLVType_Array, TLV::AnonymousTag()));
ReturnErrorOnFailure(reader.EnterContainer(outerType));
auto dst = dstOffsetList.begin();
uint8_t i = 0;

while (reader.Next() != CHIP_ERROR_END_OF_TLV && i < dstOffsetList.size())
while ((err = reader.Next()) == CHIP_NO_ERROR && i < dstOffsetList.size())
{
app::Clusters::TimeSynchronization::Structs::DSTOffsetStruct::Type dstOffset;
ReturnErrorOnFailure(dstOffset.Decode(reader));
dst[i] = dstOffset;
ReturnErrorOnFailure(dst[i].Decode(reader));
i++;
}
VerifyOrReturnError(err == CHIP_END_OF_TLV, CHIP_IM_GLOBAL_STATUS(ConstraintError));

ReturnErrorOnFailure(reader.ExitContainer(outerType));

err = reader.Next();
VerifyOrReturnError(err == CHIP_END_OF_TLV, CHIP_IM_GLOBAL_STATUS(ConstraintError));

size = i;

return CHIP_NO_ERROR;
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/time-synchronization-server/TimeSyncDataProvider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class TimeSyncDataProvider
CHIP_ERROR ClearTrustedTimeSource();

CHIP_ERROR StoreDefaultNtp(const CharSpan & defaultNtp);
CHIP_ERROR LoadDefaultNtp(MutableByteSpan & defaultNtp);
CHIP_ERROR LoadDefaultNtp(CharSpan & defaultNtp);
CHIP_ERROR ClearDefaultNtp();

CHIP_ERROR StoreTimeZone(const TimeZone & timeZoneList);
Expand Down
25 changes: 10 additions & 15 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 @@ -473,21 +473,19 @@ DataModel::Nullable<TimeSynchronization::Structs::TrustedTimeSourceStruct::Type>
return mTrustedTimeSource;
}

CHIP_ERROR TimeSynchronizationServer::GetDefaultNtp(MutableByteSpan & dntp)
CHIP_ERROR TimeSynchronizationServer::GetDefaultNtp(CharSpan & dntp)
{
return mTimeSyncDataProvider.LoadDefaultNtp(dntp);
}

DataModel::List<TimeSynchronization::Structs::TimeZoneStruct::Type> & TimeSynchronizationServer::GetTimeZone()
DataModel::List<TimeSynchronization::Structs::TimeZoneStruct::Type> TimeSynchronizationServer::GetTimeZone()
{
mTimeZoneList = DataModel::List<TimeSynchronization::Structs::TimeZoneStruct::Type>(mTz, mTimeZoneListSize);
return mTimeZoneList;
return mTimeZoneList.SubSpan(0, mTimeZoneListSize);
}

DataModel::List<TimeSynchronization::Structs::DSTOffsetStruct::Type> & TimeSynchronizationServer::GetDSTOffset()
DataModel::List<TimeSynchronization::Structs::DSTOffsetStruct::Type> TimeSynchronizationServer::GetDSTOffset()
{
mDstOffsetList = DataModel::List<TimeSynchronization::Structs::DSTOffsetStruct::Type>(mDst, mDstOffsetListSize);
return mDstOffsetList;
return mDstOffsetList.SubSpan(0, mDstOffsetListSize);
}

void TimeSynchronizationServer::ScheduleDelayedAction(System::Clock::Seconds32 delay, System::TimerCompleteCallback action,
Expand Down Expand Up @@ -551,8 +549,7 @@ TimeState TimeSynchronizationServer::GetUpdatedTimeZoneState()
mTimeZoneListSize = static_cast<uint8_t>(tzList.size() - activeTzIndex);
auto newTimeZoneList = tzList.SubSpan(activeTzIndex);
VerifyOrReturnValue(mTimeSyncDataProvider.StoreTimeZone(newTimeZoneList) == CHIP_NO_ERROR, TimeState::kInvalid);
VerifyOrReturnValue(mTimeSyncDataProvider.LoadTimeZone(TimeSynchronizationServer::Instance().GetTimeZone(),
mTimeZoneListSize) == CHIP_NO_ERROR,
VerifyOrReturnValue(mTimeSyncDataProvider.LoadTimeZone(mTimeZoneList, mTimeZoneListSize) == CHIP_NO_ERROR,
TimeState::kInvalid);
return TimeState::kChanged;
}
Expand Down Expand Up @@ -587,8 +584,7 @@ TimeState TimeSynchronizationServer::GetUpdatedDSTOffsetState()
mDstOffsetListSize = static_cast<uint8_t>(dstList.size() - activeDstIndex);
auto newDstOffsetList = dstList.SubSpan(activeDstIndex);
VerifyOrReturnValue(mTimeSyncDataProvider.StoreDSTOffset(newDstOffsetList) == CHIP_NO_ERROR, TimeState::kInvalid);
VerifyOrReturnValue(mTimeSyncDataProvider.LoadDSTOffset(TimeSynchronizationServer::Instance().GetDSTOffset(),
mDstOffsetListSize) == CHIP_NO_ERROR,
VerifyOrReturnValue(mTimeSyncDataProvider.LoadDSTOffset(mDstOffsetList, mDstOffsetListSize) == CHIP_NO_ERROR,
TimeState::kInvalid);
return TimeState::kChanged;
}
Expand Down Expand Up @@ -635,12 +631,11 @@ CHIP_ERROR TimeSynchronizationAttrAccess::ReadTrustedTimeSource(EndpointId endpo
CHIP_ERROR TimeSynchronizationAttrAccess::ReadDefaultNtp(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
CHIP_ERROR err = CHIP_NO_ERROR;
uint8_t buffer[DefaultNTP::TypeInfo::MaxLength()];
MutableByteSpan dntp(buffer);
char buffer[DefaultNTP::TypeInfo::MaxLength()];
chip::CharSpan dntp(buffer);
if (TimeSynchronizationServer::Instance().GetDefaultNtp(dntp) == CHIP_NO_ERROR && dntp.size() != 0)
{
const char * charBuf = reinterpret_cast<const char *>(buffer);
err = aEncoder.Encode(chip::CharSpan(charBuf, dntp.size()));
err = aEncoder.Encode(dntp);
}
else
{
Expand Down
6 changes: 3 additions & 3 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 @@ -94,9 +94,9 @@ class TimeSynchronizationServer
CHIP_ERROR SetDSTOffset(DataModel::DecodableList<TimeSynchronization::Structs::DSTOffsetStruct::Type> dstL);
CHIP_ERROR ClearDSTOffset(void);
DataModel::Nullable<TimeSynchronization::Structs::TrustedTimeSourceStruct::Type> & GetTrustedTimeSource(void);
CHIP_ERROR GetDefaultNtp(MutableByteSpan & dntp);
DataModel::List<TimeSynchronization::Structs::TimeZoneStruct::Type> & GetTimeZone(void);
DataModel::List<TimeSynchronization::Structs::DSTOffsetStruct::Type> & GetDSTOffset(void);
CHIP_ERROR GetDefaultNtp(CharSpan & dntp);
DataModel::List<TimeSynchronization::Structs::TimeZoneStruct::Type> GetTimeZone(void);
DataModel::List<TimeSynchronization::Structs::DSTOffsetStruct::Type> GetDSTOffset(void);

CHIP_ERROR SetUTCTime(chip::EndpointId ep, uint64_t utcTime, TimeSynchronization::GranularityEnum granularity,
TimeSynchronization::TimeSourceEnum source);
Expand Down

0 comments on commit e2a2f10

Please sign in to comment.