From 9119912cea847c2823e0a319d26b802fca0b4bbc Mon Sep 17 00:00:00 2001 From: Pankaj Garg Date: Thu, 18 Nov 2021 08:00:23 -0800 Subject: [PATCH] Send cluster specific errors from AdministratorCommissioning server (#11883) * Send cluster specific errors from AdministratorCommissioning server * address review comments * initialize optional value --- .../administrator-commissioning-server.cpp | 65 ++++++++++++------- .../operational-credentials-server.cpp | 4 +- .../administrator-commissioning-cluster.xml | 4 +- .../python/chip/clusters/Objects.py | 4 +- .../zap-generated/cluster-objects.h | 6 +- .../app-common/zap-generated/enums.h | 6 +- 6 files changed, 54 insertions(+), 35 deletions(-) diff --git a/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp b/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp index 815661f35da884..d7e260005ed39b 100644 --- a/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp +++ b/src/app/clusters/administrator-commissioning-server/administrator-commissioning-server.cpp @@ -46,36 +46,41 @@ bool emberAfAdministratorCommissioningClusterOpenCommissioningWindowCallback( auto & salt = commandData.salt; auto & passcodeID = commandData.passcodeID; - EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS; + Optional status = Optional::Missing(); PASEVerifier verifier; const uint8_t * verifierData = pakeVerifier.data(); ChipLogProgress(Zcl, "Received command to open commissioning window"); VerifyOrExit(!Server::GetInstance().GetCommissioningWindowManager().IsCommissioningWindowOpen(), - status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(sizeof(verifier) == pakeVerifier.size(), status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(iterations >= kPBKDFMinimumIterations, status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(iterations <= kPBKDFMaximumIterations, status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(salt.size() >= kPBKDFMinimumSaltLen, status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(salt.size() <= kPBKDFMaximumSaltLen, status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds, status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(discriminator <= kMaxDiscriminatorValue, status = EMBER_ZCL_STATUS_FAILURE); + status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY)); + VerifyOrExit(sizeof(verifier) == pakeVerifier.size(), status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); + VerifyOrExit(iterations >= kPBKDFMinimumIterations, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); + VerifyOrExit(iterations <= kPBKDFMaximumIterations, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); + VerifyOrExit(salt.size() >= kPBKDFMinimumSaltLen, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); + VerifyOrExit(salt.size() <= kPBKDFMaximumSaltLen, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); + VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds, + status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); + VerifyOrExit(discriminator <= kMaxDiscriminatorValue, status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); memcpy(verifier.mW0, &verifierData[0], kSpake2p_WS_Length); memcpy(verifier.mL, &verifierData[kSpake2p_WS_Length], kSpake2p_WS_Length); VerifyOrExit(Server::GetInstance().GetCommissioningWindowManager().OpenEnhancedCommissioningWindow( commissioningTimeout, discriminator, verifier, iterations, salt, passcodeID) == CHIP_NO_ERROR, - status = EMBER_ZCL_STATUS_FAILURE); + status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); ChipLogProgress(Zcl, "Commissioning window is now open"); exit: - if (status != EMBER_ZCL_STATUS_SUCCESS) + if (status.HasValue()) { - ChipLogError(Zcl, "Failed to open commissioning window. Status %d", status); + ChipLogError(Zcl, "Failed to open commissioning window. Status %d", status.Value()); + commandObj->AddClusterSpecificFailure(commandPath, status.Value()); + } + else + { + emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); } - emberAfSendImmediateDefaultResponse(status); return true; } @@ -85,22 +90,27 @@ bool emberAfAdministratorCommissioningClusterOpenBasicCommissioningWindowCallbac { auto & commissioningTimeout = commandData.commissioningTimeout; - EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS; + Optional status = Optional::Missing(); ChipLogProgress(Zcl, "Received command to open basic commissioning window"); VerifyOrExit(!Server::GetInstance().GetCommissioningWindowManager().IsCommissioningWindowOpen(), - status = EMBER_ZCL_STATUS_FAILURE); - VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds, status = EMBER_ZCL_STATUS_FAILURE); + status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_BUSY)); + VerifyOrExit(commissioningTimeout <= kMaxCommissionioningTimeoutSeconds, + status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); VerifyOrExit(Server::GetInstance().GetCommissioningWindowManager().OpenBasicCommissioningWindow(commissioningTimeout) == CHIP_NO_ERROR, - status = EMBER_ZCL_STATUS_FAILURE); + status.Emplace(StatusCode::EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR)); ChipLogProgress(Zcl, "Commissioning window is now open"); exit: - if (status != EMBER_ZCL_STATUS_SUCCESS) + if (status.HasValue()) { - ChipLogError(Zcl, "Failed to open commissioning window. Status %d", status); + ChipLogError(Zcl, "Failed to open commissioning window. Status %d", status.Value()); + commandObj->AddClusterSpecificFailure(commandPath, status.Value()); + } + else + { + emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); } - emberAfSendImmediateDefaultResponse(status); return true; } @@ -109,9 +119,18 @@ bool emberAfAdministratorCommissioningClusterRevokeCommissioningCallback( const Commands::RevokeCommissioning::DecodableType & commandData) { ChipLogProgress(Zcl, "Received command to close commissioning window"); - Server::GetInstance().GetCommissioningWindowManager().CloseCommissioningWindow(); - ChipLogProgress(Zcl, "Commissioning window is now closed"); - emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); + + if (!Server::GetInstance().GetCommissioningWindowManager().IsCommissioningWindowOpen()) + { + ChipLogError(Zcl, "Commissioning window is currently not open"); + commandObj->AddClusterSpecificFailure(commandPath, StatusCode::EMBER_ZCL_STATUS_CODE_WINDOW_NOT_OPEN); + } + else + { + Server::GetInstance().GetCommissioningWindowManager().CloseCommissioningWindow(); + ChipLogProgress(Zcl, "Commissioning window is now closed"); + emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); + } return true; } diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index 2575148ede0cb2..c9338fe6a52352 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -679,11 +679,11 @@ bool emberAfOperationalCredentialsClusterAddTrustedRootCertificateCallback( emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: commissioner has added a trusted root Cert"); - VerifyOrExit(gFabricBeingCommissioned.SetRootCert(RootCertificate) == CHIP_NO_ERROR, status = EMBER_ZCL_STATUS_FAILURE); + VerifyOrExit(gFabricBeingCommissioned.SetRootCert(RootCertificate) == CHIP_NO_ERROR, status = EMBER_ZCL_STATUS_INVALID_FIELD); exit: emberAfSendImmediateDefaultResponse(status); - if (status == EMBER_ZCL_STATUS_FAILURE) + if (status != EMBER_ZCL_STATUS_SUCCESS) { gFabricBeingCommissioned.Reset(); emberAfPrintln(EMBER_AF_PRINT_DEBUG, "OpCreds: Failed AddTrustedRootCert request."); diff --git a/src/app/zap-templates/zcl/data-model/chip/administrator-commissioning-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/administrator-commissioning-cluster.xml index 90986f77860baf..bbb0235153757f 100644 --- a/src/app/zap-templates/zcl/data-model/chip/administrator-commissioning-cluster.xml +++ b/src/app/zap-templates/zcl/data-model/chip/administrator-commissioning-cluster.xml @@ -19,9 +19,9 @@ limitations under the License. - - + + diff --git a/src/controller/python/chip/clusters/Objects.py b/src/controller/python/chip/clusters/Objects.py index 6ce944593087b8..2c99005c2e9961 100644 --- a/src/controller/python/chip/clusters/Objects.py +++ b/src/controller/python/chip/clusters/Objects.py @@ -8740,9 +8740,9 @@ class AdministratorCommissioning(Cluster): class Enums: class StatusCode(IntEnum): - kSuccess = 0x00 kBusy = 0x01 - kGeneralError = 0x02 + kPAKEParameterError = 0x02 + kWindowNotOpen = 0x03 class Commands: @dataclass diff --git a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h index e38f3fe3b44080..48577d8a2bc9bc 100644 --- a/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h +++ b/zzz_generated/app-common/app-common/zap-generated/cluster-objects.h @@ -11508,9 +11508,9 @@ namespace AdministratorCommissioning { // Enum for StatusCode enum class StatusCode : uint8_t { - kSuccess = 0x00, - kBusy = 0x01, - kGeneralError = 0x02, + kBusy = 0x01, + kPAKEParameterError = 0x02, + kWindowNotOpen = 0x03, }; #else // CHIP_USE_ENUM_CLASS_FOR_IM_ENUM using StatusCode = EmberAfStatusCode; diff --git a/zzz_generated/app-common/app-common/zap-generated/enums.h b/zzz_generated/app-common/app-common/zap-generated/enums.h index 5845dca10820d0..95f498d1503ef4 100644 --- a/zzz_generated/app-common/app-common/zap-generated/enums.h +++ b/zzz_generated/app-common/app-common/zap-generated/enums.h @@ -1497,9 +1497,9 @@ enum EmberAfStartUpOnOffValue : uint8_t // Enum for StatusCode enum EmberAfStatusCode : uint8_t { - EMBER_ZCL_STATUS_CODE_SUCCESS = 0, - EMBER_ZCL_STATUS_CODE_BUSY = 1, - EMBER_ZCL_STATUS_CODE_GENERAL_ERROR = 2, + EMBER_ZCL_STATUS_CODE_BUSY = 1, + EMBER_ZCL_STATUS_CODE_PAKE_PARAMETER_ERROR = 2, + EMBER_ZCL_STATUS_CODE_WINDOW_NOT_OPEN = 3, }; // Enum for StepMode