Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused per-controller storage delegate #17258

1 change: 0 additions & 1 deletion examples/chip-tool/commands/common/CHIPCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@ CHIP_ERROR CHIPCommand::InitializeCommissioner(std::string key, chip::FabricId f
commissionerParams.controllerNOC = nocSpan;
}

commissionerParams.storageDelegate = &mCommissionerStorage;
// TODO: Initialize IPK epoch key in ExampleOperationalCredentials issuer rather than relying on DefaultIpkValue
commissionerParams.operationalCredentialsDelegate = mCredIssuerCmds->GetCredentialIssuer();
commissionerParams.controllerVendorId = chip::VendorId::TestVendor1;
Expand Down
15 changes: 5 additions & 10 deletions examples/chip-tool/commands/pairing/CommissionedListCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,8 @@ CHIP_ERROR CommissionedListCommand::PrintInformation()
uint16_t pairedNodesIdsSize = sizeof(pairedNodesIds);
memset(pairedNodesIds, 0, pairedNodesIdsSize);

PERSISTENT_KEY_OP(static_cast<uint64_t>(0), chip::kPairedDeviceListKeyPrefix, key,
ReturnLogErrorOnFailure(mStorage.SyncGetKeyValue(key, pairedNodesIds, pairedNodesIdsSize)));

chip::SerializableU64Set<chip::Controller::kNumMaxPairedDevices> devices;
devices.Deserialize(chip::ByteSpan((uint8_t *) pairedNodesIds, pairedNodesIdsSize));

// TODO: Get the list of paired node IDs. chip-tool needs to store that as
// devices get paired.
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
uint16_t pairedDevicesCount = 0;
while (pairedNodesIds[pairedDevicesCount] != 0x0 && pairedDevicesCount < chip::Controller::kNumMaxPairedDevices)
{
Expand Down Expand Up @@ -69,13 +65,12 @@ CHIP_ERROR CommissionedListCommand::PrintInformation()

CHIP_ERROR CommissionedListCommand::PrintDeviceInformation(chip::NodeId deviceId)
{
// TODO: Controller::SerializedDevice and Controller::SerializableDevice are
// gone. Need to figure out what chip-tool should actually store/retrieve
// here.
chip::Controller::SerializedDevice deviceInfo;
uint16_t size = sizeof(deviceInfo.inner);

PERSISTENT_KEY_OP(deviceId, chip::kPairedDeviceKeyPrefix, key,
ReturnLogErrorOnFailure(mStorage.SyncGetKeyValue(key, deviceInfo.inner, size)));
VerifyOrReturnError(size <= sizeof(deviceInfo.inner), CHIP_ERROR_INVALID_DEVICE_DESCRIPTOR);

chip::Controller::SerializableDevice serializable;
constexpr size_t maxlen = BASE64_ENCODED_LEN(sizeof(serializable));
const size_t len = strnlen(chip::Uint8::to_const_char(&deviceInfo.inner[0]), maxlen);
Expand Down
1 change: 0 additions & 1 deletion examples/platform/linux/AppMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,6 @@ CHIP_ERROR InitCommissioner()
ReturnErrorOnFailure(gGroupDataProvider.Init());
factoryParams.groupDataProvider = &gGroupDataProvider;

params.storageDelegate = &gServerStorage;
params.operationalCredentialsDelegate = &gOpCredsIssuer;

ReturnErrorOnFailure(gOpCredsIssuer.Initialize(gServerStorage));
Expand Down
84 changes: 1 addition & 83 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ constexpr uint32_t kSessionEstablishmentTimeout = 40 * kMillisecondsPerSecond;

DeviceController::DeviceController()
{
mState = State::NotInitialized;
mStorageDelegate = nullptr;
mPairedDevicesInitialized = false;
mState = State::NotInitialized;
}

CHIP_ERROR DeviceController::Init(ControllerInitParams params)
Expand All @@ -118,7 +116,6 @@ CHIP_ERROR DeviceController::Init(ControllerInitParams params)
VerifyOrReturnError(params.systemState->SystemLayer() != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(params.systemState->UDPEndPointManager() != nullptr, CHIP_ERROR_INVALID_ARGUMENT);

mStorageDelegate = params.storageDelegate;
#if CONFIG_NETWORK_LAYER_BLE
VerifyOrReturnError(params.systemState->BleLayer() != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
#endif
Expand Down Expand Up @@ -231,8 +228,6 @@ CHIP_ERROR DeviceController::Shutdown()
mSystemState->SessionMgr()->ExpireAllPairingsForFabric(mFabricInfo->GetFabricIndex());
}

mStorageDelegate = nullptr;

if (mFabricInfo != nullptr)
{
mFabricInfo->Reset();
Expand All @@ -246,16 +241,6 @@ CHIP_ERROR DeviceController::Shutdown()
return CHIP_NO_ERROR;
}

bool DeviceController::DoesDevicePairingExist(const PeerId & deviceId)
{
if (InitializePairedDeviceList() == CHIP_NO_ERROR)
{
return mPairedDevices.Contains(deviceId.GetNodeId());
}

return false;
}

void DeviceController::ReleaseOperationalDevice(NodeId remoteDeviceId)
{
VerifyOrReturn(mState == State::Initialized && mFabricInfo != nullptr,
Expand Down Expand Up @@ -329,64 +314,6 @@ void DeviceController::OnFirstMessageDeliveryFailed(const SessionHandle & sessio
}
}

CHIP_ERROR DeviceController::InitializePairedDeviceList()
{
CHIP_ERROR err = CHIP_NO_ERROR;
uint8_t * buffer = nullptr;

VerifyOrExit(mStorageDelegate != nullptr, err = CHIP_ERROR_INCORRECT_STATE);

if (!mPairedDevicesInitialized)
{
constexpr uint16_t max_size = sizeof(uint64_t) * kNumMaxPairedDevices;
buffer = static_cast<uint8_t *>(chip::Platform::MemoryCalloc(max_size, 1));
uint16_t size = max_size;

VerifyOrExit(buffer != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);

CHIP_ERROR lookupError = CHIP_NO_ERROR;
PERSISTENT_KEY_OP(static_cast<uint64_t>(0), kPairedDeviceListKeyPrefix, key,
lookupError = mStorageDelegate->SyncGetKeyValue(key, buffer, size));

// It's ok to not have an entry for the Paired Device list. We treat it the same as having an empty list.
if (lookupError != CHIP_ERROR_KEY_NOT_FOUND)
{
VerifyOrExit(size <= max_size, err = CHIP_ERROR_INVALID_DEVICE_DESCRIPTOR);
err = SetPairedDeviceList(ByteSpan(buffer, size));
SuccessOrExit(err);
}
}

exit:
if (buffer != nullptr)
{
chip::Platform::MemoryFree(buffer);
}
if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed to initialize the device list with error: %" CHIP_ERROR_FORMAT, err.Format());
}

return err;
}

CHIP_ERROR DeviceController::SetPairedDeviceList(ByteSpan serialized)
{
CHIP_ERROR err = mPairedDevices.Deserialize(serialized);

if (err != CHIP_NO_ERROR)
{
ChipLogError(Controller, "Failed to recreate the device list with buffer %.*s\n", static_cast<int>(serialized.size()),
serialized.data());
}
else
{
mPairedDevicesInitialized = true;
}

return err;
}

CHIP_ERROR DeviceController::GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port)
{
VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
Expand All @@ -412,7 +339,6 @@ ControllerDeviceInitParams DeviceController::GetControllerDeviceInitParams()
.sessionManager = mSystemState->SessionMgr(),
.exchangeMgr = mSystemState->ExchangeMgr(),
.udpEndPointManager = mSystemState->UDPEndPointManager(),
.storageDelegate = mStorageDelegate,
.fabricsTable = mSystemState->Fabrics(),
};
}
Expand All @@ -423,7 +349,6 @@ DeviceCommissioner::DeviceCommissioner() :
mDeviceNOCChainCallback(OnDeviceNOCChainGeneration, this), mSetUpCodePairer(this)
{
mPairingDelegate = nullptr;
mPairedDevicesUpdated = false;
mDeviceBeingCommissioned = nullptr;
mDeviceInPASEEstablishment = nullptr;
}
Expand Down Expand Up @@ -632,10 +557,6 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
VerifyOrExit(mState == State::Initialized, err = CHIP_ERROR_INCORRECT_STATE);
VerifyOrExit(mDeviceInPASEEstablishment == nullptr, err = CHIP_ERROR_INCORRECT_STATE);

// This will initialize the commissionee device pool if it has not already been initialized.
err = InitializePairedDeviceList();
SuccessOrExit(err);

// TODO(#13940): We need to specify the peer address for BLE transport in bindings.
if (params.GetPeerAddress().GetTransportType() == Transport::Type::kBle ||
params.GetPeerAddress().GetTransportType() == Transport::Type::kUndefined)
Expand Down Expand Up @@ -1380,9 +1301,6 @@ CHIP_ERROR DeviceCommissioner::OnOperationalCredentialsProvisioningCompletion(De

mSystemState->SystemLayer()->CancelTimer(OnSessionEstablishmentTimeoutCallback, this);

mPairedDevices.Insert(device->GetDeviceId());
mPairedDevicesUpdated = true;

if (mPairingDelegate != nullptr)
{
mPairingDelegate->OnStatusUpdate(DevicePairingDelegate::SecurePairingSuccess);
Expand Down
19 changes: 0 additions & 19 deletions src/controller/CHIPDeviceController.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,12 @@ namespace Controller {
using namespace chip::Protocols::UserDirectedCommissioning;

constexpr uint16_t kNumMaxActiveDevices = CHIP_CONFIG_CONTROLLER_MAX_ACTIVE_DEVICES;
constexpr uint16_t kNumMaxPairedDevices = 128;

// Raw functions for cluster callbacks
void OnBasicFailure(void * context, CHIP_ERROR err);

struct ControllerInitParams
{
PersistentStorageDelegate * storageDelegate = nullptr;
DeviceControllerSystemState * systemState = nullptr;
DeviceDiscoveryDelegate * deviceDiscoveryDelegate = nullptr;
OperationalCredentialsDelegate * operationalCredentialsDelegate = nullptr;
Expand Down Expand Up @@ -148,12 +146,6 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public Abstr

CHIP_ERROR GetPeerAddressAndPort(PeerId peerId, Inet::IPAddress & addr, uint16_t & port);

/**
* This function returns true if the device corresponding to `deviceId` has previously been commissioned
* on the fabric.
*/
bool DoesDevicePairingExist(const PeerId & deviceId);

/**
* This function finds the device corresponding to deviceId, and establishes a secure connection with it.
* Once the connection is successfully establishes (or if it's already connected), it calls `onConnectedDevice`
Expand Down Expand Up @@ -247,21 +239,15 @@ class DLL_EXPORT DeviceController : public SessionRecoveryDelegate, public Abstr

State mState;

SerializableU64Set<kNumMaxPairedDevices> mPairedDevices;
bool mPairedDevicesInitialized;

PeerId mLocalId = PeerId();
FabricId mFabricId = kUndefinedFabricId;
FabricInfo * mFabricInfo = nullptr;

PersistentStorageDelegate * mStorageDelegate = nullptr;
// TODO(cecille): Make this configuarable.
static constexpr int kMaxCommissionableNodes = 10;
Dnssd::DiscoveredNodeData mCommissionableNodes[kMaxCommissionableNodes];
DeviceControllerSystemState * mSystemState = nullptr;

CHIP_ERROR InitializePairedDeviceList();
CHIP_ERROR SetPairedDeviceList(ByteSpan pairedDeviceSerializedSet);
ControllerDeviceInitParams GetControllerDeviceInitParams();

OperationalCredentialsDelegate * mOperationalCredentialsDelegate;
Expand Down Expand Up @@ -575,11 +561,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
DeviceProxy * mDeviceBeingCommissioned = nullptr;
CommissioneeDeviceProxy * mDeviceInPASEEstablishment = nullptr;

/* This field is true when device pairing information changes, e.g. a new device is paired, or
the pairing for a device is removed. The DeviceCommissioner uses this to decide when to
persist the device list */
bool mPairedDevicesUpdated;

CommissioningStage mCommissioningStage = CommissioningStage::kSecurePairing;
bool mRunCommissioningAfterConnection = false;

Expand Down
1 change: 0 additions & 1 deletion src/controller/CHIPDeviceControllerFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ void DeviceControllerFactory::PopulateInitParams(ControllerInitParams & controll
controllerParams.controllerNOC = params.controllerNOC;
controllerParams.controllerICAC = params.controllerICAC;
controllerParams.controllerRCAC = params.controllerRCAC;
controllerParams.storageDelegate = params.storageDelegate;

controllerParams.systemState = mSystemState;
controllerParams.controllerVendorId = params.controllerVendorId;
Expand Down
2 changes: 0 additions & 2 deletions src/controller/CHIPDeviceControllerFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ struct SetupParams
{
OperationalCredentialsDelegate * operationalCredentialsDelegate = nullptr;

PersistentStorageDelegate * storageDelegate = nullptr;

/* The following keypair must correspond to the public key used for generating
controllerNOC. It's used by controller to establish CASE sessions with devices */
Crypto::P256Keypair * operationalKeypair = nullptr;
Expand Down
5 changes: 1 addition & 4 deletions src/controller/CommissioneeDeviceProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ struct ControllerDeviceInitParams
SessionManager * sessionManager = nullptr;
Messaging::ExchangeManager * exchangeMgr = nullptr;
Inet::EndPointManager<Inet::UDPEndPoint> * udpEndPointManager = nullptr;
PersistentStorageDelegate * storageDelegate = nullptr;
#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer * bleLayer = nullptr;
#endif
Expand Down Expand Up @@ -168,8 +167,6 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
* Update data of the device.
*
* This function will set new IP address, port and MRP retransmission intervals of the device.
* Since the device settings might have been moved from RAM to the persistent storage, the function
* will load the device settings first, before making the changes.
*
* @param[in] addr Address of the device to be set.
* @param[in] config MRP parameters
Expand All @@ -190,7 +187,7 @@ class CommissioneeDeviceProxy : public DeviceProxy, public SessionReleaseDelegat
* @brief
* Called to indicate this proxy has been paired successfully.
*
* This causes the secure session parameters to be loaded and stores the session details in the session manager.
* This stores the session details in the session manager.
*/
CHIP_ERROR SetConnected();

Expand Down
5 changes: 2 additions & 3 deletions src/controller/java/AndroidDeviceControllerWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,11 @@ AndroidDeviceControllerWrapper::AllocateNew(JavaVM * vm, jobject deviceControlle
initParams.bleLayer = DeviceLayer::ConnectivityMgr().GetBleLayer();
#endif
initParams.listenPort = CHIP_PORT + 1;
setupParams.storageDelegate = wrapper.get();
setupParams.pairingDelegate = wrapper.get();
setupParams.operationalCredentialsDelegate = opCredsIssuer;
initParams.fabricIndependentStorage = setupParams.storageDelegate;
initParams.fabricIndependentStorage = wrapper.get();

wrapper->mGroupDataProvider.SetStorageDelegate(setupParams.storageDelegate);
wrapper->mGroupDataProvider.SetStorageDelegate(wrapper.get());

CHIP_ERROR err = wrapper->mGroupDataProvider.Init();
if (err != CHIP_NO_ERROR)
Expand Down
2 changes: 0 additions & 2 deletions src/controller/python/OpCredsBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ class OperationalCredentialsAdapter : public OperationalCredentialsDelegate
} // namespace chip

extern chip::Controller::Python::StorageAdapter * pychip_Storage_GetStorageAdapter();
extern chip::Controller::Python::StorageAdapter * sStorageAdapter;
extern chip::Credentials::GroupDataProviderImpl sGroupDataProvider;
extern chip::Controller::ScriptDevicePairingDelegate sPairingDelegate;

Expand Down Expand Up @@ -362,7 +361,6 @@ ChipError::StorageType pychip_OpCreds_AllocateController(OpCredsContext * contex
VerifyOrReturnError(err == CHIP_NO_ERROR, err.AsInteger());

Controller::SetupParams initParams;
initParams.storageDelegate = sStorageAdapter;
initParams.pairingDelegate = &sPairingDelegate;
initParams.operationalCredentialsDelegate = context->mAdapter.get();
initParams.operationalKeypair = &ephemeralKey;
Expand Down
1 change: 0 additions & 1 deletion src/controller/python/chip/internal/CommissionerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ extern "C" chip::Controller::DeviceCommissioner * pychip_internal_Commissioner_N
factoryParams.groupDataProvider = &gGroupDataProvider;

commissionerParams.pairingDelegate = &gPairingDelegate;
commissionerParams.storageDelegate = &gServerStorage;

err = ephemeralKey.Initialize();
SuccessOrExit(err);
Expand Down
1 change: 0 additions & 1 deletion src/darwin/Framework/CHIP/CHIPDeviceController.mm
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ - (BOOL)startup:(_Nullable id<CHIPPersistentStorageDelegate>)storageDelegate

params.groupDataProvider = _groupDataProvider;
params.fabricIndependentStorage = _persistentStorageDelegateBridge;
commissionerParams.storageDelegate = _persistentStorageDelegateBridge;
commissionerParams.pairingDelegate = _pairingDelegateBridge;

commissionerParams.operationalCredentialsDelegate = _operationalCredentialsDelegate;
Expand Down
3 changes: 0 additions & 3 deletions src/lib/support/PersistentStorageMacros.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@

namespace chip {

constexpr const char kPairedDeviceListKeyPrefix[] = "ListPairedDevices";
constexpr const char kPairedDeviceKeyPrefix[] = "PairedDevice";

// This macro generates a key for storage using a node ID and a key prefix, and performs the given action
// on that key.
#define PERSISTENT_KEY_OP(node, keyPrefix, key, action) \
Expand Down