Skip to content

Commit

Permalink
Stop requiring a fixed-size up-front allocation for our attribute value.
Browse files Browse the repository at this point in the history
  • Loading branch information
@bzbarsky-apple
bzbarsky-apple committed May 3, 2023
1 parent 88f187b commit d393274
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 48 deletions.
89 changes: 61 additions & 28 deletions src/darwin/Framework/CHIP/MTRBaseDevice.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2266,7 +2266,7 @@ static void LogStringAndReturnError(NSString * errorStr, MTRErrorCode errorCode,

static void LogStringAndReturnError(NSString * errorStr, CHIP_ERROR errorCode, NSError * __autoreleasing * error)
{
MTR_LOG_ERROR("%s", errorStr.UTF8String);
MTR_LOG_ERROR("%s: %s", errorStr.UTF8String, errorCode.AsString());
if (!error) {
return;
}
Expand All @@ -2292,30 +2292,72 @@ static bool CheckMemberOfType(NSDictionary<NSString *, id> * responseValue, NSSt
return true;
}

// Allocates a buffer, encodes the data-value as TLV, and points the TLV::Reader to
// the data. Returns false if any of that fails, in which case error gets set.
static bool EncodeDataValueToTLV(Platform::ScopedMemoryBuffer<uint8_t> & buffer, size_t bufferSize, NSDictionary * data,
TLV::TLVReader & reader, NSError * __autoreleasing * error)
{
if (!buffer.Calloc(bufferSize)) {
LogStringAndReturnError(@"Unable to allocate encoding buffer.", CHIP_ERROR_NO_MEMORY, error);
// Allocates a buffer, encodes the data-value as TLV, and points the TLV::Reader
// to the data. Returns false if any of that fails, in which case error gets
// set.
//
// Data model decoding requires a contiguous buffer (because lists walk all the
// data multiple times and TLVPacketBufferBackingStore doesn't have a way to
// checkpoint and restore its state), but we can encode into chained packet
// buffers and then decide whether we need a contiguous realloc.
static bool EncodeDataValueToTLV(System::PacketBufferHandle & buffer, Platform::ScopedMemoryBuffer<uint8_t> & flatBuffer,
NSDictionary * data, TLV::TLVReader & reader, NSError * __autoreleasing * error)
{
buffer = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSizeWithoutReserve, 0);
if (buffer.IsNull()) {
LogStringAndReturnError(@"Unable to allocate encoding buffer", CHIP_ERROR_NO_MEMORY, error);
return false;
}

TLV::TLVWriter writer;
writer.Init(buffer.Get(), bufferSize);
System::PacketBufferTLVWriter writer;
writer.Init(std::move(buffer), /* useChainedBuffers = */ true);

CHIP_ERROR errorCode = MTREncodeTLVFromDataValueDictionary(data, writer, TLV::AnonymousTag());
if (errorCode != CHIP_NO_ERROR) {
LogStringAndReturnError(@"Unable to encode data-value to TLV.", errorCode, error);
LogStringAndReturnError(@"Unable to encode data-value to TLV", errorCode, error);
return false;
}

errorCode = writer.Finalize(&buffer);
if (errorCode != CHIP_NO_ERROR) {
LogStringAndReturnError(@"Unable to encode data-value to TLV", errorCode, error);
return false;
}

reader.Init(buffer.Get(), writer.GetLengthWritten());
if (buffer->HasChainedBuffer()) {
// We need to reallocate into a single contiguous buffer.
size_t remainingData = buffer->TotalLength();
if (!flatBuffer.Calloc(remainingData)) {
LogStringAndReturnError(@"Unable to allocate decoding buffer", CHIP_ERROR_NO_MEMORY, error);
return false;
}
size_t copiedData = 0;
while (!buffer.IsNull()) {
if (buffer->DataLength() > remainingData) {
// Should never happen, but let's be extra careful about buffer
// overruns.
LogStringAndReturnError(@"Encoding buffer size is bigger than it claimed", CHIP_ERROR_INCORRECT_STATE, error);
return false;
}

memcpy(flatBuffer.Get() + copiedData, buffer->Start(), buffer->DataLength());
copiedData += buffer->DataLength();
remainingData -= buffer->DataLength();
buffer.Advance();
}
if (remainingData != 0) {
LogStringAndReturnError(
@"Did not copy all data from Encoding buffer for some reason", CHIP_ERROR_INCORRECT_STATE, error);
return false;
}
reader.Init(flatBuffer.Get(), copiedData);
} else {
reader.Init(buffer->Start(), buffer->DataLength());
}

errorCode = reader.Next(TLV::AnonymousTag());
if (errorCode != CHIP_NO_ERROR) {
LogStringAndReturnError(@"data-value TLV encoding did not create a TLV element.", errorCode, error);
LogStringAndReturnError(@"data-value TLV encoding did not create a TLV element", errorCode, error);
return false;
}

Expand Down Expand Up @@ -2374,18 +2416,10 @@ - (nullable instancetype)initWithResponseValue:(NSDictionary<NSString *, id> *)r
NSDictionary * data = responseValue[MTRDataKey];

// Encode the data to TLV and then decode from that, to reuse existing code.
// We don't know exactly how much data we can have here; if our value is a list it
// might well be larger than the amount of data that would fit in a single packet.
//
// We could start with some small buffer size and try growing it until it's
// big enough to encode into, but in practice we'd probably have to cap that
// at some max size anyway. So just start with something that is likely to
// work for any conceivable attribute, like 20KiB.

constexpr size_t bufferSize = 20 * 1042;
Platform::ScopedMemoryBuffer<uint8_t> buffer;
System::PacketBufferHandle buffer;
Platform::ScopedMemoryBuffer<uint8_t> flatBuffer;
TLV::TLVReader reader;
if (!EncodeDataValueToTLV(buffer, bufferSize, data, reader, error)) {
if (!EncodeDataValueToTLV(buffer, flatBuffer, data, reader, error)) {
return nil;
}

Expand Down Expand Up @@ -2507,11 +2541,10 @@ - (nullable instancetype)initWithResponseValue:(NSDictionary<NSString *, id> *)r
NSDictionary * data = responseValue[MTRDataKey];

// Encode the data to TLV and then decode from that, to reuse existing code.
// For an event, we know it always fits in a single Matter message.
constexpr size_t bufferSize = kMaxSecureSduLengthBytes;
Platform::ScopedMemoryBuffer<uint8_t> buffer;
System::PacketBufferHandle buffer;
Platform::ScopedMemoryBuffer<uint8_t> flatBuffer;
TLV::TLVReader reader;
if (!EncodeDataValueToTLV(buffer, bufferSize, data, reader, error)) {
if (!EncodeDataValueToTLV(buffer, flatBuffer, data, reader, error)) {
return nil;
}
auto eventPath = [path _asConcretePath];
Expand Down
98 changes: 79 additions & 19 deletions src/darwin/Framework/CHIPTests/MTRDataValueParserTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ - (void)test009_NullableDoubleAttribute
XCTAssertNil(report.error);
}

- (void)test0010_StructAttribute
- (void)test010_StructAttribute
{
// Basic Information, CapabilityMinima
NSDictionary * input = @{
Expand Down Expand Up @@ -260,7 +260,7 @@ - (void)test0010_StructAttribute
XCTAssertNil(report.error);
}

- (void)test0011_StructAttributeOtherOrder
- (void)test011_StructAttributeOtherOrder
{
// Basic Information, CapabilityMinima
NSDictionary * input = @{
Expand Down Expand Up @@ -302,7 +302,7 @@ - (void)test0011_StructAttributeOtherOrder
XCTAssertNil(report.error);
}

- (void)test0012_ListAttribute
- (void)test012_ListAttribute
{
// Descriptor, DeviceTypeList
NSDictionary * input = @{
Expand Down Expand Up @@ -379,7 +379,7 @@ - (void)test0012_ListAttribute
XCTAssertNil(report.error);
}

- (void)test0013_UnsignedIntAttributeSignMismatch
- (void)test013_UnsignedIntAttributeSignMismatch
{
// Pressure Measurement, Tolerance
NSDictionary * input = @{
Expand All @@ -397,7 +397,7 @@ - (void)test0013_UnsignedIntAttributeSignMismatch
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0014_SignedIntAttributeSignMismatch
- (void)test014_SignedIntAttributeSignMismatch
{
// Pressure Measurement, MeasuredValue
NSDictionary * input = @{
Expand All @@ -415,7 +415,7 @@ - (void)test0014_SignedIntAttributeSignMismatch
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0015_UnknownAttribute
- (void)test015_UnknownAttribute
{
// On/Off, nonexistent attribute
NSDictionary * input = @{
Expand All @@ -433,7 +433,7 @@ - (void)test0015_UnknownAttribute
XCTAssertEqual(error.code, MTRErrorCodeUnknownSchema);
}

- (void)test0016_UnknownCluster
- (void)test016_UnknownCluster
{
// Unknown cluster.
NSDictionary * input = @{
Expand All @@ -451,7 +451,7 @@ - (void)test0016_UnknownCluster
XCTAssertEqual(error.code, MTRErrorCodeUnknownSchema);
}

- (void)test0017_StringVsOctetStringMismatch
- (void)test017_StringVsOctetStringMismatch
{
// Basic Information, SerialNumber
NSDictionary * input = @{
Expand All @@ -469,7 +469,7 @@ - (void)test0017_StringVsOctetStringMismatch
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0018_OctetStringVsStringMismatch
- (void)test018_OctetStringVsStringMismatch
{
// Thread Network Diagnostics, ChannelPage0Mask
NSDictionary * input = @{
Expand All @@ -487,7 +487,7 @@ - (void)test0018_OctetStringVsStringMismatch
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0019_DoubleVsFloatMismatch
- (void)test019_DoubleVsFloatMismatch
{
// Unit Testing, float_double
NSDictionary * input = @{
Expand All @@ -510,7 +510,7 @@ - (void)test0019_DoubleVsFloatMismatch
XCTAssertNil(report.error);
}

- (void)test0020_FloatVsDoubleMismatch
- (void)test020_FloatVsDoubleMismatch
{
// Media Playback, PlaybackSpeed
NSDictionary * input = @{
Expand Down Expand Up @@ -579,7 +579,7 @@ - (void)test023_DoubleVsNullMismatch
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0024_StructFieldIntegerTypeMismatch
- (void)test024_StructFieldIntegerTypeMismatch
{
// Descriptor, DeviceTypeList
NSDictionary * input = @{
Expand Down Expand Up @@ -640,7 +640,7 @@ - (void)test0024_StructFieldIntegerTypeMismatch
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0025_EventPayloadWithSystemUptime
- (void)test025_EventPayloadWithSystemUptime
{
// Access Control, AccessControlExtensionChanged
NSDictionary * input = @{
Expand Down Expand Up @@ -722,7 +722,7 @@ - (void)test0025_EventPayloadWithSystemUptime
XCTAssertNil(report.error);
}

- (void)test0026_EventReportWithTimestampDate
- (void)test026_EventReportWithTimestampDate
{
// Basic Information, Shutdown
NSDictionary * input = @{
Expand Down Expand Up @@ -756,7 +756,7 @@ - (void)test0026_EventReportWithTimestampDate
XCTAssertTrue([report.value isKindOfClass:[MTRBasicInformationClusterShutDownEvent class]]);
}

- (void)test0027_AttributeWithDataAndError
- (void)test027_AttributeWithDataAndError
{
// Pressure Measurement, Tolerance
NSDictionary * input = @{
Expand All @@ -779,7 +779,7 @@ - (void)test0027_AttributeWithDataAndError
XCTAssertEqualObjects(report.error, input[MTRErrorKey]);
}

- (void)test0028_EventReportWithDataAndError
- (void)test028_EventReportWithDataAndError
{
// Basic Information, Shutdown
NSDictionary * input = @{
Expand Down Expand Up @@ -813,7 +813,7 @@ - (void)test0028_EventReportWithDataAndError
XCTAssertEqualObjects(report.error, input[MTRErrorKey]);
}

- (void)test0029_EventPayloadFailingSchemaCheck
- (void)test029_EventPayloadFailingSchemaCheck
{
// Access Control, AccessControlExtensionChanged
NSDictionary * input = @{
Expand Down Expand Up @@ -871,7 +871,7 @@ - (void)test0029_EventPayloadFailingSchemaCheck
XCTAssertEqual(error.code, MTRErrorCodeSchemaMismatch);
}

- (void)test0030_EventReportWithUnknownCluster
- (void)test030_EventReportWithUnknownCluster
{
NSDictionary * input = @{
MTREventPathKey : [MTREventPath eventPathWithEndpointID:@(0) clusterID:@(0xFF1FF1) eventID:@(0)],
Expand All @@ -892,7 +892,7 @@ - (void)test0030_EventReportWithUnknownCluster
XCTAssertEqual(error.code, MTRErrorCodeUnknownSchema);
}

- (void)test0031_EventReportWithUnknownEvent
- (void)test031_EventReportWithUnknownEvent
{
NSDictionary * input = @{
MTREventPathKey : [MTREventPath eventPathWithEndpointID:@(0) clusterID:@(0x0028) eventID:@(1000)],
Expand All @@ -913,4 +913,64 @@ - (void)test0031_EventReportWithUnknownEvent
XCTAssertEqual(error.code, MTRErrorCodeUnknownSchema);
}

- (void)test032_VeryLongListAttribute
{
NSDictionary * singleListItem = @{
MTRDataKey : @ {
MTRTypeKey : MTRStructureValueType,
MTRValueKey : @[
@{
MTRContextTagKey : @(0), // DeviceType
MTRDataKey : @ {
MTRTypeKey : MTRUnsignedIntegerValueType,
MTRValueKey : @(100),
},
},
@{
MTRContextTagKey : @(1), // Revision
MTRDataKey : @ {
MTRTypeKey : MTRUnsignedIntegerValueType,
MTRValueKey : @(17),
},
},
],
},
};

NSUInteger arrayLength = 1000;
NSMutableArray * inputArray = [[NSMutableArray alloc] initWithCapacity:arrayLength];
for (NSUInteger i = 0; i < arrayLength; ++i) {
[inputArray addObject:singleListItem];
}

// Descriptor, DeviceTypeList
NSDictionary * input = @{
MTRAttributePathKey : [MTRAttributePath attributePathWithEndpointID:@(0) clusterID:@(0x001d) attributeID:@(0)],
MTRDataKey : @ {
MTRTypeKey : MTRArrayValueType,
MTRValueKey : inputArray,
},
};

NSError * error;
__auto_type * report = [[MTRAttributeReport alloc] initWithResponseValue:input error:&error];
XCTAssertNil(error);
XCTAssertNotNil(report);

XCTAssertEqualObjects(input[MTRAttributePathKey], report.path);
XCTAssertNotNil(report.value);
XCTAssertTrue([report.value isKindOfClass:[NSArray class]]);

NSArray * array = report.value;
XCTAssertTrue(array.count == inputArray.count);
for (id item in array) {
XCTAssertTrue([item isKindOfClass:[MTRDescriptorClusterDeviceTypeStruct class]]);
MTRDescriptorClusterDeviceTypeStruct * deviceType = item;
XCTAssertEqualObjects(deviceType.deviceType, @(100));
XCTAssertEqualObjects(deviceType.revision, @(17));
}

XCTAssertNil(report.error);
}

@end
2 changes: 1 addition & 1 deletion src/system/TLVPacketBufferBackingStore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ CHIP_ERROR TLVPacketBufferBackingStore::FinalizeBuffer(chip::TLV::TLVWriter & wr
{
return CHIP_ERROR_INVALID_ARGUMENT;
}
mCurrentBuffer->SetDataLength(static_cast<uint16_t>(length));
mCurrentBuffer->SetDataLength(static_cast<uint16_t>(length), mHeadBuffer);

return CHIP_NO_ERROR;
}
Expand Down

0 comments on commit d393274

Please sign in to comment.