From cf81575f804eb3ab29756b9a720b03ba19e1147c Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Mon, 29 Nov 2021 17:04:19 -0500 Subject: [PATCH 1/4] Remove needless OpcredsIssuer dependency - Dependency on ExampleOperationalCredentialsIssuer was in many files where not needed, making other refactors more difficult. Remove this dependency and make CHIPCommand own its ephemeral operational credentials issuer. --- examples/chip-tool/commands/clusters/ModelCommand.h | 1 - examples/chip-tool/commands/common/CHIPCommand.cpp | 9 ++++++--- examples/chip-tool/commands/common/CHIPCommand.h | 1 - examples/chip-tool/commands/common/Command.h | 1 - examples/chip-tool/commands/common/Commands.h | 1 - examples/chip-tool/commands/discover/DiscoverCommand.h | 1 - examples/chip-tool/commands/pairing/PairingCommand.h | 1 - examples/chip-tool/commands/reporting/ReportingCommand.h | 2 -- examples/chip-tool/commands/tests/TestCommand.h | 1 - 9 files changed, 6 insertions(+), 12 deletions(-) diff --git a/examples/chip-tool/commands/clusters/ModelCommand.h b/examples/chip-tool/commands/clusters/ModelCommand.h index 71f79d4a2b3b08..422db20bf382c0 100644 --- a/examples/chip-tool/commands/clusters/ModelCommand.h +++ b/examples/chip-tool/commands/clusters/ModelCommand.h @@ -21,7 +21,6 @@ #include "../../config/PersistentStorage.h" #include "../common/CHIPCommand.h" #include -#include #include class ModelCommand : public CHIPCommand diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index 7f39cab591b99b..d4960769600fac 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -19,6 +19,7 @@ #include "CHIPCommand.h" #include +#include "controller/ExampleOperationalCredentialsIssuer.h" #include #include #include @@ -31,13 +32,15 @@ using DeviceControllerFactory = chip::Controller::DeviceControllerFactory; CHIP_ERROR CHIPCommand::Run() { + chip::Controller::ExampleOperationalCredentialsIssuer opCredsIssuer; + #if CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE // By default, Linux device is configured as a BLE peripheral while the controller needs a BLE central. ReturnLogErrorOnFailure(chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(0, true)); #endif ReturnLogErrorOnFailure(mStorage.Init()); - ReturnLogErrorOnFailure(mOpCredsIssuer.Initialize(mStorage)); + ReturnLogErrorOnFailure(opCredsIssuer.Initialize(mStorage)); ReturnLogErrorOnFailure(mFabricStorage.Initialize(&mStorage)); chip::Platform::ScopedMemoryBuffer noc; @@ -64,7 +67,7 @@ CHIP_ERROR CHIPCommand::Run() // TODO - OpCreds should only be generated for pairing command // store the credentials in persistent storage, and // generate when not available in the storage. - ReturnLogErrorOnFailure(mOpCredsIssuer.GenerateNOCChainAfterValidation(mStorage.GetLocalNodeId(), mStorage.GetFabricId(), + ReturnLogErrorOnFailure(opCredsIssuer.GenerateNOCChainAfterValidation(mStorage.GetLocalNodeId(), mStorage.GetFabricId(), ephemeralKey.Pubkey(), rcacSpan, icacSpan, nocSpan)); chip::Controller::FactoryInitParams factoryInitParams; @@ -73,7 +76,7 @@ CHIP_ERROR CHIPCommand::Run() chip::Controller::SetupParams commissionerParams; commissionerParams.storageDelegate = &mStorage; - commissionerParams.operationalCredentialsDelegate = &mOpCredsIssuer; + commissionerParams.operationalCredentialsDelegate = &opCredsIssuer; commissionerParams.ephemeralKeypair = &ephemeralKey; commissionerParams.controllerRCAC = rcacSpan; commissionerParams.controllerICAC = icacSpan; diff --git a/examples/chip-tool/commands/common/CHIPCommand.h b/examples/chip-tool/commands/common/CHIPCommand.h index 51f3aab0994948..eeeb95c8460298 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.h +++ b/examples/chip-tool/commands/common/CHIPCommand.h @@ -70,7 +70,6 @@ class CHIPCommand : public Command static void RunQueuedCommand(intptr_t commandArg); CHIP_ERROR mCommandExitStatus = CHIP_ERROR_INTERNAL; - chip::Controller::ExampleOperationalCredentialsIssuer mOpCredsIssuer; CHIP_ERROR StartWaiting(chip::System::Clock::Timeout seconds); void StopWaiting(); diff --git a/examples/chip-tool/commands/common/Command.h b/examples/chip-tool/commands/common/Command.h index 9cbac4ba0c6ade..1857db7f737f45 100644 --- a/examples/chip-tool/commands/common/Command.h +++ b/examples/chip-tool/commands/common/Command.h @@ -18,7 +18,6 @@ #pragma once -#include "controller/ExampleOperationalCredentialsIssuer.h" #include #include #include diff --git a/examples/chip-tool/commands/common/Commands.h b/examples/chip-tool/commands/common/Commands.h index cc1d027ba987b7..90dabbe33f6dc7 100644 --- a/examples/chip-tool/commands/common/Commands.h +++ b/examples/chip-tool/commands/common/Commands.h @@ -20,7 +20,6 @@ #include "../../config/PersistentStorage.h" #include "Command.h" -#include #include class Commands diff --git a/examples/chip-tool/commands/discover/DiscoverCommand.h b/examples/chip-tool/commands/discover/DiscoverCommand.h index 6e651a5fc94c8a..67d34b086b62ad 100644 --- a/examples/chip-tool/commands/discover/DiscoverCommand.h +++ b/examples/chip-tool/commands/discover/DiscoverCommand.h @@ -20,7 +20,6 @@ #include "../../config/PersistentStorage.h" #include "../common/CHIPCommand.h" -#include class DiscoverCommand : public CHIPCommand, public chip::Controller::DeviceAddressUpdateDelegate { diff --git a/examples/chip-tool/commands/pairing/PairingCommand.h b/examples/chip-tool/commands/pairing/PairingCommand.h index 2dc38bac1c35af..e481163ded7c95 100644 --- a/examples/chip-tool/commands/pairing/PairingCommand.h +++ b/examples/chip-tool/commands/pairing/PairingCommand.h @@ -23,7 +23,6 @@ #include #include -#include #include #include #include diff --git a/examples/chip-tool/commands/reporting/ReportingCommand.h b/examples/chip-tool/commands/reporting/ReportingCommand.h index 4547f256926167..232f831ba0c87b 100644 --- a/examples/chip-tool/commands/reporting/ReportingCommand.h +++ b/examples/chip-tool/commands/reporting/ReportingCommand.h @@ -21,8 +21,6 @@ #include "../../config/PersistentStorage.h" #include "../common/CHIPCommand.h" -#include - class ReportingCommand : public CHIPCommand { public: diff --git a/examples/chip-tool/commands/tests/TestCommand.h b/examples/chip-tool/commands/tests/TestCommand.h index 76206a324267c9..b67dbbee290630 100644 --- a/examples/chip-tool/commands/tests/TestCommand.h +++ b/examples/chip-tool/commands/tests/TestCommand.h @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include From 9d5506d617b515d907cbd57c4fbc29f7fe18bbef Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Mon, 29 Nov 2021 22:06:27 +0000 Subject: [PATCH 2/4] Restyled by clang-format --- examples/chip-tool/commands/common/CHIPCommand.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index d4960769600fac..d4209da6c13548 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -18,8 +18,8 @@ #include "CHIPCommand.h" -#include #include "controller/ExampleOperationalCredentialsIssuer.h" +#include #include #include #include @@ -68,7 +68,7 @@ CHIP_ERROR CHIPCommand::Run() // store the credentials in persistent storage, and // generate when not available in the storage. ReturnLogErrorOnFailure(opCredsIssuer.GenerateNOCChainAfterValidation(mStorage.GetLocalNodeId(), mStorage.GetFabricId(), - ephemeralKey.Pubkey(), rcacSpan, icacSpan, nocSpan)); + ephemeralKey.Pubkey(), rcacSpan, icacSpan, nocSpan)); chip::Controller::FactoryInitParams factoryInitParams; factoryInitParams.fabricStorage = &mFabricStorage; From bdf30eeb03d4b6770d8e47041c52adec9dd408ef Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Tue, 30 Nov 2021 17:07:38 -0500 Subject: [PATCH 3/4] Apply review comment --- examples/chip-tool/commands/common/CHIPCommand.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index 837d7a88a4c194..243cce749bab16 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -18,8 +18,8 @@ #include "CHIPCommand.h" -#include "controller/ExampleOperationalCredentialsIssuer.h" #include +#include #include #include #include From e9ceb3fe08a6d14b0fe3aea28eb29f495be9c29b Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Wed, 1 Dec 2021 14:19:27 -0500 Subject: [PATCH 4/4] Fix merge issue --- examples/chip-tool/commands/common/CHIPCommand.cpp | 9 +++------ examples/chip-tool/commands/common/CHIPCommand.h | 3 +++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/chip-tool/commands/common/CHIPCommand.cpp b/examples/chip-tool/commands/common/CHIPCommand.cpp index 243cce749bab16..f45a88bcaec6a8 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.cpp +++ b/examples/chip-tool/commands/common/CHIPCommand.cpp @@ -19,7 +19,6 @@ #include "CHIPCommand.h" #include -#include #include #include #include @@ -40,8 +39,6 @@ constexpr chip::FabricId kCommissionerGammaFabricId = 3; CHIP_ERROR CHIPCommand::Run() { - chip::Controller::ExampleOperationalCredentialsIssuer opCredsIssuer; - #if CHIP_DEVICE_LAYER_TARGET_LINUX && CHIP_DEVICE_CONFIG_ENABLE_CHIPOBLE // By default, Linux device is configured as a BLE peripheral while the controller needs a BLE central. ReturnLogErrorOnFailure(chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(0, true)); @@ -146,14 +143,14 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(std::string key, chip::FabricId f // store the credentials in persistent storage, and // generate when not available in the storage. ReturnLogErrorOnFailure(mCommissionerStorage.Init(key.c_str())); - ReturnLogErrorOnFailure(opCredsIssuer.Initialize(mCommissionerStorage)); - ReturnLogErrorOnFailure(opCredsIssuer.GenerateNOCChainAfterValidation(mCommissionerStorage.GetLocalNodeId(), fabricId, + ReturnLogErrorOnFailure(mOpCredsIssuer.Initialize(mCommissionerStorage)); + ReturnLogErrorOnFailure(mOpCredsIssuer.GenerateNOCChainAfterValidation(mCommissionerStorage.GetLocalNodeId(), fabricId, ephemeralKey.Pubkey(), rcacSpan, icacSpan, nocSpan)); std::unique_ptr commissioner = std::make_unique(); chip::Controller::SetupParams commissionerParams; commissionerParams.storageDelegate = &mCommissionerStorage; - commissionerParams.operationalCredentialsDelegate = &opCredsIssuer; + commissionerParams.operationalCredentialsDelegate = &mOpCredsIssuer; commissionerParams.ephemeralKeypair = &ephemeralKey; commissionerParams.controllerRCAC = rcacSpan; commissionerParams.controllerICAC = icacSpan; diff --git a/examples/chip-tool/commands/common/CHIPCommand.h b/examples/chip-tool/commands/common/CHIPCommand.h index 14195711e1a6ab..c28bf9b7e5e261 100644 --- a/examples/chip-tool/commands/common/CHIPCommand.h +++ b/examples/chip-tool/commands/common/CHIPCommand.h @@ -18,6 +18,8 @@ #pragma once +#include + #include "../../config/PersistentStorage.h" #include "Command.h" @@ -65,6 +67,7 @@ class CHIPCommand : public Command PersistentStorage mDefaultStorage; PersistentStorage mCommissionerStorage; chip::SimpleFabricStorage mFabricStorage; + chip::Controller::ExampleOperationalCredentialsIssuer mOpCredsIssuer; // This method returns the commissioner instance to be used for running the command. // The default commissioner instance name is "alpha", but it can be overriden by passing