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

Remove writeAttributeWithEndpointID implementation from MTRDevice. #35170

Merged
merged 1 commit into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
136 changes: 9 additions & 127 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1831,11 +1831,12 @@ static BOOL AttributeHasChangesOmittedQuality(MTRAttributePath * attributePath)
attributeID:(NSNumber *)attributeID
params:(MTRReadParams * _Nullable)params
{
#define ErrorStr "MTRDevice readAttributeWithEndpointID:clusterID:attributeID:params: must be handled by subclasses"
MTR_LOG_ERROR(ErrorStr);
#define MTRDeviceErrorStr "MTRDevice readAttributeWithEndpointID:clusterID:attributeID:params: must be handled by subclasses"
MTR_LOG_ERROR(MTRDeviceErrorStr);
#ifdef DEBUG
NSAssert(NO, @ErrorStr);
NSAssert(NO, @MTRDeviceErrorStr);
#endif // DEBUG
#undef MTRDeviceErrorStr
return nil;
}

Expand All @@ -1846,120 +1847,12 @@ - (void)writeAttributeWithEndpointID:(NSNumber *)endpointID
expectedValueInterval:(NSNumber *)expectedValueInterval
timedWriteTimeout:(NSNumber * _Nullable)timeout
{
if (timeout) {
timeout = MTRClampedNumber(timeout, @(1), @(UINT16_MAX));
}
expectedValueInterval = MTRClampedNumber(expectedValueInterval, @(1), @(UINT32_MAX));
MTRAttributePath * attributePath = [MTRAttributePath attributePathWithEndpointID:endpointID
clusterID:clusterID

attributeID:attributeID];

__block BOOL useValueAsExpectedValue = YES;
#define MTRDeviceErrorStr "MTRDevice writeAttributeWithEndpointID:clusterID:attributeID:value:expectedValueInterval:timedWriteTimeout: must be handled by subclasses"
MTR_LOG_ERROR(MTRDeviceErrorStr);
#ifdef DEBUG
os_unfair_lock_lock(&self->_lock);
[self _callFirstDelegateSynchronouslyWithBlock:^(id delegate) {
if ([delegate respondsToSelector:@selector(unitTestShouldSkipExpectedValuesForWrite:)]) {
useValueAsExpectedValue = ![delegate unitTestShouldSkipExpectedValuesForWrite:self];
}
}];
os_unfair_lock_unlock(&self->_lock);
#endif

uint64_t expectedValueID = 0;
if (useValueAsExpectedValue) {
// Commit change into expected value cache
NSDictionary * newExpectedValueDictionary = @{ MTRAttributePathKey : attributePath, MTRDataKey : value };
[self setExpectedValues:@[ newExpectedValueDictionary ]
expectedValueInterval:expectedValueInterval
expectedValueID:&expectedValueID];
}

MTRAsyncWorkItem * workItem = [[MTRAsyncWorkItem alloc] initWithQueue:self.queue];
uint64_t workItemID = workItem.uniqueID; // capture only the ID, not the work item
NSNumber * nodeID = _nodeID;

// Write request data is an array of items (for now always length 1). Each
// item is an array containing:
//
// [ attribute path, value, timedWriteTimeout, expectedValueID ]
//
// where expectedValueID is stored as NSNumber and NSNull represents nil timeouts
auto * writeData = @[ attributePath, [value copy], timeout ?: [NSNull null], @(expectedValueID) ];

NSMutableArray<NSArray *> * writeRequests = [NSMutableArray arrayWithObject:writeData];

[workItem setBatchingID:MTRDeviceWorkItemBatchingWriteID data:writeRequests handler:^(id opaqueDataCurrent, id opaqueDataNext) {
mtr_hide(self); // don't capture self accidentally
NSMutableArray<NSArray *> * writeRequestsCurrent = opaqueDataCurrent;
NSMutableArray<NSArray *> * writeRequestsNext = opaqueDataNext;

if (writeRequestsCurrent.count != 1) {
// Very unexpected!
MTR_LOG_ERROR("Batching write attribute work item [%llu]: Unexpected write request count %lu", workItemID, static_cast<unsigned long>(writeRequestsCurrent.count));
return MTRNotBatched;
}

MTRBatchingOutcome outcome = MTRNotBatched;
while (writeRequestsNext.count) {
// If paths don't match, we cannot replace the earlier write
// with the later one.
if (![writeRequestsNext[0][MTRDeviceWriteRequestFieldPathIndex]
isEqual:writeRequestsCurrent[0][MTRDeviceWriteRequestFieldPathIndex]]) {
MTR_LOG("Batching write attribute work item [%llu]: cannot replace with next work item due to path mismatch", workItemID);
return outcome;
}

// Replace our one request with the first one from the next item.
auto writeItem = writeRequestsNext.firstObject;
[writeRequestsNext removeObjectAtIndex:0];
[writeRequestsCurrent replaceObjectAtIndex:0 withObject:writeItem];
MTR_LOG("Batching write attribute work item [%llu]: replaced with new write value %@ [0x%016llX]",
workItemID, writeItem, nodeID.unsignedLongLongValue);
outcome = MTRBatchedPartially;
}
NSCAssert(writeRequestsNext.count == 0, @"should have batched everything or returned early");
return MTRBatchedFully;
}];
// The write operation will install a duplicate check handler, to return NO for "isDuplicate". Since a write operation may
// change values, only read requests after this should be considered for duplicate requests.
[workItem setDuplicateTypeID:MTRDeviceWorkItemDuplicateReadTypeID handler:^(id opaqueItemData, BOOL * isDuplicate, BOOL * stop) {
*isDuplicate = NO;
*stop = YES;
}];
[workItem setReadyHandler:^(MTRDevice * self, NSInteger retryCount, MTRAsyncWorkCompletionBlock completion) {
MTRBaseDevice * baseDevice = [self newBaseDevice];
// Make sure to use writeRequests here, because that's what our batching
// handler will modify as needed.
NSCAssert(writeRequests.count == 1, @"Incorrect number of write requests: %lu", static_cast<unsigned long>(writeRequests.count));

auto * request = writeRequests[0];
MTRAttributePath * path = request[MTRDeviceWriteRequestFieldPathIndex];

id timedWriteTimeout = request[MTRDeviceWriteRequestFieldTimeoutIndex];
if (timedWriteTimeout == [NSNull null]) {
timedWriteTimeout = nil;
}

[baseDevice
writeAttributeWithEndpointID:path.endpoint
clusterID:path.cluster
attributeID:path.attribute
value:request[MTRDeviceWriteRequestFieldValueIndex]
timedWriteTimeout:timedWriteTimeout
queue:self.queue
completion:^(NSArray<NSDictionary<NSString *, id> *> * _Nullable values, NSError * _Nullable error) {
if (error) {
MTR_LOG_ERROR("Write attribute work item [%llu] failed: %@", workItemID, error);
if (useValueAsExpectedValue) {
NSNumber * expectedValueID = request[MTRDeviceWriteRequestFieldExpectedValueIDIndex];
[self removeExpectedValueForAttributePath:attributePath expectedValueID:expectedValueID.unsignedLongLongValue];
}
}
completion(MTRAsyncWorkComplete);
}];
}];
[_asyncWorkQueue enqueueWorkItem:workItem descriptionWithFormat:@"write %@ 0x%llx 0x%llx", endpointID, clusterID.unsignedLongLongValue, attributeID.unsignedLongLongValue];
NSAssert(NO, @MTRDeviceErrorStr);
#endif // DEBUG
#undef MTRDeviceErrorStr
}

- (void)invokeCommandWithEndpointID:(NSNumber *)endpointID
Expand Down Expand Up @@ -2796,11 +2689,6 @@ - (NSArray *)_getAttributesToReportWithNewExpectedValues:(NSArray<NSDictionary<N
return attributesToReport;
}

- (void)setExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)values expectedValueInterval:(NSNumber *)expectedValueInterval
{
[self setExpectedValues:values expectedValueInterval:expectedValueInterval expectedValueID:nil];
}

// expectedValueID is an out-argument that returns an identifier to be used when removing expected values
- (void)setExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)values
expectedValueInterval:(NSNumber *)expectedValueInterval
Expand Down Expand Up @@ -2833,12 +2721,6 @@ - (void)removeExpectedValuesForAttributePaths:(NSArray<MTRAttributePath *> *)att
}
}

- (void)removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath expectedValueID:(uint64_t)expectedValueID
{
std::lock_guard lock(_lock);
[self _removeExpectedValueForAttributePath:attributePath expectedValueID:expectedValueID];
}

- (void)_removeExpectedValueForAttributePath:(MTRAttributePath *)attributePath expectedValueID:(uint64_t)expectedValueID
{
os_unfair_lock_assert_owner(&self->_lock);
Expand Down
5 changes: 0 additions & 5 deletions src/darwin/Framework/CHIP/MTRDevice_Concrete.mm
Original file line number Diff line number Diff line change
Expand Up @@ -3644,11 +3644,6 @@ - (NSArray *)_getAttributesToReportWithNewExpectedValues:(NSArray<NSDictionary<N
return attributesToReport;
}

- (void)setExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)values expectedValueInterval:(NSNumber *)expectedValueInterval
{
[self setExpectedValues:values expectedValueInterval:expectedValueInterval expectedValueID:nil];
}

// expectedValueID is an out-argument that returns an identifier to be used when removing expected values
- (void)setExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)values
expectedValueInterval:(NSNumber *)expectedValueInterval
Expand Down
4 changes: 0 additions & 4 deletions src/darwin/Framework/CHIP/MTRDevice_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ MTR_DIRECT_MEMBERS
- (instancetype)initForSubclassesWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller;
- (instancetype)initWithNodeID:(NSNumber *)nodeID controller:(MTRDeviceController *)controller;

// Called from MTRClusters for writes and commands
- (void)setExpectedValues:(NSArray<NSDictionary<NSString *, id> *> *)values
expectedValueInterval:(NSNumber *)expectedValueIntervalMs;

// called by controller to clean up and shutdown
- (void)invalidate;

Expand Down
Loading