Skip to content

Commit

Permalink
Look for more flash space savings in cluster-objects.cpp (#29561)
Browse files Browse the repository at this point in the history 
* Updates to cluster objects: use direct context tag comparisons

* Also optimize code size usage for structure decoding

* Completely split struct iterations and use std::variant based returns on an iterator

* Another space save: using a common err for all decodes and single macro invocation seems to make code even smaller

* One more cleanup return and proper use of first/last to remove unused code on empty structs (mainly command inputs)

* Added a comment and a missed auto keyword

* Missed some first/last iterations
  • Loading branch information
@andy31415 @pull
andy31415 authored and pull[bot] committed Oct 17, 2023
1 parent 91eb864 commit 1166835
Show file tree
Hide file tree
Showing 3 changed files with 10,973 additions and 13,399 deletions.
43 changes: 22 additions & 21 deletions src/app/zap-templates/partials/cluster-objects-struct.zapt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,32 +113,33 @@ CHIP_ERROR Type::Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const
{{/if}}

CHIP_ERROR DecodableType::Decode(TLV::TLVReader &reader) {
CHIP_ERROR err = CHIP_NO_ERROR;
TLV::TLVType outer;
VerifyOrReturnError(TLV::kTLVType_Structure == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
err = reader.EnterContainer(outer);
ReturnErrorOnFailure(err);
while ((err = reader.Next()) == CHIP_NO_ERROR) {
if (!TLV::IsContextTag(reader.GetTag()))
detail::StructDecodeIterator __iterator(reader);
while (true) {
auto __element = __iterator.Next();
if (std::holds_alternative<CHIP_ERROR>(__element)) {
return std::get<CHIP_ERROR>(__element);
}

{{#zcl_struct_items}}
{{#first}}
CHIP_ERROR err = CHIP_NO_ERROR;
const uint8_t __context_tag = std::get<uint8_t>(__element);

{{/first~}}
{{! NOTE: using if/else instead of switch because it seems to generate smaller code. ~}}
if (__context_tag == to_underlying(Fields::k{{asUpperCamelCase label}}))
{
continue;
err = DataModel::Decode(reader, {{asLowerCamelCase label}});
}
switch (TLV::TagNumFromTag(reader.GetTag()))
else
{{#last}}
{
{{#zcl_struct_items}}
case to_underlying(Fields::k{{asUpperCamelCase label}}):
ReturnErrorOnFailure(DataModel::Decode(reader, {{asLowerCamelCase label}}));
break;
{{/zcl_struct_items}}
default:
break;
}
}

VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
ReturnErrorOnFailure(reader.ExitContainer(outer));

return CHIP_NO_ERROR;
ReturnErrorOnFailure(err);
{{/last}}
{{/zcl_struct_items}}
}
}

} // namespace {{asUpperCamelCase name}}
Expand Down
157 changes: 85 additions & 72 deletions src/app/zap-templates/templates/app/cluster-objects-src.zapt
View file
Original file line number Diff line number Diff line change
@@ -1,46 +1,56 @@
{{> header}}

#include <app-common/zap-generated/cluster-objects.h>
#include <variant>

namespace chip {
namespace app {
namespace Clusters {

namespace detail {

CHIP_ERROR FlightCheckDecodeAndEnterStruct(TLV::TLVReader & reader, TLV::TLVType & outer)
{
TLV::TLVReader temp_reader;

// Make a copy of the struct reader to do pre-checks.
temp_reader.Init(reader);
class StructDecodeIterator {
public:
// may return a context tag, a CHIP_ERROR (end iteration)
using EntryElement = std::variant<uint8_t, CHIP_ERROR>;

StructDecodeIterator(TLV::TLVReader &reader) : mReader(reader){}

// Iterate through structure elements. Returns one of:
// - uint8_t CONTEXT TAG (keep iterating)
// - CHIP_ERROR (including CHIP_NO_ERROR) which should be a final
// return value (stop iterating)
EntryElement Next() {
if (!mEntered) {
VerifyOrReturnError(TLV::kTLVType_Structure == mReader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
ReturnErrorOnFailure(mReader.EnterContainer(mOuter));
mEntered = true;
}

while (true) {
CHIP_ERROR err = mReader.Next();
if (err != CHIP_NO_ERROR) {
VerifyOrReturnError(err == CHIP_ERROR_END_OF_TLV, err);
break;
}

// Ensure we have a single struct and that it's properly bounded.
CHIP_ERROR err = CHIP_NO_ERROR;
VerifyOrReturnError(TLV::kTLVType_Structure == temp_reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
ReturnErrorOnFailure(temp_reader.EnterContainer(outer));
while ((err = temp_reader.Next()) == CHIP_NO_ERROR)
{
if (!TLV::IsContextTag(temp_reader.GetTag()))
{
const TLV::Tag tag = mReader.GetTag();
if (!TLV::IsContextTag(tag)) {
continue;
}
}
VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
ReturnErrorOnFailure(temp_reader.ExitContainer(outer));
}

// Guaranteed to work due to prior checks.
VerifyOrDie(reader.EnterContainer(outer) == CHIP_NO_ERROR);
return CHIP_NO_ERROR;
}
// we know context tags are 8-bit
return static_cast<uint8_t>(TLV::TagNumFromTag(tag));
}

void ExitStructAfterDecode(TLV::TLVReader & reader, TLV::TLVType & outer)
{
// Ensure we exit the container. Will be OK since FlightCheckDecodeAndEnterStruct will have
// already been called, and generated code properly iterates over entire container.
VerifyOrDie(reader.Next() == CHIP_END_OF_TLV);
VerifyOrDie(reader.ExitContainer(outer) == CHIP_NO_ERROR);
}
return mReader.ExitContainer(mOuter);
}

private:
bool mEntered = false;
TLV::TLVType mOuter;
TLV::TLVReader &mReader;
};

// Structs shared across multiple clusters.
namespace Structs {
Expand Down Expand Up @@ -75,34 +85,37 @@ CHIP_ERROR Type::Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const{
{{#zcl_command_arguments}}
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}}));
{{/zcl_command_arguments}}
ReturnErrorOnFailure(aWriter.EndContainer(outer));
return CHIP_NO_ERROR;
return aWriter.EndContainer(outer);
}

CHIP_ERROR DecodableType::Decode(TLV::TLVReader &reader) {
TLV::TLVType outer;
ReturnErrorOnFailure(chip::app::Clusters::detail::FlightCheckDecodeAndEnterStruct(reader, outer));
detail::StructDecodeIterator __iterator(reader);
while (true) {
auto __element = __iterator.Next();
if (std::holds_alternative<CHIP_ERROR>(__element)) {
return std::get<CHIP_ERROR>(__element);
}

{{#zcl_command_arguments~}}
{{#first}}
CHIP_ERROR err = CHIP_NO_ERROR;
const uint8_t __context_tag = std::get<uint8_t>(__element);

CHIP_ERROR err = CHIP_NO_ERROR;
while ((err = reader.Next()) == CHIP_NO_ERROR) {
if (!TLV::IsContextTag(reader.GetTag()))
{{/first~}}
{{! NOTE: using if/else instead of switch because it seems to generate smaller code. ~}}
if (__context_tag == to_underlying(Fields::k{{asUpperCamelCase label}}))
{
continue;
err = DataModel::Decode(reader, {{asLowerCamelCase label}});
}
switch (TLV::TagNumFromTag(reader.GetTag()))
else
{{#last}}
{
{{#zcl_command_arguments}}
case to_underlying(Fields::k{{asUpperCamelCase label}}):
ReturnErrorOnFailure(DataModel::Decode(reader, {{asLowerCamelCase label}}));
break;
{{/zcl_command_arguments}}
default:
break;
}
}

chip::app::Clusters::detail::ExitStructAfterDecode(reader, outer);
return CHIP_NO_ERROR;
ReturnErrorOnFailure(err);
{{/last}}
{{/zcl_command_arguments}}
}
}
} // namespace {{asUpperCamelCase name}}.
{{/zcl_commands}}
Expand All @@ -114,14 +127,11 @@ CHIP_ERROR TypeInfo::DecodableType::Decode(TLV::TLVReader &reader, const Concret
{
{{#zcl_attributes_server}}
case Attributes::{{asUpperCamelCase label}}::TypeInfo::GetAttributeId():
ReturnErrorOnFailure(DataModel::Decode(reader, {{asLowerCamelCase label}}));
break;
return DataModel::Decode(reader, {{asLowerCamelCase label}});
{{/zcl_attributes_server}}
default:
break;
return CHIP_NO_ERROR;
}

return CHIP_NO_ERROR;
}
} // namespace Attributes

Expand All @@ -138,34 +148,37 @@ CHIP_ERROR Type::Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const{
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase name}}), {{asLowerCamelCase name}}));
{{/if_is_fabric_scoped_struct}}
{{/zcl_event_fields}}
ReturnErrorOnFailure(aWriter.EndContainer(outer));
return CHIP_NO_ERROR;
return aWriter.EndContainer(outer);
}

CHIP_ERROR DecodableType::Decode(TLV::TLVReader &reader) {
TLV::TLVType outer;
ReturnErrorOnFailure(chip::app::Clusters::detail::FlightCheckDecodeAndEnterStruct(reader, outer));
detail::StructDecodeIterator __iterator(reader);
while (true) {
auto __element = __iterator.Next();
if (std::holds_alternative<CHIP_ERROR>(__element)) {
return std::get<CHIP_ERROR>(__element);
}

{{#zcl_event_fields}}
{{#first}}
CHIP_ERROR err = CHIP_NO_ERROR;
const uint8_t __context_tag = std::get<uint8_t>(__element);

CHIP_ERROR err = CHIP_NO_ERROR;
while ((err = reader.Next()) == CHIP_NO_ERROR) {
if (!TLV::IsContextTag(reader.GetTag()))
{{/first~}}
{{! NOTE: using if/else instead of switch because it seems to generate smaller code. ~}}
if (__context_tag == to_underlying(Fields::k{{asUpperCamelCase name}}))
{
continue;
err = DataModel::Decode(reader, {{asLowerCamelCase name}});
}
switch (TLV::TagNumFromTag(reader.GetTag()))
else
{{#last}}
{
{{#zcl_event_fields}}
case to_underlying(Fields::k{{asUpperCamelCase name}}):
ReturnErrorOnFailure(DataModel::Decode(reader, {{asLowerCamelCase name}}));
break;
{{/zcl_event_fields}}
default:
break;
}
}

chip::app::Clusters::detail::ExitStructAfterDecode(reader, outer);
return CHIP_NO_ERROR;
ReturnErrorOnFailure(err);
{{/last}}
{{/zcl_event_fields}}
}
}
} // namespace {{asUpperCamelCase name}}.
{{/zcl_events}}
Expand Down
Loading

0 comments on commit 1166835

Please sign in to comment.