From fda93f9bdaded2893103a770805acb6b4964cc68 Mon Sep 17 00:00:00 2001 From: Li-An Ding Date: Tue, 2 Nov 2021 22:47:59 -0700 Subject: [PATCH] Simplfy the implementation of Mode Select Cluster --- .../mode-select-server/mode-select-server.cpp | 22 ++--- .../static-supported-modes-manager.cpp | 47 ++++++---- .../static-supported-modes-manager.h | 94 ++++--------------- .../supported-modes-manager.h | 42 +++------ 4 files changed, 69 insertions(+), 136 deletions(-) diff --git a/src/app/clusters/mode-select-server/mode-select-server.cpp b/src/app/clusters/mode-select-server/mode-select-server.cpp index 8662d94f7ace7b..9ba8a40fd3d8a1 100644 --- a/src/app/clusters/mode-select-server/mode-select-server.cpp +++ b/src/app/clusters/mode-select-server/mode-select-server.cpp @@ -51,25 +51,23 @@ CHIP_ERROR ModeSelectAttrAccess::Read(const ConcreteAttributePath & aPath, Attri { VerifyOrDie(aPath.mClusterId == ModeSelect::Id); - const ModeSelect::StaticSupportedModesManager & gSupportedModeManager = - ModeSelect::StaticSupportedModesManager::getStaticSupportedModesManagerInstance(); + const ModeSelect::SupportedModesManager * gSupportedModeManager = + ModeSelect::getSupportedModesManager(); if (ModeSelect::Attributes::SupportedModes::Id == aPath.mAttributeId) { - const ModeSelect::StaticSupportedModesManager::IteratorFactory * iteratorFactory = - gSupportedModeManager.getIteratorFactory(aPath.mEndpointId); - if (iteratorFactory == nullptr) + const ModeSelect::SupportedModesManager::ModeOptionsProvider modeOptionsProvider = + gSupportedModeManager->getModeOptionsProvider(aPath.mEndpointId); + if (modeOptionsProvider.begin() == nullptr) { aEncoder.Encode(DataModel::List()); return CHIP_NO_ERROR; } CHIP_ERROR err; - err = aEncoder.EncodeList([iteratorFactory](const TagBoundEncoder & encoder) -> CHIP_ERROR { - const auto & end = *(iteratorFactory->end()); - for (auto it = *(iteratorFactory->begin()); it != end; ++it) + err = aEncoder.EncodeList([modeOptionsProvider](const TagBoundEncoder & encoder) -> CHIP_ERROR { + const auto * end = modeOptionsProvider.end(); + for (auto * it = modeOptionsProvider.begin(); it != end; ++it) { - emberAfPrintln(EMBER_AF_PRINT_DEBUG, "ModeSelect: dereferencing it"); - emberAfPrintln(EMBER_AF_PRINT_DEBUG, "ModeSelect: it= %p", (void *) it.operator->()); auto & modeOption = *it; ReturnErrorOnFailure(encoder.Encode(modeOption)); } @@ -90,9 +88,7 @@ bool emberAfModeSelectClusterChangeToModeCallback(CommandHandler * commandHandle uint8_t newMode = commandData.newMode; // Check that the newMode matches one of the supported options const ModeSelect::Structs::ModeOptionStruct::Type * modeOptionPtr; - const ModeSelect::StaticSupportedModesManager & gSupportedModeManager = - ModeSelect::StaticSupportedModesManager::getStaticSupportedModesManagerInstance(); - EmberAfStatus checkSupportedModeStatus = gSupportedModeManager.getModeOptionByMode(endpointId, newMode, &modeOptionPtr); + EmberAfStatus checkSupportedModeStatus = ModeSelect::getSupportedModesManager()->getModeOptionByMode(endpointId, newMode, &modeOptionPtr); if (EMBER_ZCL_STATUS_SUCCESS != checkSupportedModeStatus) { emberAfPrintln(EMBER_AF_PRINT_DEBUG, "ModeSelect: Failed to find the option with mode %" PRIu8, newMode); diff --git a/src/app/clusters/mode-select-server/static-supported-modes-manager.cpp b/src/app/clusters/mode-select-server/static-supported-modes-manager.cpp index d65d0bda793437..5b99cc6f9156be 100644 --- a/src/app/clusters/mode-select-server/static-supported-modes-manager.cpp +++ b/src/app/clusters/mode-select-server/static-supported-modes-manager.cpp @@ -11,7 +11,7 @@ using namespace chip::app::Clusters; using namespace chip::app::Clusters::ModeSelect; using ModeOptionStructType = Structs::ModeOptionStruct::Type; -using storage_value_type = const ModeOptionStructType *; +using storage_value_type = const ModeOptionStructType; namespace { Structs::ModeOptionStruct::Type buildModeOptionStruct(const char * label, uint8_t mode, uint32_t semanticTag) { @@ -23,35 +23,43 @@ Structs::ModeOptionStruct::Type buildModeOptionStruct(const char * label, uint8_ } } // namespace -const Structs::ModeOptionStruct::Type StaticSupportedModesManager::blackOption = buildModeOptionStruct("Black", 0, 0); -const Structs::ModeOptionStruct::Type StaticSupportedModesManager::cappuccinoOption = buildModeOptionStruct("Cappuccino", 4, 0); -const Structs::ModeOptionStruct::Type StaticSupportedModesManager::espressoOption = buildModeOptionStruct("Espresso", 7, 0); -storage_value_type StaticSupportedModesManager::coffeeOptions[] = { &blackOption, &cappuccinoOption, &espressoOption }; -const Span StaticSupportedModesManager::coffeeOptionsSpan = - Span(StaticSupportedModesManager::coffeeOptions); -const map> StaticSupportedModesManager::optionsByEndpoints = { - { 1, StaticSupportedModesManager::coffeeOptionsSpan } +// TODO: Configure your options for each endpoint +storage_value_type StaticSupportedModesManager::coffeeOptions[] = { + buildModeOptionStruct("Black", 0, 0), + buildModeOptionStruct("Cappuccino", 4, 0), + buildModeOptionStruct("Espresso", 7, 0) +}; +const StaticSupportedModesManager::EndpointSpanPair StaticSupportedModesManager::supportedOptionsByEndpoints[ENDPOINTS_WITH_MODES] = { + EndpointSpanPair(1, Span(StaticSupportedModesManager::coffeeOptions)) // Options for Endpoint 1 }; const StaticSupportedModesManager StaticSupportedModesManager::instance = StaticSupportedModesManager(); -const StaticSupportedModesManager::IteratorFactory * StaticSupportedModesManager::getIteratorFactory(EndpointId endpointId) const +const SupportedModesManager::ModeOptionsProvider StaticSupportedModesManager::getModeOptionsProvider(EndpointId endpointId) const { - const auto & it = _iteratorFactoriesByEndpoints.find(endpointId); - return (it == _iteratorFactoriesByEndpoints.end()) ? nullptr : &(it->second); + for (size_t i = 0; i < ENDPOINTS_WITH_MODES; ++i) + { + const EndpointSpanPair& endpointSpanPair = supportedOptionsByEndpoints[i]; + if (endpointSpanPair.mEndpointId == endpointId) + { + return ModeOptionsProvider(endpointSpanPair.mSpan.data(), endpointSpanPair.mSpan.end()); + } + } + return ModeOptionsProvider(nullptr, nullptr); } EmberAfStatus StaticSupportedModesManager::getModeOptionByMode(unsigned short endpointId, unsigned char mode, const ModeOptionStructType ** dataPtr) const { - auto * iteratorFactory = this->getIteratorFactory(endpointId); - if (iteratorFactory == nullptr) + auto modeOptionsProvider = this->getModeOptionsProvider(endpointId); + if (modeOptionsProvider.begin() == nullptr) { return EMBER_ZCL_STATUS_UNSUPPORTED_CLUSTER; } - const StaticSupportedModesManager::Iterator & begin = *(this->getIteratorFactory(endpointId)->begin()); - const StaticSupportedModesManager::Iterator & end = *(this->getIteratorFactory(endpointId)->end()); - for (auto it = begin; it != end; ++it) + auto* begin = this->getModeOptionsProvider(endpointId).begin(); + auto* end = this->getModeOptionsProvider(endpointId).end(); + + for (auto* it = begin; it != end; ++it) { auto & modeOption = *it; if (modeOption.mode == mode) @@ -63,3 +71,8 @@ EmberAfStatus StaticSupportedModesManager::getModeOptionByMode(unsigned short en emberAfPrintln(EMBER_AF_PRINT_DEBUG, "Cannot find the mode %c", mode); return EMBER_ZCL_STATUS_INVALID_ARGUMENT; } + +const ModeSelect::SupportedModesManager* ModeSelect::getSupportedModesManager() +{ + return &StaticSupportedModesManager::instance; +} diff --git a/src/app/clusters/mode-select-server/static-supported-modes-manager.h b/src/app/clusters/mode-select-server/static-supported-modes-manager.h index b48b96e7a8e60c..2128c80d8298b6 100644 --- a/src/app/clusters/mode-select-server/static-supported-modes-manager.h +++ b/src/app/clusters/mode-select-server/static-supported-modes-manager.h @@ -24,6 +24,10 @@ #include #include +#ifndef ENDPOINTS_WITH_MODES +#define ENDPOINTS_WITH_MODES 1 +#endif + namespace chip { namespace app { namespace Clusters { @@ -36,97 +40,31 @@ namespace ModeSelect { class StaticSupportedModesManager : public chip::app::Clusters::ModeSelect::SupportedModesManager { using ModeOptionStructType = Structs::ModeOptionStruct::Type; - using storage_value_type = const ModeOptionStructType *; - - static const ModeOptionStructType blackOption; - static const ModeOptionStructType cappuccinoOption; - static const ModeOptionStructType espressoOption; - - static const ModeOptionStructType * coffeeOptions[]; - static const Span coffeeOptionsSpan; - static const std::map> optionsByEndpoints; - -public: - static const StaticSupportedModesManager instance; + using storage_value_type = const ModeOptionStructType; - struct Iterator : public chip::app::Clusters::ModeSelect::SupportedModesManager::ModeOptionStructIterator + struct EndpointSpanPair { - using iterator_category = std::forward_iterator_tag; - using difference_type = std::ptrdiff_t; - using pointer = storage_value_type *; - using base_iterator_type = chip::app::Clusters::ModeSelect::SupportedModesManager::ModeOptionStructIterator; - - Iterator(const pointer aPtr) : mPtr(aPtr) {} - ~Iterator() = default; - - const ModeOptionStructType & operator*() const override { return **mPtr; } - const ModeOptionStructType * operator->() override { return *mPtr; } - const ModeOptionStructType * operator->() const override { return *mPtr; } - - // Prefix increment - base_iterator_type & operator++() override - { - ++mPtr; - return *this; - } - - bool operator==(const base_iterator_type & other) const override - { - // Warning: we are not doing type check - // TODO: use of typeid requires -frtti - // if (typeid(other) != typeid(*this)) - // { - // return false; - // } - return this->operator->() == other.operator->(); - } - bool operator!=(const base_iterator_type & other) const override { return !((*this) == other); } - - private: - pointer mPtr; - }; + const EndpointId mEndpointId; + const Span mSpan; - struct IteratorFactory : public chip::app::Clusters::ModeSelect::SupportedModesManager::ModeOptionStructIteratorFactory - { - using pointer = Iterator *; - using const_pointer = const pointer; - - IteratorFactory(const Span & aSupportedOptions) : - _begin(Iterator(aSupportedOptions.data())), _end(Iterator(aSupportedOptions.data() + aSupportedOptions.size())) - {} - ~IteratorFactory() = default; + EndpointSpanPair(const EndpointId aEndpointId, const Span&& aSpan ): mEndpointId(aEndpointId), mSpan(aSpan) {} + }; - const Iterator * begin() const override { return &_begin; } - const Iterator * end() const override { return &_end; } + static storage_value_type coffeeOptions[]; + static const EndpointSpanPair supportedOptionsByEndpoints[ENDPOINTS_WITH_MODES]; - private: - const Iterator _begin; - const Iterator _end; - }; +public: + static const StaticSupportedModesManager instance; - const IteratorFactory * getIteratorFactory(EndpointId endpointId) const override; + const SupportedModesManager::ModeOptionsProvider getModeOptionsProvider(EndpointId endpointId) const override; EmberAfStatus getModeOptionByMode(EndpointId endpointId, uint8_t mode, const ModeOptionStructType ** dataPtr) const override; ~StaticSupportedModesManager(){}; - StaticSupportedModesManager() : StaticSupportedModesManager(&optionsByEndpoints) {} + StaticSupportedModesManager() {} static inline const StaticSupportedModesManager & getStaticSupportedModesManagerInstance() { return instance; } - -private: - StaticSupportedModesManager(const std::map> * const supportedModes) : - _iteratorFactoriesByEndpoints(std::map()) - { - for (auto & entry : *supportedModes) - { - _iteratorFactoriesByEndpoints.insert( - std::pair(entry.first, IteratorFactory(entry.second))); - } - } - // TODO: Implement move constructor? - - std::map _iteratorFactoriesByEndpoints; }; } // namespace ModeSelect diff --git a/src/app/clusters/mode-select-server/supported-modes-manager.h b/src/app/clusters/mode-select-server/supported-modes-manager.h index f4d6d50216c61e..827b89503b3515 100644 --- a/src/app/clusters/mode-select-server/supported-modes-manager.h +++ b/src/app/clusters/mode-select-server/supported-modes-manager.h @@ -41,53 +41,37 @@ class SupportedModesManager using ModeOptionStructType = Structs::ModeOptionStruct::Type; public: - struct ModeOptionStructIterator - { - using iterator_category = std::forward_iterator_tag; - using difference_type = std::ptrdiff_t; - using value_type = const ModeOptionStructType; - using pointer = value_type *; - using reference = value_type &; - - virtual reference operator*() const = 0; - virtual pointer operator->() = 0; - virtual pointer operator->() const = 0; - - // Prefix increment - virtual ModeOptionStructIterator & operator++() = 0; - - virtual bool operator==(const ModeOptionStructIterator & other) const = 0; - virtual bool operator!=(const ModeOptionStructIterator & other) const = 0; - - virtual ~ModeOptionStructIterator() {} - }; /** - * A factory that can return the ModeOptionStructIterators for a specific endpoint. + * A class that can return the supported ModeOptions for a specific endpoint. */ - struct ModeOptionStructIteratorFactory + struct ModeOptionsProvider { - using const_pointer = const ModeOptionStructIterator *; + using pointer = const ModeOptionStructType*; /** * Returns the ModeOptionStructIterator to the first option. */ - virtual const_pointer begin() const = 0; + inline pointer begin() const { return mBegin; } /** * Returns the ModeOptionStructIterator to an element after the last option. */ - virtual const_pointer end() const = 0; + inline pointer end() const { return mEnd; } + + ModeOptionsProvider(const pointer aBegin, const pointer aEnd): mBegin(aBegin), mEnd(aEnd) {} + + pointer mBegin; + pointer mEnd; - virtual ~ModeOptionStructIteratorFactory() {} }; /** * Given the endpointId, returns all its supported modes options. * @param endpointId - * @return The iterator factory for the endpoint, or nullptr if the endpoint doesn't support ModeSelectCluster. + * @return The mode options provider for the endpoint. */ - virtual const ModeOptionStructIteratorFactory * getIteratorFactory(EndpointId endpointId) const = 0; + virtual const ModeOptionsProvider getModeOptionsProvider(EndpointId endpointId) const = 0; /** * Given the endpointId and a mode value, find the ModeOptionStruct that matches the mode. @@ -102,6 +86,8 @@ class SupportedModesManager virtual ~SupportedModesManager() {} }; +const SupportedModesManager* getSupportedModesManager (); + } // namespace ModeSelect } // namespace Clusters } // namespace app