Add ability to report endpoint device types in the DataModel::Provide…

…r interface (#35861) * Add separate listing for device types * Make unit tests work * Restyle * Fixup include * Fixup include * Add clang-tidy comments * Added a comment about future todo * more comments * Attempt to make the code more readable * Fix cast * Add comment for newly created issue. --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Oct 3, 2024 · cc6c773 · cc6c773
1 parent 2651245
commit cc6c773
Show file tree

Hide file tree

Showing 13 changed files with 260 additions and 10 deletions.
diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider.cpp
@@ -292,6 +292,57 @@ std::optional<DataModel::CommandEntry> EnumeratorCommandFinder::FindCommandEntry
     return (*id == kInvalidCommandId) ? DataModel::CommandEntry::kInvalid : CommandEntryFrom(path, *id);
 }
 
+// TODO: DeviceTypeEntry content is IDENTICAL to EmberAfDeviceType, so centralizing
+//       to a common type is probably better. Need to figure out dependencies since
+//       this would make ember return datamodel-provider types.
+//       See: https://github.com/project-chip/connectedhomeip/issues/35889
+DataModel::DeviceTypeEntry DeviceTypeEntryFromEmber(const EmberAfDeviceType & other)
+{
+    DataModel::DeviceTypeEntry entry;
+
+    entry.deviceTypeId      = other.deviceId;
+    entry.deviceTypeVersion = other.deviceVersion;
+
+    return entry;
+}
+
+// Explicitly compare for identical entries. note that types are different,
+// so you must do `a == b` and the `b == a` will not work.
+bool operator==(const DataModel::DeviceTypeEntry & a, const EmberAfDeviceType & b)
+{
+    return (a.deviceTypeId == b.deviceId) && (a.deviceTypeVersion == b.deviceVersion);
+}
+
+/// Find the `index` where one of the following holds:
+///    - types[index - 1] == previous OR
+///    - index == types.size()  // i.e. not found or there is no next
+///
+/// hintWherePreviousMayBe represents a search hint where previous may exist.
+unsigned FindNextDeviceTypeIndex(Span<const EmberAfDeviceType> types, const DataModel::DeviceTypeEntry previous,
+                                 unsigned hintWherePreviousMayBe)
+{
+    if (hintWherePreviousMayBe < types.size())
+    {
+        // this is a valid hint ... see if we are lucky
+        if (previous == types[hintWherePreviousMayBe])
+        {
+            return hintWherePreviousMayBe + 1; // return the next index
+        }
+    }
+
+    // hint was not useful. We have to do a full search
+    for (unsigned idx = 0; idx < types.size(); idx++)
+    {
+        if (previous == types[idx])
+        {
+            return idx + 1;
+        }
+    }
+
+    // cast should be safe as we know we do not have that many types
+    return static_cast<unsigned>(types.size());
+}
+
 const ConcreteCommandPath kInvalidCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId);
 
 } // namespace
@@ -731,6 +782,58 @@ ConcreteCommandPath CodegenDataModelProvider::NextGeneratedCommand(const Concret
     return ConcreteCommandPath(before.mEndpointId, before.mClusterId, *commandId);
 }
 
+std::optional<DataModel::DeviceTypeEntry> CodegenDataModelProvider::FirstDeviceType(EndpointId endpoint)
+{
+    // 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.
+    std::optional<unsigned> endpoint_index = TryFindEndpointIndex(endpoint);
+    if (!endpoint_index.has_value())
+    {
+        return std::nullopt;
+    }
+
+    CHIP_ERROR err                            = CHIP_NO_ERROR;
+    Span<const EmberAfDeviceType> deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err);
+
+    if (deviceTypes.empty())
+    {
+        return std::nullopt;
+    }
+
+    // we start at the beginning
+    mDeviceTypeIterationHint = 0;
+    return DeviceTypeEntryFromEmber(deviceTypes[0]);
+}
+
+std::optional<DataModel::DeviceTypeEntry> CodegenDataModelProvider::NextDeviceType(EndpointId endpoint,
+                                                                                   const DataModel::DeviceTypeEntry & previous)
+{
+    // 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.
+    std::optional<unsigned> endpoint_index = TryFindEndpointIndex(endpoint);
+    if (!endpoint_index.has_value())
+    {
+        return std::nullopt;
+    }
+
+    CHIP_ERROR err                            = CHIP_NO_ERROR;
+    Span<const EmberAfDeviceType> deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err);
+
+    unsigned idx = FindNextDeviceTypeIndex(deviceTypes, previous, mDeviceTypeIterationHint);
+
+    if (idx >= deviceTypes.size())
+    {
+        return std::nullopt;
+    }
+
+    mDeviceTypeIterationHint = idx;
+    return DeviceTypeEntryFromEmber(deviceTypes[idx]);
+}
+
 bool CodegenDataModelProvider::EventPathIncludesAccessibleConcretePath(const EventPathParams & path,
                                                                        const Access::SubjectDescriptor & descriptor)
 {

diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider.h b/src/app/codegen-data-model-provider/CodegenDataModelProvider.h
@@ -98,6 +98,10 @@ class CodegenDataModelProvider : public chip::app::DataModel::Provider
     EndpointId NextEndpoint(EndpointId before) override;
     bool EndpointExists(EndpointId endpoint) override;
 
+    std::optional<DataModel::DeviceTypeEntry> FirstDeviceType(EndpointId endpoint) override;
+    std::optional<DataModel::DeviceTypeEntry> NextDeviceType(EndpointId endpoint,
+                                                             const DataModel::DeviceTypeEntry & previous) override;
+
     DataModel::ClusterEntry FirstCluster(EndpointId endpoint) override;
     DataModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override;
     std::optional<DataModel::ClusterInfo> GetClusterInfo(const ConcreteClusterPath & path) override;
@@ -116,9 +120,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/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp
@@ -63,6 +63,7 @@
 #include <lib/support/Span.h>
 #include <protocols/interaction_model/StatusCode.h>
 
+#include <optional>
 #include <vector>
 
 using namespace chip;
@@ -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,48 @@ 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<DeviceTypeEntry> 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());
+
+    // Mock endpoint 2 has 1 device types
+    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());
+
+    // 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
@@ -105,6 +105,18 @@ struct CommandEntry
     static const CommandEntry kInvalid;
 };
 
+/// Represents a device type that resides on an endpoint
+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
 ///
 /// The data model can be viewed as a tree of endpoint/cluster/(attribute+commands+events)
@@ -130,6 +142,11 @@ class ProviderMetadataTree
     virtual EndpointId NextEndpoint(EndpointId before) = 0;
     virtual bool EndpointExists(EndpointId id);
 
+    // This iteration describes device types registered on an endpoint
+    virtual std::optional<DeviceTypeEntry> FirstDeviceType(EndpointId endpoint)                                  = 0;
+    virtual std::optional<DeviceTypeEntry> 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<ClusterInfo> 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
@@ -216,6 +216,17 @@ EndpointId TestImCustomDataModel::NextEndpoint(EndpointId before)
     return CodegenDataModelProviderInstance()->NextEndpoint(before);
 }
 
+std::optional<DataModel::DeviceTypeEntry> TestImCustomDataModel::FirstDeviceType(EndpointId endpoint)
+{
+    return std::nullopt;
+}
+
+std::optional<DataModel::DeviceTypeEntry> 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
@@ -131,6 +131,9 @@ class TestImCustomDataModel : public DataModel::Provider
 
     EndpointId FirstEndpoint() override;
     EndpointId NextEndpoint(EndpointId before) override;
+    std::optional<DataModel::DeviceTypeEntry> FirstDeviceType(EndpointId endpoint) override;
+    std::optional<DataModel::DeviceTypeEntry> NextDeviceType(EndpointId endpoint,
+                                                             const DataModel::DeviceTypeEntry & previous) override;
     DataModel::ClusterEntry FirstCluster(EndpointId endpoint) override;
     DataModel::ClusterEntry NextCluster(const ConcreteClusterPath & before) override;
     std::optional<DataModel::ClusterInfo> GetClusterInfo(const ConcreteClusterPath & path) override;

diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp
@@ -1007,13 +1007,15 @@ uint8_t emberAfGetClusterCountForEndpoint(EndpointId endpoint)
 
 Span<const EmberAfDeviceType> emberAfDeviceTypeListFromEndpoint(EndpointId endpoint, CHIP_ERROR & err)
 {
-    uint16_t endpointIndex = emberAfIndexFromEndpoint(endpoint);
-    Span<const EmberAfDeviceType> ret;
+    return emberAfDeviceTypeListFromEndpointIndex(emberAfIndexFromEndpoint(endpoint), err);
+}
 
+chip::Span<const EmberAfDeviceType> emberAfDeviceTypeListFromEndpointIndex(unsigned endpointIndex, CHIP_ERROR & err)
+{
     if (endpointIndex == 0xFFFF)
     {
         err = CHIP_ERROR_INVALID_ARGUMENT;
-        return ret;
+        return Span<const EmberAfDeviceType>();
     }
 
     err = CHIP_NO_ERROR;

diff --git 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<const EmberAfDeviceType> emberAfDeviceTypeListFromEndpoint(chip::EndpointId endpoint, CHIP_ERROR & err);
+chip::Span<const EmberAfDeviceType> 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/MockNodeConfig.cpp b/src/app/util/mock/MockNodeConfig.cpp
@@ -16,10 +16,12 @@
  *    limitations under the License.
  */
 
+#include "app/util/af-types.h"
 #include <app/util/mock/MockNodeConfig.h>
 
 #include <app/util/att-storage.h>
 #include <app/util/attribute-storage.h>
+#include <initializer_list>
 #include <lib/support/CodeUtils.h>
 
 #include <utility>
@@ -174,8 +176,10 @@ const MockAttributeConfig * MockClusterConfig::attributeById(AttributeId attribu
     return findById(attributes, attributeId, outIndex);
 }
 
-MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list<MockClusterConfig> aClusters) :
-    id(aId), clusters(aClusters), mEmberEndpoint{}
+MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list<MockClusterConfig> aClusters,
+                                       std::initializer_list<EmberAfDeviceType> aDeviceTypes) :
+    id(aId),
+    clusters(aClusters), mDeviceTypes(aDeviceTypes), mEmberEndpoint{}
 {
     VerifyOrDie(aClusters.size() < UINT8_MAX);
 
@@ -189,7 +193,8 @@ MockEndpointConfig::MockEndpointConfig(EndpointId aId, std::initializer_list<Moc
 }
 
 MockEndpointConfig::MockEndpointConfig(const MockEndpointConfig & other) :
-    id(other.id), clusters(other.clusters), mEmberClusters(other.mEmberClusters), mEmberEndpoint(other.mEmberEndpoint)
+    id(other.id), clusters(other.clusters), mEmberClusters(other.mEmberClusters), mDeviceTypes(other.mDeviceTypes),
+    mEmberEndpoint(other.mEmberEndpoint)
 {
     // fix self-referencing pointers
     mEmberEndpoint.cluster = mEmberClusters.data();

diff --git a/src/app/util/mock/MockNodeConfig.h b/src/app/util/mock/MockNodeConfig.h
@@ -101,20 +101,26 @@ struct MockClusterConfig
 
 struct MockEndpointConfig
 {
-    MockEndpointConfig(EndpointId aId, std::initializer_list<MockClusterConfig> aClusters = {});
+    MockEndpointConfig(EndpointId aId, std::initializer_list<MockClusterConfig> aClusters = {},
+                       std::initializer_list<EmberAfDeviceType> aDeviceTypes = {});
 
     // Endpoint-config is self-referential: mEmberEndpoint.clusters references  mEmberClusters.data()
     MockEndpointConfig(const MockEndpointConfig & other);
     MockEndpointConfig & operator=(const MockEndpointConfig &) = delete;
 
     const MockClusterConfig * clusterById(ClusterId clusterId, ptrdiff_t * outIndex = nullptr) const;
     const EmberAfEndpointType * emberEndpoint() const { return &mEmberEndpoint; }
+    Span<const EmberAfDeviceType> deviceTypes() const
+    {
+        return Span<const EmberAfDeviceType>(mDeviceTypes.data(), mDeviceTypes.size());
+    }
 
     const EndpointId id;
     const std::vector<MockClusterConfig> clusters;
 
 private:
     std::vector<EmberAfCluster> mEmberClusters;
+    std::vector<EmberAfDeviceType> mDeviceTypes;
     EmberAfEndpointType mEmberEndpoint;
 };