Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use TLV-based encoding for ember data buffer. #36172

Merged
merged 30 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
dbaf112
Use TLV-based encoding for ember data buffer.
andy31415 Oct 17, 2024
7372298
Restyled by clang-format
restyled-commits Oct 21, 2024
8a82ac7
Restyled by shfmt
restyled-commits Oct 21, 2024
bff7f03
Undo unrelated change
andy31415 Oct 21, 2024
a89f016
Add some casts to make android compiler happy
andy31415 Oct 21, 2024
30a3f32
Updates based on code review
andy31415 Oct 21, 2024
f90de00
Added unit tests for min/max int64 values
andy31415 Oct 21, 2024
9048c88
Rename PascalString to PascalStringType
andy31415 Oct 21, 2024
254af50
Fix rename
andy31415 Oct 21, 2024
a702c04
Restyle
andy31415 Oct 21, 2024
e466c11
Add helper methods inside odd sized integers to localize code logic
andreilitvin Oct 22, 2024
5836cea
Restyled by clang-format
restyled-commits Oct 22, 2024
fac403a
Fix up negative ranges
andreilitvin Oct 22, 2024
a8b76f6
Fixed ranges
andreilitvin Oct 22, 2024
717348c
Fix signed max bug and update unit tests
andreilitvin Oct 22, 2024
d6073eb
Merge branch 'master' into ember_tlv_reader_encoding
andreilitvin Oct 22, 2024
8c91bb0
Make android happy
andreilitvin Oct 22, 2024
e8b5cbe
Typo fix
andreilitvin Oct 22, 2024
ecdca36
Switch up unit tests
andy31415 Oct 23, 2024
e0811b7
Merge branch 'master' into ember_tlv_reader_encoding
andy31415 Oct 23, 2024
f80cf68
Update a nullable check to make use of ValueToNullValue
andy31415 Oct 23, 2024
cee5602
Add namespace prefix
andy31415 Oct 23, 2024
4738e1e
Update src/app/codegen-data-model-provider/EmberDataBuffer.cpp
andy31415 Oct 24, 2024
c719787
Update src/app/codegen-data-model-provider/EmberDataBuffer.h
andy31415 Oct 24, 2024
23095c3
Correct comments:signed, not unsigned
andreilitvin Oct 24, 2024
972643b
Use constructors for the buffer info
andreilitvin Oct 24, 2024
3c87c45
Rename things to EmberAttributeDataBuffer
andreilitvin Oct 24, 2024
4e05e99
Merge branch 'master' into ember_tlv_reader_encoding
andreilitvin Oct 24, 2024
a2558cf
Undo submodule updates
andreilitvin Oct 24, 2024
efe8b28
Restyled by clang-format
restyled-commits Oct 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions scripts/build_coverage.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ SUPPORTED_TESTS=(unit yaml all)
CODE="core"
TESTS="unit"
skip_gn=false
TEST_TARGET=check

help() {

Expand All @@ -70,6 +71,7 @@ help() {
'unit': Run unit test to drive the coverage check. --default
'yaml': Run yaml test to drive the coverage check.
'all': Run unit & yaml test to drive the coverage check.
--target Specific test target to run (e.g. TestEmberAttributeBuffer.run)
"
}

Expand All @@ -89,6 +91,9 @@ for i in "$@"; do
TESTS="${i#*=}"
shift
;;
--target=*)
TEST_TARGET="${i#*=}"
;;
-o=* | --output_root=*)
OUTPUT_ROOT="${i#*=}"
COVERAGE_ROOT="$OUTPUT_ROOT/coverage"
Expand Down Expand Up @@ -121,20 +126,21 @@ if [ "$skip_gn" == false ]; then
# Generates ninja files
EXTRA_GN_ARGS=""
if [[ "$TESTS" == "yaml" || "$TESTS" == "all" ]]; then
EXTRA_GN_ARGS="$EXTRA_GN_ARGS chip_build_all_clusters_app=true"
EXTRA_GN_ARGS="$EXTRA_GN_ARGS chip_build_all_clusters_app=true"
else
EXTRA_GN_ARGS="$EXTRA_GN_ARGS chip_build_tools=false"
fi
gn --root="$CHIP_ROOT" gen "$OUTPUT_ROOT" --args="use_coverage=true$EXTRA_GN_ARGS"
ninja -C "$OUTPUT_ROOT"

# Run unit tests
if [[ "$TESTS" == "unit" || "$TESTS" == "all" ]]; then
ninja -C "$OUTPUT_ROOT" check
ninja -C "$OUTPUT_ROOT" "$TEST_TARGET"
fi

# Run yaml tests
if [[ "$TESTS" == "yaml" || "$TESTS" == "all" ]]; then
ninja -C "$OUTPUT_ROOT"

scripts/run_in_build_env.sh \
"./scripts/tests/run_test_suite.py \
--chip-tool ""$OUTPUT_ROOT/chip-tool \
Expand Down
2 changes: 2 additions & 0 deletions src/app/codegen-data-model-provider/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import("//build_overrides/chip.gni")
# CodegenDataModelProvider.h
# CodegenDataModelProvider_Read.cpp
# CodegenDataModelProvider_Write.cpp
# EmberDataBuffer.cpp
# EmberDataBuffer.h
# EmberMetadata.cpp
# EmberMetadata.h
# Instance.cpp
Expand Down
198 changes: 5 additions & 193 deletions src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <app/AttributeAccessInterface.h>
#include <app/AttributeAccessInterfaceRegistry.h>
#include <app/RequiredPrivilege.h>
#include <app/codegen-data-model-provider/EmberDataBuffer.h>
#include <app/codegen-data-model-provider/EmberMetadata.h>
#include <app/data-model/FabricScoped.h>
#include <app/reporting/reporting.h>
Expand Down Expand Up @@ -85,198 +86,6 @@ std::optional<CHIP_ERROR> TryWriteViaAccessInterface(const ConcreteDataAttribute
return decoder.TriedDecode() ? std::make_optional(CHIP_NO_ERROR) : std::nullopt;
}

/// Metadata of what a ember/pascal short string means (prepended by a u8 length)
struct ShortPascalString
{
using LengthType = uint8_t;
static constexpr LengthType kNullLength = 0xFF;

static void SetLength(uint8_t * buffer, LengthType value) { *buffer = value; }
};

/// Metadata of what a ember/pascal LONG string means (prepended by a u16 length)
struct LongPascalString
{
using LengthType = uint16_t;
static constexpr LengthType kNullLength = 0xFFFF;

// Encoding for ember string lengths is little-endian (see ember-strings.cpp)
static void SetLength(uint8_t * buffer, LengthType value) { Encoding::LittleEndian::Put16(buffer, value); }
};

// ember assumptions ... should just work
static_assert(sizeof(ShortPascalString::LengthType) == 1);
static_assert(sizeof(LongPascalString::LengthType) == 2);

/// Convert the value stored in 'decoder' into an ember format span 'out'
///
/// The value converted will be of type T (e.g. CharSpan or ByteSpan) and it will be converted
/// via the given ENCODING (i.e. ShortPascalString or LongPascalString)
///
/// isNullable defines if the value of NULL is allowed to be converted.
template <typename T, class ENCODING>
CHIP_ERROR DecodeStringLikeIntoEmberBuffer(AttributeValueDecoder decoder, bool isNullable, MutableByteSpan & out)
{
T workingValue;

if (isNullable)
{
typename DataModel::Nullable<T> nullableWorkingValue;
ReturnErrorOnFailure(decoder.Decode(nullableWorkingValue));

if (nullableWorkingValue.IsNull())
{
VerifyOrReturnError(out.size() >= sizeof(typename ENCODING::LengthType), CHIP_ERROR_BUFFER_TOO_SMALL);
ENCODING::SetLength(out.data(), ENCODING::kNullLength);
out.reduce_size(sizeof(typename ENCODING::LengthType));
return CHIP_NO_ERROR;
}

// continue encoding non-null value
workingValue = nullableWorkingValue.Value();
}
else
{
ReturnErrorOnFailure(decoder.Decode(workingValue));
}

auto len = static_cast<typename ENCODING::LengthType>(workingValue.size());
VerifyOrReturnError(out.size() >= sizeof(len) + len, CHIP_ERROR_BUFFER_TOO_SMALL);

uint8_t * output_buffer = out.data();

ENCODING::SetLength(output_buffer, len);
output_buffer += sizeof(len);

memcpy(output_buffer, workingValue.data(), workingValue.size());
output_buffer += workingValue.size();

out.reduce_size(static_cast<size_t>(output_buffer - out.data()));
return CHIP_NO_ERROR;
}

/// Decodes a numeric data value of type T from the `decoder` into a ember-encoded buffer `out`
///
/// isNullable defines if the value of NULL is allowed to be decoded.
template <typename T>
CHIP_ERROR DecodeIntoEmberBuffer(AttributeValueDecoder & decoder, bool isNullable, MutableByteSpan & out)
{
using Traits = NumericAttributeTraits<T>;
typename Traits::StorageType storageValue;

if (isNullable)
{
DataModel::Nullable<typename Traits::WorkingType> workingValue;
ReturnErrorOnFailure(decoder.Decode(workingValue));

if (workingValue.IsNull())
{
Traits::SetNull(storageValue);
}
else
{
// This guards against trying to decode something that overlaps nullable, for example
// Nullable<uint8_t>(0xFF) is not representable because 0xFF is the encoding of NULL in ember
// as well as odd-sized integers (e.g. full 32-bit value like 0x11223344 cannot be written
// to a 3-byte odd-sized integger).
VerifyOrReturnError(Traits::CanRepresentValue(isNullable, workingValue.Value()), CHIP_ERROR_INVALID_ARGUMENT);
Traits::WorkingToStorage(workingValue.Value(), storageValue);
}

VerifyOrReturnError(out.size() >= sizeof(storageValue), CHIP_ERROR_INVALID_ARGUMENT);
}
else
{
typename Traits::WorkingType workingValue;
ReturnErrorOnFailure(decoder.Decode(workingValue));

Traits::WorkingToStorage(workingValue, storageValue);

VerifyOrReturnError(out.size() >= sizeof(storageValue), CHIP_ERROR_INVALID_ARGUMENT);

// Even non-nullable values may be outside range: e.g. odd-sized integers have working values
// that are larger than the storage values (e.g. a uint32_t being stored as a 3-byte integer)
VerifyOrReturnError(Traits::CanRepresentValue(isNullable, workingValue), CHIP_ERROR_INVALID_ARGUMENT);
}

const uint8_t * data = Traits::ToAttributeStoreRepresentation(storageValue);

// The decoding + ToAttributeStoreRepresentation will result in data being
// stored in native format/byteorder, suitable to directly be stored in the data store
memcpy(out.data(), data, sizeof(storageValue));
out.reduce_size(sizeof(storageValue));

return CHIP_NO_ERROR;
}

/// Read the data from "decoder" into an ember-formatted buffer "out"
///
/// `out` is a in/out buffer:
/// - its initial size determines the maximum size of the buffer
/// - its output size reflects the actual data size
///
/// Uses the attribute `metadata` to determine how the data is to be encoded into out.
CHIP_ERROR DecodeValueIntoEmberBuffer(AttributeValueDecoder & decoder, const EmberAfAttributeMetadata * metadata,
MutableByteSpan & out)
{
VerifyOrReturnError(metadata != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

const bool isNullable = metadata->IsNullable();

switch (AttributeBaseType(metadata->attributeType))
{
case ZCL_BOOLEAN_ATTRIBUTE_TYPE: // Boolean
return DecodeIntoEmberBuffer<bool>(decoder, isNullable, out);
case ZCL_INT8U_ATTRIBUTE_TYPE: // Unsigned 8-bit integer
return DecodeIntoEmberBuffer<uint8_t>(decoder, isNullable, out);
case ZCL_INT16U_ATTRIBUTE_TYPE: // Unsigned 16-bit integer
return DecodeIntoEmberBuffer<uint16_t>(decoder, isNullable, out);
case ZCL_INT24U_ATTRIBUTE_TYPE: // Unsigned 24-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<3, false>>(decoder, isNullable, out);
case ZCL_INT32U_ATTRIBUTE_TYPE: // Unsigned 32-bit integer
return DecodeIntoEmberBuffer<uint32_t>(decoder, isNullable, out);
case ZCL_INT40U_ATTRIBUTE_TYPE: // Unsigned 40-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<5, false>>(decoder, isNullable, out);
case ZCL_INT48U_ATTRIBUTE_TYPE: // Unsigned 48-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<6, false>>(decoder, isNullable, out);
case ZCL_INT56U_ATTRIBUTE_TYPE: // Unsigned 56-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<7, false>>(decoder, isNullable, out);
case ZCL_INT64U_ATTRIBUTE_TYPE: // Unsigned 64-bit integer
return DecodeIntoEmberBuffer<uint64_t>(decoder, isNullable, out);
case ZCL_INT8S_ATTRIBUTE_TYPE: // Signed 8-bit integer
return DecodeIntoEmberBuffer<int8_t>(decoder, isNullable, out);
case ZCL_INT16S_ATTRIBUTE_TYPE: // Signed 16-bit integer
return DecodeIntoEmberBuffer<int16_t>(decoder, isNullable, out);
case ZCL_INT24S_ATTRIBUTE_TYPE: // Signed 24-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<3, true>>(decoder, isNullable, out);
case ZCL_INT32S_ATTRIBUTE_TYPE: // Signed 32-bit integer
return DecodeIntoEmberBuffer<int32_t>(decoder, isNullable, out);
case ZCL_INT40S_ATTRIBUTE_TYPE: // Signed 40-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<5, true>>(decoder, isNullable, out);
case ZCL_INT48S_ATTRIBUTE_TYPE: // Signed 48-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<6, true>>(decoder, isNullable, out);
case ZCL_INT56S_ATTRIBUTE_TYPE: // Signed 56-bit integer
return DecodeIntoEmberBuffer<OddSizedInteger<7, true>>(decoder, isNullable, out);
case ZCL_INT64S_ATTRIBUTE_TYPE: // Signed 64-bit integer
return DecodeIntoEmberBuffer<int64_t>(decoder, isNullable, out);
case ZCL_SINGLE_ATTRIBUTE_TYPE: // 32-bit float
return DecodeIntoEmberBuffer<float>(decoder, isNullable, out);
case ZCL_DOUBLE_ATTRIBUTE_TYPE: // 64-bit float
return DecodeIntoEmberBuffer<double>(decoder, isNullable, out);
case ZCL_CHAR_STRING_ATTRIBUTE_TYPE: // Char string
return DecodeStringLikeIntoEmberBuffer<CharSpan, ShortPascalString>(decoder, isNullable, out);
case ZCL_LONG_CHAR_STRING_ATTRIBUTE_TYPE:
return DecodeStringLikeIntoEmberBuffer<CharSpan, LongPascalString>(decoder, isNullable, out);
case ZCL_OCTET_STRING_ATTRIBUTE_TYPE: // Octet string
return DecodeStringLikeIntoEmberBuffer<ByteSpan, ShortPascalString>(decoder, isNullable, out);
case ZCL_LONG_OCTET_STRING_ATTRIBUTE_TYPE:
return DecodeStringLikeIntoEmberBuffer<ByteSpan, LongPascalString>(decoder, isNullable, out);
default:
ChipLogError(DataManagement, "Attribute type 0x%x not handled", static_cast<int>(metadata->attributeType));
return CHIP_IM_GLOBAL_STATUS(Failure);
}
}

} // namespace

DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const DataModel::WriteAttributeRequest & request,
Expand Down Expand Up @@ -414,7 +223,10 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat
}

MutableByteSpan dataBuffer = gEmberAttributeIOBufferSpan;
ReturnErrorOnFailure(DecodeValueIntoEmberBuffer(decoder, *attributeMetadata, dataBuffer));
{
Ember::EmberAttributeBuffer emberData(*attributeMetadata, dataBuffer);
ReturnErrorOnFailure(decoder.Decode(emberData));
}

Protocols::InteractionModel::Status status;

Expand Down
Loading
Loading