Skip to content

Commit

Permalink
Factor out common parts of list iterators into shared super-classes. (p…
Browse files Browse the repository at this point in the history 
…roject-chip#36279)

* Factor out common parts of list iterators into shared super-classes.

Saves some codesize.

Fixes project-chip#36264

* Remove unused variables.
  • Loading branch information
@bzbarsky-apple @yyzhong-g
bzbarsky-apple authored and yyzhong-g committed Dec 11, 2024
1 parent d072237 commit 011b171
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 62 deletions.
184 changes: 128 additions & 56 deletions src/app/data-model/DecodableList.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,103 @@ class DecodableListBase
}

protected:
class Iterator
{
public:
Iterator(const TLV::TLVReader & reader)
{
mStatus = CHIP_NO_ERROR;
mReader.Init(reader);
}

bool Next()
{
if (mReader.GetContainerType() == TLV::kTLVType_NotSpecified)
{
return false;
}

if (mStatus == CHIP_NO_ERROR)
{
mStatus = mReader.Next();
}

return (mStatus == CHIP_NO_ERROR);
}

/*
* Returns the result of all previous operations on this iterator.
*
* Notably, if the end-of-list was encountered in a previous call to Next,
* the status returned shall be CHIP_NO_ERROR.
*/
CHIP_ERROR GetStatus() const
{
if (mStatus == CHIP_END_OF_TLV)
{
return CHIP_NO_ERROR;
}

return mStatus;
}

protected:
CHIP_ERROR mStatus;
TLV::TLVReader mReader;
};

TLV::TLVReader mReader;
chip::Optional<FabricIndex> mFabricIndex;
};

template <bool IsFabricScoped>
class FabricIndexListMemberMixin
{
};

template <>
class FabricIndexListMemberMixin<true>
{
public:
void SetFabricIndex(FabricIndex fabricIndex) { mFabricIndex.SetValue(fabricIndex); }

protected:
Optional<FabricIndex> mFabricIndex;
};

template <bool IsFabricScoped>
class FabricIndexIteratorMemberMixin
{
};

template <>
class FabricIndexIteratorMemberMixin<true>
{
public:
FabricIndexIteratorMemberMixin(const Optional<FabricIndex> & fabricindex) : mFabricIndex(fabricindex) {}

protected:
const Optional<FabricIndex> mFabricIndex;
};

template <bool IsFabricScoped>
class DecodableMaybeFabricScopedList : public DecodableListBase, public FabricIndexListMemberMixin<IsFabricScoped>
{
public:
static constexpr bool kIsFabricScoped = IsFabricScoped;

protected:
class Iterator : public DecodableListBase::Iterator, public FabricIndexIteratorMemberMixin<IsFabricScoped>
{
public:
template <bool IsActuallyFabricScoped = IsFabricScoped, std::enable_if_t<IsActuallyFabricScoped, bool> = true>
Iterator(const TLV::TLVReader & reader, const Optional<FabricIndex> & fabricIndex) :
DecodableListBase::Iterator(reader), FabricIndexIteratorMemberMixin<IsFabricScoped>(fabricIndex)
{}

template <bool IsActuallyFabricScoped = IsFabricScoped, std::enable_if_t<!IsActuallyFabricScoped, bool> = true>
Iterator(const TLV::TLVReader & reader) : DecodableListBase::Iterator(reader)
{}
};
};

} // namespace detail
Expand All @@ -109,21 +204,15 @@ class DecodableListBase
*
*/
template <typename T>
class DecodableList : public detail::DecodableListBase
class DecodableList : public detail::DecodableMaybeFabricScopedList<DataModel::IsFabricScoped<T>::value>
{
public:
DecodableList() {}

static constexpr bool kIsFabricScoped = DataModel::IsFabricScoped<T>::value;

template <typename T0 = T, std::enable_if_t<DataModel::IsFabricScoped<T0>::value, bool> = true>
void SetFabricIndex(FabricIndex fabricIndex)
class Iterator : public detail::DecodableMaybeFabricScopedList<DataModel::IsFabricScoped<T>::value>::Iterator
{
mFabricIndex.SetValue(fabricIndex);
}
using IteratorBase = typename detail::DecodableMaybeFabricScopedList<DataModel::IsFabricScoped<T>::value>::Iterator;

class Iterator
{
public:
/*
* Initialize the iterator with a reference to a reader.
Expand All @@ -133,11 +222,13 @@ class DecodableList : public detail::DecodableListBase
* have a `kTLVType_NotSpecified` container type if there is
* no list.
*/
Iterator(const TLV::TLVReader & reader, Optional<FabricIndex> fabricIndex) : mFabricIndex(fabricIndex)
{
mStatus = CHIP_NO_ERROR;
mReader.Init(reader);
}
template <typename T0 = T, std::enable_if_t<DataModel::IsFabricScoped<T0>::value, bool> = true>
Iterator(const TLV::TLVReader & reader, Optional<FabricIndex> fabricIndex) : IteratorBase(reader, fabricIndex)
{}

template <typename T0 = T, std::enable_if_t<!DataModel::IsFabricScoped<T0>::value, bool> = true>
Iterator(const TLV::TLVReader & reader) : IteratorBase(reader)
{}

/*
* Increments the iterator to point to the next list element
Expand All @@ -157,17 +248,17 @@ class DecodableList : public detail::DecodableListBase
template <typename T0 = T, std::enable_if_t<!DataModel::IsFabricScoped<T0>::value, bool> = true>
bool Next()
{
return DoNext();
return IteratorBase::Next() && DecodeValue();
}

template <typename T0 = T, std::enable_if_t<DataModel::IsFabricScoped<T0>::value, bool> = true>
bool Next()
{
bool hasNext = DoNext();
bool hasNext = IteratorBase::Next() && DecodeValue();

if (hasNext && mFabricIndex.HasValue())
if (hasNext && this->mFabricIndex.HasValue())
{
mValue.SetFabricIndex(mFabricIndex.Value());
mValue.SetFabricIndex(this->mFabricIndex.Value());
}

return hasNext;
Expand All @@ -179,36 +270,10 @@ class DecodableList : public detail::DecodableListBase
*/
const T & GetValue() const { return mValue; }

/*
* Returns the result of all previous operations on this iterator.
*
* Notably, if the end-of-list was encountered in a previous call to Next,
* the status returned shall be CHIP_NO_ERROR.
*/
CHIP_ERROR GetStatus() const
{
if (mStatus == CHIP_END_OF_TLV)
{
return CHIP_NO_ERROR;
}

return mStatus;
}

private:
bool DoNext()
bool DecodeValue()
{
if (mReader.GetContainerType() == TLV::kTLVType_NotSpecified)
{
return false;
}

if (mStatus == CHIP_NO_ERROR)
{
mStatus = mReader.Next();
}

if (mStatus == CHIP_NO_ERROR)
if (this->mStatus == CHIP_NO_ERROR)
{
//
// Re-construct mValue to reset its state back to cluster object defaults.
Expand All @@ -218,22 +283,29 @@ class DecodableList : public detail::DecodableListBase
// data from previous decode attempts will continue to linger and give
// an incorrect view of the state as seen from a client.
//
mValue = T();
mStatus = DataModel::Decode(mReader, mValue);
mValue = T();
this->mStatus = DataModel::Decode(this->mReader, mValue);
}

return (mStatus == CHIP_NO_ERROR);
return (this->mStatus == CHIP_NO_ERROR);
}

T mValue;
CHIP_ERROR mStatus;
TLV::TLVReader mReader;
// TODO: Consider some setup where this field does not exist when T
// is not a fabric scoped struct.
const Optional<FabricIndex> mFabricIndex;
};

Iterator begin() const { return Iterator(mReader, mFabricIndex); }
// Need this->mReader and this->mFabricIndex for the name lookup to realize
// those can be found in our superclasses.
template <typename T0 = T, std::enable_if_t<DataModel::IsFabricScoped<T0>::value, bool> = true>
Iterator begin() const
{
return Iterator(this->mReader, this->mFabricIndex);
}

template <typename T0 = T, std::enable_if_t<!DataModel::IsFabricScoped<T0>::value, bool> = true>
Iterator begin() const
{
return Iterator(this->mReader);
}
};

} // namespace DataModel
Expand Down
9 changes: 4 additions & 5 deletions src/controller/java/OTAProviderDelegateBridge.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,10 @@ void OTAProviderDelegateBridge::HandleQueryImage(CommandHandler * commandObj, co
VendorId vendorId = commandData.vendorID;
uint16_t productId = commandData.productID;
uint32_t softwareVersion = commandData.softwareVersion;
DataModel::DecodableList<OTADownloadProtocol> protocolsSupported = commandData.protocolsSupported;
Optional<uint16_t> hardwareVersion = commandData.hardwareVersion;
Optional<chip::CharSpan> location = commandData.location;
Optional<bool> requestorCanConsent = commandData.requestorCanConsent;
Optional<chip::ByteSpan> metadataForProvider = commandData.metadataForProvider;
Optional<uint16_t> hardwareVersion = commandData.hardwareVersion;
Optional<chip::CharSpan> location = commandData.location;
Optional<bool> requestorCanConsent = commandData.requestorCanConsent;
Optional<chip::ByteSpan> metadataForProvider = commandData.metadataForProvider;

bool isBDXProtocolSupported = false;

Expand Down
1 change: 0 additions & 1 deletion src/controller/tests/TestEventChunking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ void TestReadCallback::OnAttributeData(const app::ConcreteDataAttributePath & aP
{
app::DataModel::DecodableList<ByteSpan> v;
EXPECT_EQ(app::DataModel::Decode(*apData, v), CHIP_NO_ERROR);
auto it = v.begin();
size_t arraySize = 0;
EXPECT_EQ(v.ComputeSize(&arraySize), CHIP_NO_ERROR);
EXPECT_EQ(arraySize, 4u);
Expand Down

0 comments on commit 011b171

Please sign in to comment.