From c0fe4b57bfb654b2aec8d4ea9aa7c8c12f29b991 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Thu, 28 Aug 2025 11:00:26 +0800 Subject: [PATCH 1/2] revert commit c91d9d5a `components/esp_matter: resize group data provider when ep count change` --- .../data_model/esp_matter_data_model.cpp | 45 ------------------- components/esp_matter/esp_matter_core.cpp | 16 +++++++ 2 files changed, 16 insertions(+), 45 deletions(-) diff --git a/components/esp_matter/data_model/esp_matter_data_model.cpp b/components/esp_matter/data_model/esp_matter_data_model.cpp index 754f85b21..d7b30ba13 100644 --- a/components/esp_matter/data_model/esp_matter_data_model.cpp +++ b/components/esp_matter/data_model/esp_matter_data_model.cpp @@ -31,7 +31,6 @@ #include "credentials/GroupDataProviderImpl.h" #define ESP_MATTER_MAX_DEVICE_TYPE_COUNT CONFIG_ESP_MATTER_MAX_DEVICE_TYPE_COUNT -#define MAX_GROUPS_PER_FABRIC_PER_ENDPOINT CONFIG_MAX_GROUPS_PER_FABRIC_PER_ENDPOINT static const char *TAG = "data_model"; @@ -291,44 +290,6 @@ static int get_next_index() return 0xFFFF; } -// global instance so that we can reset it when needed. -// We may need to reset it when new endpoint is added or existing endpoint is removed. -// This is specifically for bridged device. -static chip::Credentials::GroupDataProviderImpl *s_group_data_provider = nullptr; -static uint16_t s_groups_server_cluster_count = 0; - -static void resize_group_data_provider() -{ - // don't do anything if the count is the same - uint16_t groups_server_cluster_count = node::get_server_cluster_endpoint_count(chip::app::Clusters::Groups::Id); - if (s_groups_server_cluster_count == groups_server_cluster_count) { - return; - } - - s_groups_server_cluster_count = groups_server_cluster_count; - uint16_t max_groups_per_fabric = s_groups_server_cluster_count * MAX_GROUPS_PER_FABRIC_PER_ENDPOINT; - auto group_data_provider = new (std::nothrow) chip::Credentials::GroupDataProviderImpl(max_groups_per_fabric, CHIP_CONFIG_MAX_GROUP_KEYS_PER_FABRIC); - if (!group_data_provider) { - ESP_LOGE(TAG, "Failed to allocate memory for group data provider"); - return; - } - - group_data_provider->SetStorageDelegate(&chip::Server::GetInstance().GetPersistentStorage()); - group_data_provider->SetSessionKeystore(chip::Server::GetInstance().GetSessionKeystore()); - - // As we are re-using the persistent storage instance from the Server class instance, - // which has all the data from the previous endpoints, so no harm in re-sizing. - group_data_provider->Init(); - - // delete the old one if it exists - if (s_group_data_provider) { - delete s_group_data_provider; - } - - s_group_data_provider = group_data_provider; - chip::Credentials::SetGroupDataProvider(s_group_data_provider); -} - static esp_err_t disable(endpoint_t *endpoint) { /* Take lock if not already taken */ @@ -568,9 +529,6 @@ esp_err_t enable(endpoint_t *endpoint) } ESP_LOGI(TAG, "Dynamic endpoint %" PRIu16 " added", current_endpoint->endpoint_id); - // resize the group data provider to match the new endpoint count - resize_group_data_provider(); - return err; cleanup: @@ -1566,9 +1524,6 @@ esp_err_t destroy(node_t *node, endpoint_t *endpoint) } esp_matter_mem_free(current_endpoint); - // resize the group data provider to match the new endpoint count - resize_group_data_provider(); - return ESP_OK; } diff --git a/components/esp_matter/esp_matter_core.cpp b/components/esp_matter/esp_matter_core.cpp index 1e56eba03..876b799d8 100644 --- a/components/esp_matter/esp_matter_core.cpp +++ b/components/esp_matter/esp_matter_core.cpp @@ -65,6 +65,8 @@ using chip::DeviceLayer::GetDiagnosticDataProvider; using chip::DeviceLayer::ThreadStackMgr; #endif +#define MAX_GROUPS_PER_FABRIC_PER_ENDPOINT CONFIG_MAX_GROUPS_PER_FABRIC_PER_ENDPOINT + static const char *TAG = "esp_matter_core"; static bool esp_matter_started = false; @@ -219,6 +221,20 @@ static void esp_matter_chip_init_task(intptr_t context) initParams.testEventTriggerDelegate = test_event_trigger::get_delegate(); initParams.dataModelProvider = chip::app::CodegenDataModelProviderInstance(initParams.persistentStorageDelegate); +#ifdef CONFIG_ESP_MATTER_ENABLE_DATA_MODEL + { + // We should reserve one endpoint for root node endpoint + uint8_t max_groups_server_cluster_count = CONFIG_ESP_MATTER_MAX_DYNAMIC_ENDPOINT_COUNT - 1; + uint16_t max_groups_per_fabric = max_groups_server_cluster_count * MAX_GROUPS_PER_FABRIC_PER_ENDPOINT; + static chip::Credentials::GroupDataProviderImpl groupDataProvider(max_groups_per_fabric, + CHIP_CONFIG_MAX_GROUP_KEYS_PER_FABRIC); + groupDataProvider.SetStorageDelegate(initParams.persistentStorageDelegate); + groupDataProvider.SetSessionKeystore(initParams.sessionKeystore); + groupDataProvider.Init(); + initParams.groupDataProvider = &groupDataProvider; + } +#endif // CONFIG_ESP_MATTER_ENABLE_DATA_MODEL + CHIP_ERROR ret = chip::Server::GetInstance().GetFabricTable().AddFabricDelegate(&s_fabric_delegate); if (ret != CHIP_NO_ERROR) { From d128385607d8caea3864a9709a8f185aa9f62ac2 Mon Sep 17 00:00:00 2001 From: WanqQixiang Date: Thu, 28 Aug 2025 12:04:39 +0800 Subject: [PATCH 2/2] esp_matter_controller: Set Listener for Group Data Provider of Controller --- .../core/esp_matter_controller_client.cpp | 3 +- .../core/esp_matter_controller_client.h | 80 ++++++++++++++++--- 2 files changed, 70 insertions(+), 13 deletions(-) diff --git a/components/esp_matter_controller/core/esp_matter_controller_client.cpp b/components/esp_matter_controller/core/esp_matter_controller_client.cpp index e13579ede..4cca969bb 100644 --- a/components/esp_matter_controller/core/esp_matter_controller_client.cpp +++ b/components/esp_matter_controller/core/esp_matter_controller_client.cpp @@ -76,6 +76,7 @@ esp_err_t matter_controller_client::init(NodeId node_id, FabricId fabric_id, uin m_group_data_provider.SetStorageDelegate(&m_default_storage); m_group_data_provider.SetSessionKeystore(factory_init_params.sessionKeystore); + m_group_data_provider.SetListener(&m_group_data_provider_listener); ESP_RETURN_ON_FALSE(m_group_data_provider.Init() == CHIP_NO_ERROR, ESP_FAIL, TAG, "Failed to initialize group data provider"); factory_init_params.groupDataProvider = @@ -102,7 +103,7 @@ esp_err_t matter_controller_client::setup_commissioner() CHIP_ERROR err = chip::DeviceLayer::Internal::BLEMgr().Init(); // This function will return CHIP_ERROR_INCORRECT_STATE if BLE Manager is already initialized. ESP_RETURN_ON_FALSE(err == CHIP_NO_ERROR || err == CHIP_ERROR_INCORRECT_STATE, ESP_FAIL, TAG, - "Failed to initialze the BLE manager"); + "Failed to initialize the BLE manager"); ESP_RETURN_ON_FALSE(chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureBle(0, true) == CHIP_NO_ERROR, ESP_FAIL, TAG, "Failed to configure BLEManager"); #endif diff --git a/components/esp_matter_controller/core/esp_matter_controller_client.h b/components/esp_matter_controller/core/esp_matter_controller_client.h index 60fbdea75..ffb731b87 100644 --- a/components/esp_matter_controller/core/esp_matter_controller_client.h +++ b/components/esp_matter_controller/core/esp_matter_controller_client.h @@ -17,9 +17,9 @@ #include #include -#include -#include #include +#include +#include #include #include #include @@ -51,19 +51,17 @@ namespace controller { #ifndef CONFIG_ESP_MATTER_ENABLE_MATTER_SERVER typedef void (*remove_fabric_callback)(chip::NodeId remoteNodeId, CHIP_ERROR status); -class auto_fabric_remover : private chip::Controller::CurrentFabricRemover -{ +class auto_fabric_remover : private chip::Controller::CurrentFabricRemover { public: static esp_err_t remove_fabric(chip::Controller::DeviceController *controller, chip::NodeId remote_node, remove_fabric_callback callback) { - auto * remover = chip::Platform::New(controller, callback); + auto *remover = chip::Platform::New(controller, callback); if (remover == nullptr) { return ESP_ERR_NO_MEM; } CHIP_ERROR err = remover->CurrentFabricRemover::RemoveCurrentFabric(remote_node, &remover->m_matter_callback); - if (err != CHIP_NO_ERROR) - { + if (err != CHIP_NO_ERROR) { // If failing to call RemoveCurrentFabric(), delete the remover here. Otherwise the remover will be deleted // in on_remove_current_fabric(). chip::Platform::Delete(remover); @@ -71,11 +69,13 @@ public: return err == CHIP_NO_ERROR ? ESP_OK : ESP_FAIL; } - auto_fabric_remover(chip::Controller::DeviceController *controller, remove_fabric_callback callback) : - chip::Controller::CurrentFabricRemover(controller), m_matter_callback(on_remove_current_fabric, this), - m_remove_fabric_callback(callback) {} + auto_fabric_remover(chip::Controller::DeviceController *controller, remove_fabric_callback callback) + : chip::Controller::CurrentFabricRemover(controller) + , m_matter_callback(on_remove_current_fabric, this) + , m_remove_fabric_callback(callback) {} + private: - static void on_remove_current_fabric(void * context, chip::NodeId remote_node, CHIP_ERROR status) + static void on_remove_current_fabric(void *context, chip::NodeId remote_node, CHIP_ERROR status) { auto *self = static_cast(context); if (self && self->m_remove_fabric_callback) { @@ -157,6 +157,59 @@ public: } private: + class GroupDataProviderListener final : public chip::Credentials::GroupDataProvider::GroupListener { + public: + GroupDataProviderListener() {} + + CHIP_ERROR Init(chip::Controller::DeviceControllerSystemState *systemState) + { + VerifyOrReturnError(systemState != nullptr, CHIP_ERROR_INVALID_ARGUMENT); + + mSystemState = systemState; + return CHIP_NO_ERROR; + }; + + void OnGroupAdded(chip::FabricIndex fabric_index, + const chip::Credentials::GroupDataProvider::GroupInfo &new_group) override + { + auto *fabricTable = mSystemState->Fabrics(); + if (!fabricTable) { + return; + } + const chip::FabricInfo *fabric = fabricTable->FindFabricWithIndex(fabric_index); + if (fabric == nullptr) { + ChipLogError(AppServer, "Group added to nonexistent fabric?"); + return; + } + + if (mSystemState->TransportMgr()->MulticastGroupJoinLeave( + chip::Transport::PeerAddress::Multicast(fabric->GetFabricId(), new_group.group_id), true) != + CHIP_NO_ERROR) { + ChipLogError(AppServer, "Unable to listen to group"); + } + }; + + void OnGroupRemoved(chip::FabricIndex fabric_index, + const chip::Credentials::GroupDataProvider::GroupInfo &old_group) override + { + auto *fabricTable = mSystemState->Fabrics(); + if (!fabricTable) { + return; + } + const chip::FabricInfo *fabric = fabricTable->FindFabricWithIndex(fabric_index); + if (fabric == nullptr) { + ChipLogError(AppServer, "Group removed from nonexistent fabric?"); + return; + } + + mSystemState->TransportMgr()->MulticastGroupJoinLeave( + chip::Transport::PeerAddress::Multicast(fabric->GetFabricId(), old_group.group_id), false); + }; + + private: + chip::Controller::DeviceControllerSystemState *mSystemState; + }; + matter_controller_client() {} bool m_operational_advertising = true; @@ -166,6 +219,7 @@ private: chip::Crypto::RawKeySessionKeystore m_session_key_store; chip::Credentials::GroupDataProviderImpl m_group_data_provider{k_max_groups_per_fabric, k_max_group_keys_per_fabric}; + GroupDataProviderListener m_group_data_provider_listener; credentials_issuer *m_credentials_issuer; NodeId m_controller_node_id; FabricId m_controller_fabric_id; @@ -191,7 +245,9 @@ class ESPCommissionerCallback : public CommissionerCallback { { NodeId gRemoteId = chip::kTestDeviceNodeId; chip::RendezvousParameters params = chip::RendezvousParameters() - .SetSetupPINCode(pincode).SetDiscriminator(longDiscriminator).SetPeerAddress(peerAddress); + .SetSetupPINCode(pincode) + .SetDiscriminator(longDiscriminator) + .SetPeerAddress(peerAddress); do { chip::Crypto::DRBG_get_bytes(reinterpret_cast(&gRemoteId), sizeof(gRemoteId)); } while (!chip::IsOperationalNodeId(gRemoteId));