Skip to content

Commit dbb38c1

Browse files
bzbarsky-applebzbarsky-apple
committedApr 16, 2024·
Enable chunking on writes via MTRBaseDevice's writeAttributeWithEndpointID:.
We used to just encode the list a single TLV item, which did not allow the write to chunk it. To fix that, create a DataModel::List with the individual list items, and encode that.
1 parent 4aadee7 commit dbb38c1

File tree

2 files changed

+215
-3
lines changed

2 files changed

+215
-3
lines changed
 

‎src/darwin/Framework/CHIP/MTRBaseDevice.mm

+48-3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
#include <app/ClusterStateCache.h>
4848
#include <app/InteractionModelEngine.h>
4949
#include <app/ReadClient.h>
50+
#include <app/data-model/List.h>
5051
#include <controller/CommissioningWindowOpener.h>
5152
#include <controller/ReadInteraction.h>
5253
#include <controller/WriteInteraction.h>
@@ -633,10 +634,11 @@ static CHIP_ERROR MTREncodeTLVFromDataValueDictionary(id object, chip::TLV::TLVW
633634
}
634635
NSString * typeName = ((NSDictionary *) object)[MTRTypeKey];
635636
id value = ((NSDictionary *) object)[MTRValueKey];
636-
if (!typeName) {
637+
if (![typeName isKindOfClass:[NSString class]]) {
637638
MTR_LOG_ERROR("Error: Object to encode is corrupt");
638639
return CHIP_ERROR_INVALID_ARGUMENT;
639640
}
641+
640642
if ([typeName isEqualToString:MTRSignedIntegerValueType]) {
641643
if (![value isKindOfClass:[NSNumber class]]) {
642644
MTR_LOG_ERROR("Error: Object to encode has corrupt signed integer type: %@", [value class]);
@@ -1219,10 +1221,53 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
12191221
auto onFailureCb = [failureCb, bridge](
12201222
const app::ConcreteAttributePath * attribPath, CHIP_ERROR aError) { failureCb(bridge, aError); };
12211223

1222-
return chip::Controller::WriteAttribute<MTRDataValueDictionaryDecodableType>(session,
1224+
// To handle list chunking properly, we have to convert lists into
1225+
// DataModel::List here, because that's special-cased in
1226+
// WriteClient.
1227+
if (![value isKindOfClass:NSDictionary.class]) {
1228+
MTR_LOG_ERROR("Error: Unsupported object to write as attribute value: %@", value);
1229+
return CHIP_ERROR_INVALID_ARGUMENT;
1230+
}
1231+
1232+
NSDictionary<NSString *, id> * dataValue = value;
1233+
NSString * typeName = dataValue[MTRTypeKey];
1234+
if (![typeName isKindOfClass:NSString.class]) {
1235+
MTR_LOG_ERROR("Error: Object to encode is corrupt: %@", dataValue);
1236+
return CHIP_ERROR_INVALID_ARGUMENT;
1237+
}
1238+
1239+
if (![typeName isEqualToString:MTRArrayValueType]) {
1240+
return chip::Controller::WriteAttribute<MTRDataValueDictionaryDecodableType>(session,
1241+
static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
1242+
static_cast<chip::ClusterId>([clusterID unsignedLongValue]),
1243+
static_cast<chip::AttributeId>([attributeID unsignedLongValue]), MTRDataValueDictionaryDecodableType(value),
1244+
onSuccessCb, onFailureCb, (timeoutMs == nil) ? NullOptional : Optional<uint16_t>([timeoutMs unsignedShortValue]));
1245+
}
1246+
1247+
// Now we are dealing with a list.
1248+
NSArray<NSDictionary<NSString *, id> *> * arrayValue = value[MTRValueKey];
1249+
if (![arrayValue isKindOfClass:NSArray.class]) {
1250+
MTR_LOG_ERROR("Error: Object to encode claims to be a list but isn't: %@", arrayValue);
1251+
return CHIP_ERROR_INVALID_ARGUMENT;
1252+
}
1253+
1254+
std::vector<MTRDataValueDictionaryDecodableType> encodableVector;
1255+
encodableVector.reserve(arrayValue.count);
1256+
1257+
for (NSDictionary<NSString *, id> * arrayItem in arrayValue) {
1258+
if (![arrayItem isKindOfClass:NSDictionary.class]) {
1259+
MTR_LOG_ERROR("Error: Can't encode corrupt list: %@", arrayValue);
1260+
return CHIP_ERROR_INVALID_ARGUMENT;
1261+
}
1262+
1263+
encodableVector.push_back(MTRDataValueDictionaryDecodableType(arrayItem[MTRDataKey]));
1264+
}
1265+
1266+
DataModel::List<MTRDataValueDictionaryDecodableType> encodableList(encodableVector.data(), encodableVector.size());
1267+
return chip::Controller::WriteAttribute<DataModel::List<MTRDataValueDictionaryDecodableType>>(session,
12231268
static_cast<chip::EndpointId>([endpointID unsignedShortValue]),
12241269
static_cast<chip::ClusterId>([clusterID unsignedLongValue]),
1225-
static_cast<chip::AttributeId>([attributeID unsignedLongValue]), MTRDataValueDictionaryDecodableType(value),
1270+
static_cast<chip::AttributeId>([attributeID unsignedLongValue]), encodableList,
12261271
onSuccessCb, onFailureCb, (timeoutMs == nil) ? NullOptional : Optional<uint16_t>([timeoutMs unsignedShortValue]));
12271272
});
12281273
std::move(*bridge).DispatchAction(self);

‎src/darwin/Framework/CHIPTests/MTROTAProviderTests.m

+167
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
// module headers
1919
#import <Matter/Matter.h>
2020

21+
#import "MTRDeviceTestDelegate.h"
2122
#import "MTRErrorTestUtils.h"
2223
#import "MTRTestKeys.h"
2324
#import "MTRTestResetCommissioneeHelper.h"
@@ -1564,6 +1565,172 @@ - (void)test007_DoBDXTransferIncrementalOtaUpdate
15641565
}
15651566
#endif // ENABLE_REAL_OTA_UPDATE_TESTS
15661567

1568+
- (void)test008_TestWriteDefaultOTAProviders
1569+
{
1570+
__auto_type * runner = [[MTROTARequestorAppRunner alloc] initWithPayload:kOnboardingPayload1 testcase:self];
1571+
MTRDevice * device = [runner commissionWithNodeID:@(kDeviceId1)];
1572+
1573+
dispatch_queue_t queue = dispatch_get_main_queue();
1574+
1575+
__auto_type dataValue = ^(uint16_t endpoint) {
1576+
return @{
1577+
MTRTypeKey : MTRArrayValueType,
1578+
MTRValueKey : @[
1579+
@{
1580+
MTRDataKey : @ {
1581+
MTRTypeKey : MTRStructureValueType,
1582+
MTRValueKey : @[
1583+
@{
1584+
MTRContextTagKey : @(1),
1585+
MTRDataKey : @ {
1586+
MTRTypeKey : MTRUnsignedIntegerValueType,
1587+
MTRValueKey : @(kDeviceId1),
1588+
},
1589+
},
1590+
@{
1591+
MTRContextTagKey : @(2),
1592+
MTRDataKey : @ {
1593+
MTRTypeKey : MTRUnsignedIntegerValueType,
1594+
MTRValueKey : @(endpoint),
1595+
},
1596+
},
1597+
],
1598+
},
1599+
},
1600+
],
1601+
};
1602+
};
1603+
1604+
{
1605+
// Test with MTRBaseDevice first.
1606+
MTRBaseDevice * baseDevice = [MTRBaseDevice deviceWithNodeID:device.nodeID
1607+
controller:device.deviceController];
1608+
1609+
__auto_type * cluster = [[MTRBaseClusterOTASoftwareUpdateRequestor alloc] initWithDevice:baseDevice
1610+
endpointID:@(0)
1611+
queue:queue];
1612+
__auto_type * providerLocation = [[MTROTASoftwareUpdateRequestorClusterProviderLocation alloc] init];
1613+
providerLocation.providerNodeID = @(kDeviceId1);
1614+
providerLocation.endpoint = @(0);
1615+
__auto_type * value = @[ providerLocation ];
1616+
1617+
__auto_type * writeBaseClusterExpectation = [self expectationWithDescription:@"Write succeeded via MTRBaseCluster"];
1618+
[cluster writeAttributeDefaultOTAProvidersWithValue:value
1619+
completion:^(NSError * _Nullable error) {
1620+
XCTAssertNil(error);
1621+
[writeBaseClusterExpectation fulfill];
1622+
}];
1623+
[self waitForExpectations:@[ writeBaseClusterExpectation ] timeout:kTimeoutInSeconds];
1624+
1625+
__auto_type * writeBaseDeviceExpectation = [self expectationWithDescription:@"Write succeeded via MTRBaseDevice"];
1626+
[baseDevice writeAttributeWithEndpointID:@(0)
1627+
clusterID:@(MTRClusterIDTypeOTASoftwareUpdateRequestorID)
1628+
attributeID:@(MTRAttributeIDTypeClusterOTASoftwareUpdateRequestorAttributeDefaultOTAProvidersID)
1629+
value:dataValue(0)
1630+
timedWriteTimeout:nil
1631+
queue:queue
1632+
completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
1633+
XCTAssertNil(error);
1634+
XCTAssertNotNil(values);
1635+
XCTAssertEqual(values.count, 1);
1636+
1637+
for (NSDictionary<NSString *, id> * value in values) {
1638+
XCTAssertNil(value[MTRErrorKey]);
1639+
}
1640+
[writeBaseDeviceExpectation fulfill];
1641+
}];
1642+
[self waitForExpectations:@[ writeBaseDeviceExpectation ] timeout:kTimeoutInSeconds];
1643+
}
1644+
1645+
{
1646+
// Now test with MTRDevice
1647+
__auto_type * delegate = [[MTRDeviceTestDelegate alloc] init];
1648+
// Make sure we don't have expected value notifications confusing our
1649+
// attribute reports.
1650+
delegate.skipExpectedValuesForWrite = YES;
1651+
1652+
XCTestExpectation * gotReportsExpectation = [self expectationWithDescription:@"Subscription established"];
1653+
delegate.onReportEnd = ^() {
1654+
[gotReportsExpectation fulfill];
1655+
};
1656+
1657+
[device setDelegate:delegate queue:queue];
1658+
1659+
[self waitForExpectations:@[ gotReportsExpectation ] timeout:60];
1660+
1661+
delegate.onReportEnd = nil;
1662+
1663+
__auto_type * expectedAttributePath = [MTRAttributePath attributePathWithEndpointID:@(0)
1664+
clusterID:@(MTRClusterIDTypeOTASoftwareUpdateRequestorID)
1665+
attributeID:@(MTRAttributeIDTypeClusterOTASoftwareUpdateRequestorAttributeDefaultOTAProvidersID)];
1666+
1667+
__block __auto_type * expectedValue = dataValue(1);
1668+
1669+
__block __auto_type * writeExpectation = [self expectationWithDescription:@"Write succeeded via MTRCluster"];
1670+
delegate.onAttributeDataReceived = ^(NSArray<NSDictionary<NSString *, id> *> * data) {
1671+
XCTAssertNotNil(data);
1672+
XCTAssertEqual(data.count, 1);
1673+
NSDictionary<NSString *, id> * item = data[0];
1674+
1675+
XCTAssertNil(item[MTRErrorKey]);
1676+
1677+
MTRAttributePath * path = item[MTRAttributePathKey];
1678+
XCTAssertNotNil(path);
1679+
1680+
XCTAssertEqualObjects(path, expectedAttributePath);
1681+
1682+
NSDictionary<NSString *, id> * receivedValue = item[MTRDataKey];
1683+
1684+
// We can't use XCTAssertEqualObjects to compare receivedValue to
1685+
// expectedValue here, because receivedValue has a DataVersion
1686+
// that's missing from expectedValue, and the struct in it has an
1687+
// extra FabricIndex field.
1688+
XCTAssertEqualObjects(receivedValue[MTRTypeKey], MTRArrayValueType);
1689+
1690+
NSArray * receivedArray = receivedValue[MTRValueKey];
1691+
NSArray * expectedArray = expectedValue[MTRValueKey];
1692+
1693+
XCTAssertEqual(receivedArray.count, expectedArray.count);
1694+
1695+
for (NSUInteger i = 0; i < receivedArray.count; ++i) {
1696+
NSDictionary * receivedItem = receivedArray[i][MTRDataKey];
1697+
NSDictionary * expectedItem = expectedArray[i][MTRDataKey];
1698+
1699+
XCTAssertEqual(receivedItem[MTRTypeKey], MTRStructureValueType);
1700+
XCTAssertEqual(expectedItem[MTRTypeKey], MTRStructureValueType);
1701+
1702+
NSArray * receivedFields = receivedItem[MTRValueKey];
1703+
NSArray * expectedFields = expectedItem[MTRValueKey];
1704+
1705+
// Account for the extra FabricIndex.
1706+
XCTAssertEqual(receivedFields.count, expectedFields.count + 1);
1707+
for (NSUInteger j = 0; j < expectedFields.count; ++j) {
1708+
XCTAssertEqualObjects(receivedFields[j], expectedFields[j]);
1709+
}
1710+
}
1711+
1712+
[writeExpectation fulfill];
1713+
};
1714+
1715+
__auto_type * cluster = [[MTRClusterOTASoftwareUpdateRequestor alloc] initWithDevice:device
1716+
endpointID:@(0)
1717+
queue:queue];
1718+
[cluster writeAttributeDefaultOTAProvidersWithValue:expectedValue
1719+
expectedValueInterval:@(0)];
1720+
[self waitForExpectations:@[ writeExpectation ] timeout:kTimeoutInSeconds];
1721+
1722+
expectedValue = dataValue(2);
1723+
writeExpectation = [self expectationWithDescription:@"Write succeeded via MTRDevice"];
1724+
[device writeAttributeWithEndpointID:@(0)
1725+
clusterID:@(MTRClusterIDTypeOTASoftwareUpdateRequestorID)
1726+
attributeID:@(MTRAttributeIDTypeClusterOTASoftwareUpdateRequestorAttributeDefaultOTAProvidersID)
1727+
value:expectedValue
1728+
expectedValueInterval:@(0)
1729+
timedWriteTimeout:nil];
1730+
[self waitForExpectations:@[ writeExpectation ] timeout:kTimeoutInSeconds];
1731+
}
1732+
}
1733+
15671734
- (void)test999_TearDown
15681735
{
15691736
[[self class] shutdownStack];

0 commit comments

Comments
 (0)
Please sign in to comment.