From 4cb44c6c5caa1757d4d8a7f237df542608df5d5e Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 12:54:11 -0400 Subject: [PATCH 1/9] Add separate listing for device types --- .../CodegenDataModelProvider.cpp | 75 +++++++++++++++++++ .../CodegenDataModelProvider.h | 11 ++- src/app/data-model-provider/MetadataTypes.h | 12 +++ src/app/tests/test-interaction-model-api.cpp | 11 +++ src/app/tests/test-interaction-model-api.h | 3 + src/app/util/attribute-storage.cpp | 8 +- src/app/util/attribute-storage.h | 1 + src/app/util/mock/attribute-storage.cpp | 12 +++ .../tests/data_model/DataModelFixtures.cpp | 11 +++ .../tests/data_model/DataModelFixtures.h | 3 + 10 files changed, 141 insertions(+), 6 deletions(-) diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp index aa8e2e20d9cc8d..d5b2e6eed1c2a0 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp @@ -14,6 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include "app/util/af-types.h" #include #include @@ -286,6 +287,21 @@ std::optional EnumeratorCommandFinder::FindCommandEntry return (*id == kInvalidCommandId) ? DataModel::CommandEntry::kInvalid : CommandEntryFrom(path, *id); } +DataModel::DeviceTypeEntry DeviceTypeEntryFromEmber(const EmberAfDeviceType & other) +{ + DataModel::DeviceTypeEntry entry; + + entry.deviceTypeId = other.deviceId; + entry.deviceTypeVersion = other.deviceVersion; + + return entry; +} + +bool IsSameDeviceTypeEntry(const DataModel::DeviceTypeEntry & a, const EmberAfDeviceType & b) +{ + return (a.deviceTypeId == b.deviceId) && (a.deviceTypeVersion == b.deviceVersion); +} + const ConcreteCommandPath kInvalidCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId); } // namespace @@ -720,5 +736,64 @@ ConcreteCommandPath CodegenDataModelProvider::NextGeneratedCommand(const Concret return ConcreteCommandPath(before.mEndpointId, before.mClusterId, *commandId); } +std::optional CodegenDataModelProvider::FirstDeviceType(EndpointId endpoint) +{ + std::optional endpoint_index = TryFindEndpointIndex(endpoint); + if (!endpoint_index.has_value()) + { + return std::nullopt; + } + + CHIP_ERROR err = CHIP_NO_ERROR; + Span deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err); + + if (deviceTypes.empty()) + { + return std::nullopt; + } + + // we start at the beginning + mDeviceTypeIterationHint = 0; + return DeviceTypeEntryFromEmber(deviceTypes[0]); +} + +std::optional CodegenDataModelProvider::NextDeviceType(EndpointId endpoint, + const DataModel::DeviceTypeEntry & previous) +{ + std::optional endpoint_index = TryFindEndpointIndex(endpoint); + if (!endpoint_index.has_value()) + { + return std::nullopt; + } + + CHIP_ERROR err = CHIP_NO_ERROR; + Span deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err); + + unsigned idx = 0; + if ((mDeviceTypeIterationHint < deviceTypes.size()) && IsSameDeviceTypeEntry(previous, deviceTypes[mDeviceTypeIterationHint])) + { + idx = mDeviceTypeIterationHint + 1; // target the NEXT device + } + else + { + while (idx < deviceTypes.size()) + { + idx++; + if (IsSameDeviceTypeEntry(previous, deviceTypes[idx - 1])) + { + break; + } + } + } + + if (idx >= deviceTypes.size()) + { + return std::nullopt; + } + + mDeviceTypeIterationHint = idx; + return DeviceTypeEntryFromEmber(deviceTypes[idx]); +} + } // namespace app } // namespace chip diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h index 1462ce03c1d5a9..941d2ae5a9a774 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h @@ -95,6 +95,10 @@ class CodegenDataModelProvider : public chip::app::DataModel::Provider EndpointId FirstEndpoint() override; EndpointId NextEndpoint(EndpointId before) override; + std::optional FirstDeviceType(EndpointId endpoint) override; + std::optional NextDeviceType(EndpointId endpoint, + const DataModel::DeviceTypeEntry & previous) override; + DataModel::ClusterEntry FirstCluster(EndpointId endpoint) override; DataModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; std::optional GetClusterInfo(const ConcreteClusterPath & path) override; @@ -113,9 +117,10 @@ class CodegenDataModelProvider : public chip::app::DataModel::Provider private: // Iteration is often done in a tight loop going through all values. // To avoid N^2 iterations, cache a hint of where something is positioned - uint16_t mEndpointIterationHint = 0; - unsigned mClusterIterationHint = 0; - unsigned mAttributeIterationHint = 0; + uint16_t mEndpointIterationHint = 0; + unsigned mClusterIterationHint = 0; + unsigned mAttributeIterationHint = 0; + unsigned mDeviceTypeIterationHint = 0; EmberCommandListIterator mAcceptedCommandsIterator; EmberCommandListIterator mGeneratedCommandsIterator; diff --git a/src/app/data-model-provider/MetadataTypes.h b/src/app/data-model-provider/MetadataTypes.h index 5e46d63b967e81..3ee938231a1458 100644 --- a/src/app/data-model-provider/MetadataTypes.h +++ b/src/app/data-model-provider/MetadataTypes.h @@ -105,6 +105,13 @@ struct CommandEntry static const CommandEntry kInvalid; }; +/// Represents a device type that resides on an endpoint +struct DeviceTypeEntry +{ + DeviceTypeId deviceTypeId; + uint8_t deviceTypeVersion; +}; + /// Provides metadata information for a data model /// /// The data model can be viewed as a tree of endpoint/cluster/(attribute+commands+events) @@ -129,6 +136,11 @@ class ProviderMetadataTree virtual EndpointId FirstEndpoint() = 0; virtual EndpointId NextEndpoint(EndpointId before) = 0; + // This iteration describes device types registered on an endpoint + virtual std::optional FirstDeviceType(EndpointId endpoint) = 0; + virtual std::optional NextDeviceType(EndpointId endpoint, const DeviceTypeEntry & previous) = 0; + + // This iteration will list all clusters on a given endpoint virtual ClusterEntry FirstCluster(EndpointId endpoint) = 0; virtual ClusterEntry NextCluster(const ConcreteClusterPath & before) = 0; virtual std::optional GetClusterInfo(const ConcreteClusterPath & path) = 0; diff --git a/src/app/tests/test-interaction-model-api.cpp b/src/app/tests/test-interaction-model-api.cpp index 5750e098b995f3..b69c4234273cb9 100644 --- a/src/app/tests/test-interaction-model-api.cpp +++ b/src/app/tests/test-interaction-model-api.cpp @@ -216,6 +216,17 @@ EndpointId TestImCustomDataModel::NextEndpoint(EndpointId before) return CodegenDataModelProviderInstance()->NextEndpoint(before); } +std::optional TestImCustomDataModel::FirstDeviceType(EndpointId endpoint) +{ + return std::nullopt; +} + +std::optional TestImCustomDataModel::NextDeviceType(EndpointId endpoint, + const DataModel::DeviceTypeEntry & previous) +{ + return std::nullopt; +} + ClusterEntry TestImCustomDataModel::FirstCluster(EndpointId endpoint) { return CodegenDataModelProviderInstance()->FirstCluster(endpoint); diff --git a/src/app/tests/test-interaction-model-api.h b/src/app/tests/test-interaction-model-api.h index 3f4bd27cee36c1..df410c656b9b03 100644 --- a/src/app/tests/test-interaction-model-api.h +++ b/src/app/tests/test-interaction-model-api.h @@ -126,6 +126,9 @@ class TestImCustomDataModel : public DataModel::Provider EndpointId FirstEndpoint() override; EndpointId NextEndpoint(EndpointId before) override; + std::optional FirstDeviceType(EndpointId endpoint) override; + std::optional NextDeviceType(EndpointId endpoint, + const DataModel::DeviceTypeEntry & previous) override; DataModel::ClusterEntry FirstCluster(EndpointId endpoint) override; DataModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; std::optional GetClusterInfo(const ConcreteClusterPath & path) override; diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp index 7247031b071042..0d1b3827e8b365 100644 --- a/src/app/util/attribute-storage.cpp +++ b/src/app/util/attribute-storage.cpp @@ -1007,13 +1007,15 @@ uint8_t emberAfGetClusterCountForEndpoint(EndpointId endpoint) Span emberAfDeviceTypeListFromEndpoint(EndpointId endpoint, CHIP_ERROR & err) { - uint16_t endpointIndex = emberAfIndexFromEndpoint(endpoint); - Span ret; + return emberAfDeviceTypeListFromEndpointIndex(emberAfIndexFromEndpoint(endpoint), err); +} +chip::Span emberAfDeviceTypeListFromEndpointIndex(unsigned endpointIndex, CHIP_ERROR & err) +{ if (endpointIndex == 0xFFFF) { err = CHIP_ERROR_INVALID_ARGUMENT; - return ret; + return Span(); } err = CHIP_NO_ERROR; diff --git a/src/app/util/attribute-storage.h b/src/app/util/attribute-storage.h index 23a9b9b5fd20c3..9d2dcc60bb3f05 100644 --- a/src/app/util/attribute-storage.h +++ b/src/app/util/attribute-storage.h @@ -281,6 +281,7 @@ const EmberAfCluster * emberAfGetNthCluster(chip::EndpointId endpoint, uint8_t n // Retrieve the device type list associated with a specific endpoint. // chip::Span emberAfDeviceTypeListFromEndpoint(chip::EndpointId endpoint, CHIP_ERROR & err); +chip::Span emberAfDeviceTypeListFromEndpointIndex(unsigned endpointIndex, CHIP_ERROR & err); // // Override the device type list current associated with an endpoint with a user-provided list. The buffers backing diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp index 87ffa8bbbdd89f..3b9bed9de145e7 100644 --- a/src/app/util/mock/attribute-storage.cpp +++ b/src/app/util/mock/attribute-storage.cpp @@ -313,6 +313,18 @@ DataVersion * emberAfDataVersionStorage(const chip::app::ConcreteClusterPath & a return &dataVersion; } +chip::Span emberAfDeviceTypeListFromEndpoint(chip::EndpointId endpoint, CHIP_ERROR & err) +{ + // TODO: implement + return chip::Span(); +} + +chip::Span emberAfDeviceTypeListFromEndpointIndex(unsigned index, CHIP_ERROR & err) +{ + // TODO: implement + return chip::Span(); +} + void emberAfAttributeChanged(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId, AttributesChangedListener * listener) { diff --git a/src/controller/tests/data_model/DataModelFixtures.cpp b/src/controller/tests/data_model/DataModelFixtures.cpp index e30641fc530532..a5533dc51de57c 100644 --- a/src/controller/tests/data_model/DataModelFixtures.cpp +++ b/src/controller/tests/data_model/DataModelFixtures.cpp @@ -679,6 +679,17 @@ EndpointId CustomDataModel::NextEndpoint(EndpointId before) return CodegenDataModelProviderInstance()->NextEndpoint(before); } +std::optional CustomDataModel::FirstDeviceType(EndpointId endpoint) +{ + return std::nullopt; +} + +std::optional CustomDataModel::NextDeviceType(EndpointId endpoint, + const DataModel::DeviceTypeEntry & previous) +{ + return std::nullopt; +} + ClusterEntry CustomDataModel::FirstCluster(EndpointId endpoint) { return CodegenDataModelProviderInstance()->FirstCluster(endpoint); diff --git a/src/controller/tests/data_model/DataModelFixtures.h b/src/controller/tests/data_model/DataModelFixtures.h index 9c23b395bb7810..e9b32aead650ee 100644 --- a/src/controller/tests/data_model/DataModelFixtures.h +++ b/src/controller/tests/data_model/DataModelFixtures.h @@ -124,6 +124,9 @@ class CustomDataModel : public DataModel::Provider EndpointId FirstEndpoint() override; EndpointId NextEndpoint(EndpointId before) override; + std::optional FirstDeviceType(EndpointId endpoint) override; + std::optional NextDeviceType(EndpointId endpoint, + const DataModel::DeviceTypeEntry & previous) override; DataModel::ClusterEntry FirstCluster(EndpointId endpoint) override; DataModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override; std::optional GetClusterInfo(const ConcreteClusterPath & path) override; From 0954600e5bba7f6b2d10f111d6ee0edda98f0a36 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 13:33:40 -0400 Subject: [PATCH 2/9] Make unit tests work --- .../tests/TestCodegenModelViaMocks.cpp | 57 +++++++++++++++++++ src/app/data-model-provider/MetadataTypes.h | 5 ++ src/app/util/mock/MockNodeConfig.cpp | 13 +++-- src/app/util/mock/MockNodeConfig.h | 11 +++- src/app/util/mock/attribute-storage.cpp | 20 +++++-- 5 files changed, 93 insertions(+), 13 deletions(-) diff --git a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp index 5314684fc6f9a4..111e88efeed84a 100644 --- a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ +#include #include #include @@ -86,6 +87,15 @@ constexpr EndpointId kEndpointIdThatIsMissing = kMockEndpointMin - 1; constexpr AttributeId kReadOnlyAttributeId = 0x5001; +constexpr DeviceTypeId kDeviceTypeId1 = 123; +constexpr uint8_t kDeviceTypeId1Version = 10; + +constexpr DeviceTypeId kDeviceTypeId2 = 1122; +constexpr uint8_t kDeviceTypeId2Version = 11; + +constexpr DeviceTypeId kDeviceTypeId3 = 3; +constexpr uint8_t kDeviceTypeId3Version = 33; + static_assert(kEndpointIdThatIsMissing != kInvalidEndpointId); static_assert(kEndpointIdThatIsMissing != kMockEndpoint1); static_assert(kEndpointIdThatIsMissing != kMockEndpoint2); @@ -270,6 +280,10 @@ const MockNodeConfig gTestNodeConfig({ MockClusterConfig(MockClusterId(2), { ClusterRevision::Id, FeatureMap::Id, MockAttributeId(1), }), + }, { + { kDeviceTypeId1, kDeviceTypeId1Version}, + { kDeviceTypeId2, kDeviceTypeId2Version}, + { kDeviceTypeId3, kDeviceTypeId3Version}, }), MockEndpointConfig(kMockEndpoint2, { MockClusterConfig(MockClusterId(1), { @@ -296,6 +310,8 @@ const MockNodeConfig gTestNodeConfig({ {11}, /* acceptedCommands */ {4, 6} /* generatedCommands */ ), + }, { + { kDeviceTypeId2, kDeviceTypeId2Version}, }), MockEndpointConfig(kMockEndpoint3, { MockClusterConfig(MockClusterId(1), { @@ -2580,3 +2596,44 @@ TEST(TestCodegenModelViaMocks, EmberWriteInvalidDataType) ASSERT_EQ(model.WriteAttribute(test.GetRequest(), decoder), Status::Failure); ASSERT_TRUE(model.ChangeListener().DirtyList().empty()); } + +TEST(TestCodegenModelViaMocks, DeviceTypeIteration) +{ + UseMockNodeConfig config(gTestNodeConfig); + CodegenDataModelProviderWithContext model; + + // Mock endpoint 1 has 3 device types + std::optional entry = model.FirstDeviceType(kMockEndpoint1); + ASSERT_EQ(entry, + std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId1, .deviceTypeVersion = kDeviceTypeId1Version })); + entry = model.NextDeviceType(kMockEndpoint1, *entry); + ASSERT_EQ(entry, + std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId2, .deviceTypeVersion = kDeviceTypeId2Version })); + entry = model.NextDeviceType(kMockEndpoint1, *entry); + ASSERT_EQ(entry, + std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId3, .deviceTypeVersion = kDeviceTypeId3Version })); + entry = model.NextDeviceType(kMockEndpoint1, *entry); + ASSERT_FALSE(entry.has_value()); + + // Mock endpoint 2 has 1 device types + entry = model.FirstDeviceType(kMockEndpoint2); + ASSERT_EQ(entry, + std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId2, .deviceTypeVersion = kDeviceTypeId2Version })); + entry = model.NextDeviceType(kMockEndpoint2, *entry); + ASSERT_FALSE(entry.has_value()); + + // out of order query works + entry = std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId2, .deviceTypeVersion = kDeviceTypeId2Version }); + entry = model.NextDeviceType(kMockEndpoint1, *entry); + ASSERT_EQ(entry, + std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId3, .deviceTypeVersion = kDeviceTypeId3Version })); + + // invalid query fails + entry = std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId1, .deviceTypeVersion = kDeviceTypeId1Version }); + entry = model.NextDeviceType(kMockEndpoint2, *entry); + ASSERT_FALSE(entry.has_value()); + + // empty endpoint works + entry = model.FirstDeviceType(kMockEndpoint3); + ASSERT_FALSE(entry.has_value()); +} diff --git a/src/app/data-model-provider/MetadataTypes.h b/src/app/data-model-provider/MetadataTypes.h index 3ee938231a1458..228e2a84ad96eb 100644 --- a/src/app/data-model-provider/MetadataTypes.h +++ b/src/app/data-model-provider/MetadataTypes.h @@ -110,6 +110,11 @@ struct DeviceTypeEntry { DeviceTypeId deviceTypeId; uint8_t deviceTypeVersion; + + bool operator==(const DeviceTypeEntry & other) const + { + return (deviceTypeId == other.deviceTypeId) && (deviceTypeVersion == other.deviceTypeVersion); + } }; /// Provides metadata information for a data model diff --git a/src/app/util/mock/MockNodeConfig.cpp b/src/app/util/mock/MockNodeConfig.cpp index b48c846e65072d..98a83d1528d8ab 100644 --- a/src/app/util/mock/MockNodeConfig.cpp +++ b/src/app/util/mock/MockNodeConfig.cpp @@ -16,10 +16,12 @@ * limitations under the License. */ +#include "app/util/af-types.h" #include #include #include +#include #include #include @@ -114,8 +116,7 @@ MockClusterConfig::MockClusterConfig(ClusterId aId, std::initializer_list aEvents, std::initializer_list aAcceptedCommands, std::initializer_list aGeneratedCommands) : - id(aId), - attributes(aAttributes), events(aEvents), mEmberCluster{}, mAcceptedCommands(aAcceptedCommands), + id(aId), attributes(aAttributes), events(aEvents), mEmberCluster{}, mAcceptedCommands(aAcceptedCommands), mGeneratedCommands(aGeneratedCommands) { VerifyOrDie(aAttributes.size() < UINT16_MAX); @@ -174,8 +175,9 @@ const MockAttributeConfig * MockClusterConfig::attributeById(AttributeId attribu return findById(attributes, attributeId, outIndex); } -MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list aClusters) : - id(aId), clusters(aClusters), mEmberEndpoint{} +MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list aClusters, + std::initializer_list aDeviceTypes) : + id(aId), clusters(aClusters), mDeviceTypes(aDeviceTypes), mEmberEndpoint{} { VerifyOrDie(aClusters.size() < UINT8_MAX); @@ -189,7 +191,8 @@ MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list aClusters = {}); + MockEndpointConfig(EndpointId aId, std::initializer_list aClusters = {}, + std::initializer_list aDeviceTypes = {}); // Endpoint-config is self-referential: mEmberEndpoint.clusters references mEmberClusters.data() MockEndpointConfig(const MockEndpointConfig & other); @@ -109,12 +109,17 @@ struct MockEndpointConfig const MockClusterConfig * clusterById(ClusterId clusterId, ptrdiff_t * outIndex = nullptr) const; const EmberAfEndpointType * emberEndpoint() const { return &mEmberEndpoint; } + Span deviceTypes() const + { + return Span(mDeviceTypes.data(), mDeviceTypes.size()); + } const EndpointId id; const std::vector clusters; private: std::vector mEmberClusters; + std::vector mDeviceTypes; EmberAfEndpointType mEmberEndpoint; }; diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp index 3b9bed9de145e7..4545bf4edefd2a 100644 --- a/src/app/util/mock/attribute-storage.cpp +++ b/src/app/util/mock/attribute-storage.cpp @@ -313,16 +313,26 @@ DataVersion * emberAfDataVersionStorage(const chip::app::ConcreteClusterPath & a return &dataVersion; } -chip::Span emberAfDeviceTypeListFromEndpoint(chip::EndpointId endpoint, CHIP_ERROR & err) +chip::Span emberAfDeviceTypeListFromEndpoint(chip::EndpointId endpointId, CHIP_ERROR & err) { - // TODO: implement - return chip::Span(); + auto endpoint = GetMockNodeConfig().endpointById(endpointId); + + if (endpoint == nullptr) + { + return chip::Span(); + } + + return endpoint->deviceTypes(); } chip::Span emberAfDeviceTypeListFromEndpointIndex(unsigned index, CHIP_ERROR & err) { - // TODO: implement - return chip::Span(); + if (index >= GetMockNodeConfig().endpoints.size()) + { + return chip::Span(); + } + + return GetMockNodeConfig().endpoints[index].deviceTypes(); } void emberAfAttributeChanged(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId, From 438d90c33ac4252466377233a39fbd15e720cfa5 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 13:34:14 -0400 Subject: [PATCH 3/9] Restyle --- src/app/util/mock/MockNodeConfig.cpp | 6 ++++-- src/app/util/mock/MockNodeConfig.h | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/app/util/mock/MockNodeConfig.cpp b/src/app/util/mock/MockNodeConfig.cpp index 98a83d1528d8ab..c6572ed0aef075 100644 --- a/src/app/util/mock/MockNodeConfig.cpp +++ b/src/app/util/mock/MockNodeConfig.cpp @@ -116,7 +116,8 @@ MockClusterConfig::MockClusterConfig(ClusterId aId, std::initializer_list aEvents, std::initializer_list aAcceptedCommands, std::initializer_list aGeneratedCommands) : - id(aId), attributes(aAttributes), events(aEvents), mEmberCluster{}, mAcceptedCommands(aAcceptedCommands), + id(aId), + attributes(aAttributes), events(aEvents), mEmberCluster{}, mAcceptedCommands(aAcceptedCommands), mGeneratedCommands(aGeneratedCommands) { VerifyOrDie(aAttributes.size() < UINT16_MAX); @@ -177,7 +178,8 @@ const MockAttributeConfig * MockClusterConfig::attributeById(AttributeId attribu MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list aClusters, std::initializer_list aDeviceTypes) : - id(aId), clusters(aClusters), mDeviceTypes(aDeviceTypes), mEmberEndpoint{} + id(aId), + clusters(aClusters), mDeviceTypes(aDeviceTypes), mEmberEndpoint{} { VerifyOrDie(aClusters.size() < UINT8_MAX); diff --git a/src/app/util/mock/MockNodeConfig.h b/src/app/util/mock/MockNodeConfig.h index 73f41fb6a80056..39f11052a751be 100644 --- a/src/app/util/mock/MockNodeConfig.h +++ b/src/app/util/mock/MockNodeConfig.h @@ -56,7 +56,8 @@ struct MockAttributeConfig MockAttributeConfig(AttributeId aId, EmberAfAttributeMetadata metadata) : id(aId), attributeMetaData(metadata) {} MockAttributeConfig(AttributeId aId, EmberAfAttributeType type, EmberAfAttributeMask mask = ATTRIBUTE_MASK_WRITABLE | ATTRIBUTE_MASK_NULLABLE) : - id(aId), attributeMetaData(internal::DefaultAttributeMetadata(aId)) + id(aId), + attributeMetaData(internal::DefaultAttributeMetadata(aId)) { attributeMetaData.attributeType = type; attributeMetaData.mask = mask; From 6d360b3b732011cb7e8cbd413477d55dab767593 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 13:37:40 -0400 Subject: [PATCH 4/9] Fixup include --- .../codegen-data-model-provider/CodegenDataModelProvider.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp index d5b2e6eed1c2a0..a02a89e5ea5d93 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp @@ -14,7 +14,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "app/util/af-types.h" #include #include @@ -25,6 +24,7 @@ #include #include #include +#include #include #include #include From 4082e6def43d310f92394f828e1192d003ccab95 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 13:38:10 -0400 Subject: [PATCH 5/9] Fixup include --- .../tests/TestCodegenModelViaMocks.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp index 111e88efeed84a..aa3cdcd331bf92 100644 --- a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp @@ -15,7 +15,6 @@ * limitations under the License. */ -#include #include #include @@ -64,6 +63,7 @@ #include #include +#include #include using namespace chip; From d95a89f374577ee7cd6d400ad73e68733de0bacc Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 14:01:53 -0400 Subject: [PATCH 6/9] Make use of DataModel::Provider when doing device type resolution --- src/app/server/Server.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 426e3c686572c0..e4db190c507608 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -19,8 +19,10 @@ #include +#include #include #include +#include #include #include #include @@ -81,6 +83,33 @@ using chip::Transport::TcpListenParameters; namespace { +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +class DeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolver +{ +public: + bool IsDeviceTypeOnEndpoint(chip::DeviceTypeId deviceType, chip::EndpointId endpoint) override + { + chip::app::DataModel::Provider * model = chip::app::InteractionModelEngine::GetInstance()->GetDataModelProvider(); + + for (auto type = model->FirstDeviceType(endpoint); type.has_value(); type = model->NextDeviceType(endpoint, *type)) + { + if (type->deviceTypeId == deviceType) + { +#if CHIP_CONFIG_USE_EMBER_DATA_MODEL + VerifyOrDie(chip::app::IsDeviceTypeOnEndpoint(deviceType, endpoint)); +#endif // CHIP_CONFIG_USE_EMBER_DATA_MODEL + return true; + } + } +#if CHIP_CONFIG_USE_EMBER_DATA_MODEL + VerifyOrDie(!chip::app::IsDeviceTypeOnEndpoint(deviceType, endpoint)); +#endif // CHIP_CONFIG_USE_EMBER_DATA_MODEL + return false; + } +} sDeviceTypeResolver; +#else // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + +// Ember implementation of the device type resolver class DeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolver { public: @@ -89,6 +118,7 @@ class DeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolve return chip::app::IsDeviceTypeOnEndpoint(deviceType, endpoint); } } sDeviceTypeResolver; +#endif } // namespace From c3d686c924e168c976dff70432658c942352f2d8 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Tue, 1 Oct 2024 16:27:27 -0400 Subject: [PATCH 7/9] Add clang-tidy comments --- .../tests/TestCodegenModelViaMocks.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp index aa3cdcd331bf92..3840de9c8151dc 100644 --- a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp @@ -2606,12 +2606,15 @@ TEST(TestCodegenModelViaMocks, DeviceTypeIteration) std::optional entry = model.FirstDeviceType(kMockEndpoint1); ASSERT_EQ(entry, std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId1, .deviceTypeVersion = kDeviceTypeId1Version })); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): Assert above that this is not none entry = model.NextDeviceType(kMockEndpoint1, *entry); ASSERT_EQ(entry, std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId2, .deviceTypeVersion = kDeviceTypeId2Version })); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): Assert above that this is not none entry = model.NextDeviceType(kMockEndpoint1, *entry); ASSERT_EQ(entry, std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId3, .deviceTypeVersion = kDeviceTypeId3Version })); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): Assert above that this is not none entry = model.NextDeviceType(kMockEndpoint1, *entry); ASSERT_FALSE(entry.has_value()); @@ -2619,6 +2622,7 @@ TEST(TestCodegenModelViaMocks, DeviceTypeIteration) entry = model.FirstDeviceType(kMockEndpoint2); ASSERT_EQ(entry, std::make_optional(DeviceTypeEntry{ .deviceTypeId = kDeviceTypeId2, .deviceTypeVersion = kDeviceTypeId2Version })); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access): Assert above that this is not none entry = model.NextDeviceType(kMockEndpoint2, *entry); ASSERT_FALSE(entry.has_value()); From f279b09824431a20863687ce04729299e24f0819 Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 4 Oct 2024 09:43:35 -0400 Subject: [PATCH 8/9] Fix merge error: duplicate declaration --- .../codegen-data-model-provider/CodegenDataModelProvider.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h index 8b0bc79cc1aa05..f6d6e4e4ed4be3 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h @@ -98,10 +98,6 @@ class CodegenDataModelProvider : public chip::app::DataModel::Provider EndpointId NextEndpoint(EndpointId before) override; bool EndpointExists(EndpointId endpoint) override; - std::optional FirstDeviceType(EndpointId endpoint) override; - std::optional NextDeviceType(EndpointId endpoint, - const DataModel::DeviceTypeEntry & previous) override; - std::optional FirstDeviceType(EndpointId endpoint) override; std::optional NextDeviceType(EndpointId endpoint, const DataModel::DeviceTypeEntry & previous) override; From 45675c162c120829e387ac5d2fc81302303009dc Mon Sep 17 00:00:00 2001 From: Andrei Litvin Date: Fri, 4 Oct 2024 09:44:34 -0400 Subject: [PATCH 9/9] Fix a few more merge conflict markers --- .../CodegenDataModelProvider.cpp | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp index 729d66265791b5..972e0565131dd6 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp @@ -784,14 +784,11 @@ ConcreteCommandPath CodegenDataModelProvider::NextGeneratedCommand(const Concret std::optional CodegenDataModelProvider::FirstDeviceType(EndpointId endpoint) { -<<<<<<< HEAD -======= // Use the `Index` version even though `emberAfDeviceTypeListFromEndpoint` would work because // index finding is cached in TryFindEndpointIndex and this avoids an extra `emberAfIndexFromEndpoint` // during `Next` loops. This avoids O(n^2) on number of indexes when iterating over all device types. // // Not actually needed for `First`, however this makes First and Next consistent. ->>>>>>> master std::optional endpoint_index = TryFindEndpointIndex(endpoint); if (!endpoint_index.has_value()) { @@ -814,12 +811,9 @@ std::optional CodegenDataModelProvider::FirstDeviceT std::optional CodegenDataModelProvider::NextDeviceType(EndpointId endpoint, const DataModel::DeviceTypeEntry & previous) { -<<<<<<< HEAD -======= // Use the `Index` version even though `emberAfDeviceTypeListFromEndpoint` would work because // index finding is cached in TryFindEndpointIndex and this avoids an extra `emberAfIndexFromEndpoint` // during `Next` loops. This avoids O(n^2) on number of indexes when iterating over all device types. ->>>>>>> master std::optional endpoint_index = TryFindEndpointIndex(endpoint); if (!endpoint_index.has_value()) { @@ -829,26 +823,7 @@ std::optional CodegenDataModelProvider::NextDeviceTy CHIP_ERROR err = CHIP_NO_ERROR; Span deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err); -<<<<<<< HEAD - unsigned idx = 0; - if ((mDeviceTypeIterationHint < deviceTypes.size()) && IsSameDeviceTypeEntry(previous, deviceTypes[mDeviceTypeIterationHint])) - { - idx = mDeviceTypeIterationHint + 1; // target the NEXT device - } - else - { - while (idx < deviceTypes.size()) - { - idx++; - if (IsSameDeviceTypeEntry(previous, deviceTypes[idx - 1])) - { - break; - } - } - } -======= unsigned idx = FindNextDeviceTypeIndex(deviceTypes, previous, mDeviceTypeIterationHint); ->>>>>>> master if (idx >= deviceTypes.size()) {