diff --git a/src/darwin/Framework/CHIP/MTRDevice.h b/src/darwin/Framework/CHIP/MTRDevice.h index 9d3b0768ff4061..a2a31f294877e6 100644 --- a/src/darwin/Framework/CHIP/MTRDevice.h +++ b/src/darwin/Framework/CHIP/MTRDevice.h @@ -338,8 +338,11 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1)) * If the timeout is set to 0, the request will not expire and completion will not be called until * the log is fully retrieved or an error occurs. * @param queue The queue on which completion will be called. - * @param completion The completion that will be called to return the URL of the requested log if successful. Otherwise - * returns an error. + * @param completion The completion handler that is called after attempting to retrieve the requested log. + * - If there are no logs available, it returns a nil URL and a nil error. + * - In cases of errors where no logs are available, it returns a nil URL and a non-nil error. + * - If log retrieval is successful and logs are available, it returns a non-nil URL and a nil error. + * - If an error occurs but parts of the logs has been downloaded, it returns a non-nil URL and a non-nil error. */ - (void)downloadLogOfType:(MTRDiagnosticLogType)type timeout:(NSTimeInterval)timeout diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm index decd879a657ef3..c23eb463b16fc1 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceController.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceController.mm @@ -1222,12 +1222,15 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID queue:(dispatch_queue_t)queue completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion { - [_factory downloadLogFromNodeWithID:nodeID - controller:self - type:type - timeout:timeout - queue:queue - completion:completion]; + [self asyncDispatchToMatterQueue:^() { + [_factory downloadLogFromNodeWithID:nodeID + controller:self + type:type + timeout:timeout + queue:queue + completion:completion]; + } + errorHandler:nil]; } @end diff --git a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm index 319c33fdac999e..6f2c769fff2ad5 100644 --- a/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm @@ -1078,24 +1078,20 @@ - (void)downloadLogFromNodeWithID:(NSNumber *)nodeID queue:(dispatch_queue_t)queue completion:(void (^)(NSURL * _Nullable url, NSError * _Nullable error))completion { - dispatch_sync(_chipWorkQueue, ^{ - if (![self isRunning]) { - return; - } + assertChipStackLockedByCurrentThread(); - if (_diagnosticLogsDownloader == nil) { - _diagnosticLogsDownloader = [[MTRDiagnosticLogsDownloader alloc] init]; - auto systemState = _controllerFactory->GetSystemState(); - systemState->BDXTransferServer()->SetDelegate([_diagnosticLogsDownloader getBridge]); - } + if (_diagnosticLogsDownloader == nil) { + _diagnosticLogsDownloader = [[MTRDiagnosticLogsDownloader alloc] init]; + auto systemState = _controllerFactory->GetSystemState(); + systemState->BDXTransferServer()->SetDelegate([_diagnosticLogsDownloader getBridge]); + } - [_diagnosticLogsDownloader downloadLogFromNodeWithID:nodeID - controller:controller - type:type - timeout:timeout - queue:queue - completion:completion]; - }); + [_diagnosticLogsDownloader downloadLogFromNodeWithID:nodeID + controller:controller + type:type + timeout:timeout + queue:queue + completion:completion]; } - (void)operationalInstanceAdded:(chip::PeerId &)operationalID diff --git a/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm b/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm index 4814b37d7a82f4..20e2acb03031e4 100644 --- a/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm +++ b/src/darwin/Framework/CHIP/MTRDiagnosticLogsDownloader.mm @@ -59,7 +59,7 @@ - (instancetype)initWithType:(MTRDiagnosticLogType)type - (void)writeToFile:(NSData *)data error:(out NSError **)error; -- (BOOL)compare:(NSString *)fileDesignator +- (BOOL)matches:(NSString *)fileDesignator fabricIndex:(NSNumber *)fabricIndex nodeID:(NSNumber *)nodeID; @@ -139,7 +139,7 @@ - (void)handleBDXTransferSessionEndForFileDesignator:(NSString *)fileDesignator private: static void OnTransferTimeout(chip::System::Layer * layer, void * context); - MTRDiagnosticLogsDownloader * mDelegate; + MTRDiagnosticLogsDownloader * __weak mDelegate; AbortHandler mAbortHandler; }; @@ -162,7 +162,7 @@ - (instancetype)initWithType:(MTRDiagnosticLogType)type Download * strongSelf = weakSelf; if (strongSelf) { // If a fileHandle exists, it means that the BDX session has been initiated and a file has - // been created to host the data of the session. So even if there is an error it may be some + // been created to host the data of the session. So even if there is an error there may be some // data in the logs that the caller may find useful. For this reason, fileURL is passed in even // when there is an error but fileHandle is not nil. completion(strongSelf->_fileHandle ? fileURL : nil, bdxError); @@ -192,7 +192,7 @@ - (void)checkInteractionModelResponse:(MTRDiagnosticLogsClusterRetrieveLogsRespo VerifyOrReturn(![status isEqual:@(MTRDiagnosticLogsStatusBusy)], [self failure:[MTRError errorForCHIPErrorCode:CHIP_ERROR_BUSY]]); VerifyOrReturn(![status isEqual:@(MTRDiagnosticLogsStatusDenied)], [self failure:[MTRError errorForCHIPErrorCode:CHIP_ERROR_ACCESS_DENIED]]); - // If there is not logs for the given type, forward it to the caller with a nil url and stop here. + // If there is no logs for the given type, forward it to the caller with a nil url and stop here. VerifyOrReturn(![status isEqual:@(MTRDiagnosticLogsStatusNoLogs)], [self success]); // If the whole log content fits into the response LogContent field, forward it to the caller @@ -200,7 +200,7 @@ - (void)checkInteractionModelResponse:(MTRDiagnosticLogsClusterRetrieveLogsRespo if ([status isEqual:@(MTRDiagnosticLogsStatusExhausted)]) { NSError * writeError = nil; [self writeToFile:response.logContent error:&writeError]; - VerifyOrReturn(nil == writeError, [self failure:[MTRError errorForCHIPErrorCode:CHIP_ERROR_INTERNAL]]); + VerifyOrReturn(nil == writeError, [self failure:writeError]); [self success]; return; @@ -238,7 +238,7 @@ - (void)deleteFile [[NSFileManager defaultManager] removeItemAtPath:[_fileURL path] error:&error]; if (nil != error) { // There is an error but there is really not much we can do at that point besides logging it. - MTR_LOG_ERROR("Error: %@", error); + MTR_LOG_ERROR("Error trying to delete the log file: %@. Error: %@", _fileURL, error); } } @@ -253,7 +253,7 @@ - (BOOL)compare:(NSString *)fileDesignator fabricIndex:(NSNumber *)fabricIndex nodeID:(NSNumber *)nodeID { - return [_fileDesignator isEqualToString:fileDesignator] && _fabricIndex == fabricIndex && _nodeID == nodeID; + return [_fileDesignator isEqualToString:fileDesignator] && [_fabricIndex isEqualToNumber:fabricIndex] && [_nodeID isEqualToNumber:nodeID]; } - (void)failure:(NSError * _Nullable)error @@ -329,7 +329,7 @@ - (void)dealloc - (Download * _Nullable)get:(NSString *)fileDesignator fabricIndex:(NSNumber *)fabricIndex nodeID:(NSNumber *)nodeID { for (Download * download in _downloads) { - if ([download compare:fileDesignator fabricIndex:fabricIndex nodeID:nodeID]) { + if ([download matches:fileDesignator fabricIndex:fabricIndex nodeID:nodeID]) { return download; } }