[darwin-framework-tool] Address post-landing comments of project-chip…

…#31638

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

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

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

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,15 +192,15 @@ - (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
     // and stop here.
     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;
         }
     }