Improve error logging during Darwin controller startup.

Right now we are not reporting any details about what error we encountered. We should report those details.
project-chip · Jul 28, 2022 · f6446b5 · f6446b5
1 parent 1679342
commit f6446b5
Showing 1 changed file with 16 additions and 15 deletions.
diff --git a/src/darwin/Framework/CHIP/MTRDeviceController.mm b/src/darwin/Framework/CHIP/MTRDeviceController.mm
@@ -196,19 +196,20 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
         chip::Crypto::P256Keypair * signingKeypair = nullptr;
         if (startupParams.nocSigner) {
             errorCode = _signingKeypairBridge.Init(startupParams.nocSigner);
-            if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorSigningKeypairInit]) {
+            if ([self checkForStartError:errorCode logMsg:kErrorSigningKeypairInit]) {
                 return;
             }
             signingKeypair = &_signingKeypairBridge;
         }
         errorCode = _operationalCredentialsDelegate->Init(_factory.storageDelegateBridge, signingKeypair, startupParams.ipk,
             startupParams.rootCertificate, startupParams.intermediateCertificate);
-        if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalCredentialsInit]) {
+        if ([self checkForStartError:errorCode logMsg:kErrorOperationalCredentialsInit]) {
             return;
         }
 
         _cppCommissioner = new chip::Controller::DeviceCommissioner();
-        if ([self checkForStartError:(_cppCommissioner != nullptr) logMsg:kErrorCommissionerInit]) {
+        if (_cppCommissioner == nullptr) {
+            [self checkForStartError:CHIP_ERROR_NO_MEMORY logMsg:kErrorCommissionerInit];
             return;
         }
 
@@ -228,7 +229,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
 
         if (startupParams.operationalKeypair != nil) {
             errorCode = _operationalKeypairBridge.Init(startupParams.operationalKeypair);
-            if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorOperationalKeypairInit]) {
+            if ([self checkForStartError:errorCode logMsg:kErrorOperationalKeypairInit]) {
                 return;
             }
             commissionerParams.operationalKeypair = &_operationalKeypairBridge;
@@ -244,28 +245,28 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
                 errorCode = _operationalCredentialsDelegate->GenerateNOC([startupParams.nodeId unsignedLongLongValue],
                     startupParams.fabricId, chip::kUndefinedCATs, commissionerParams.operationalKeypair->Pubkey(), noc);
 
-                if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorGenerateNOC]) {
+                if ([self checkForStartError:errorCode logMsg:kErrorGenerateNOC]) {
                     return;
                 }
             } else {
                 // Generate a new random keypair.
                 uint8_t csrBuffer[chip::Crypto::kMAX_CSR_Length];
                 chip::MutableByteSpan csr(csrBuffer);
                 errorCode = startupParams.fabricTable->AllocatePendingOperationalKey(startupParams.fabricIndex, csr);
-                if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorKeyAllocation]) {
+                if ([self checkForStartError:errorCode logMsg:kErrorKeyAllocation]) {
                     return;
                 }
 
                 chip::Crypto::P256PublicKey pubKey;
                 errorCode = VerifyCertificateSigningRequest(csr.data(), csr.size(), pubKey);
-                if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCSRValidation]) {
+                if ([self checkForStartError:errorCode logMsg:kErrorCSRValidation]) {
                     return;
                 }
 
                 errorCode = _operationalCredentialsDelegate->GenerateNOC(
                     [startupParams.nodeId unsignedLongLongValue], startupParams.fabricId, chip::kUndefinedCATs, pubKey, noc);
 
-                if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorGenerateNOC]) {
+                if ([self checkForStartError:errorCode logMsg:kErrorGenerateNOC]) {
                     return;
                 }
             }
@@ -277,7 +278,7 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
         auto & factory = chip::Controller::DeviceControllerFactory::GetInstance();
 
         errorCode = factory.SetupCommissioner(commissionerParams, *_cppCommissioner);
-        if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorCommissionerInit]) {
+        if ([self checkForStartError:errorCode logMsg:kErrorCommissionerInit]) {
             return;
         }
 
@@ -286,13 +287,13 @@ - (BOOL)startup:(MTRDeviceControllerStartupParamsInternal *)startupParams
         uint8_t compressedIdBuffer[sizeof(uint64_t)];
         chip::MutableByteSpan compressedId(compressedIdBuffer);
         errorCode = _cppCommissioner->GetCompressedFabricIdBytes(compressedId);
-        if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorIPKInit]) {
+        if ([self checkForStartError:errorCode logMsg:kErrorIPKInit]) {
             return;
         }
 
         errorCode = chip::Credentials::SetSingleIpkEpochKey(
             _factory.groupData, fabricIdx, _operationalCredentialsDelegate->GetIPK(), compressedId);
-        if ([self checkForStartError:(CHIP_NO_ERROR == errorCode) logMsg:kErrorIPKInit]) {
+        if ([self checkForStartError:errorCode logMsg:kErrorIPKInit]) {
             return;
         }
 
@@ -637,13 +638,13 @@ - (void)clearDeviceAttestationDelegateBridge
     }
 }
 
-- (BOOL)checkForStartError:(BOOL)condition logMsg:(NSString *)logMsg
+- (BOOL)checkForStartError:(CHIP_ERROR)errorCode logMsg:(NSString *)logMsg
 {
-    if (condition) {
+    if (CHIP_NO_ERROR == errorCode) {
         return NO;
     }
 
-    MTR_LOG_ERROR("Error: %@", logMsg);
+    MTR_LOG_ERROR("Error(%" CHIP_ERROR_FORMAT "): %@", errorCode.Format(), logMsg);
 
     return YES;
 }
@@ -654,7 +655,7 @@ - (BOOL)checkForError:(CHIP_ERROR)errorCode logMsg:(NSString *)logMsg error:(NSE
         return NO;
     }
 
-    MTR_LOG_ERROR("Error(%s): %s", chip::ErrorStr(errorCode), [logMsg UTF8String]);
+    MTR_LOG_ERROR("Error(%" CHIP_ERROR_FORMAT "): %s", errorCode.Format(), [logMsg UTF8String]);
     if (error) {
         *error = [MTRError errorForCHIPErrorCode:errorCode];
     }