From 1f8e6b5f58d1387364ecc3d4d137a8914ba9acfd Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Fri, 1 Apr 2022 11:00:02 -0700 Subject: [PATCH] Use SupportedLocalesIterator to read ReadSupportedLocales from DeviceInfoProvider --- .../localization-configuration-server.cpp | 103 +++++++++++++----- src/include/platform/DeviceInfoProvider.h | 14 ++- src/include/platform/PlatformManager.h | 7 -- .../internal/GenericPlatformManagerImpl.h | 8 -- src/lib/support/Span.h | 11 ++ src/platform/Darwin/PlatformManagerImpl.cpp | 17 --- src/platform/Darwin/PlatformManagerImpl.h | 1 - src/platform/Linux/DeviceInfoProviderImpl.cpp | 71 ++++++++++++ src/platform/Linux/DeviceInfoProviderImpl.h | 14 +++ src/platform/Linux/PlatformManagerImpl.cpp | 16 --- src/platform/Linux/PlatformManagerImpl.h | 1 - src/platform/fake/PlatformManagerImpl.h | 5 - 12 files changed, 183 insertions(+), 85 deletions(-) diff --git a/src/app/clusters/localization-configuration-server/localization-configuration-server.cpp b/src/app/clusters/localization-configuration-server/localization-configuration-server.cpp index 750f28192adb7e..5063e7378ec219 100644 --- a/src/app/clusters/localization-configuration-server/localization-configuration-server.cpp +++ b/src/app/clusters/localization-configuration-server/localization-configuration-server.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include using namespace chip; @@ -57,18 +58,32 @@ LocalizationConfigurationAttrAccess gAttrAccess; CHIP_ERROR LocalizationConfigurationAttrAccess::ReadSupportedLocales(AttributeValueEncoder & aEncoder) { CHIP_ERROR err = CHIP_NO_ERROR; - DeviceLayer::AttributeList supportedLocales; - if (DeviceLayer::PlatformMgr().GetSupportedLocales(supportedLocales) == CHIP_NO_ERROR) + DeviceLayer::DeviceInfoProvider * provider = DeviceLayer::GetDeviceInfoProvider(); + + if (provider) { - err = aEncoder.EncodeList([&supportedLocales](const auto & encoder) -> CHIP_ERROR { - for (const Span & locale : supportedLocales) - { - ReturnErrorOnFailure(encoder.Encode(locale)); - } + DeviceLayer::DeviceInfoProvider::SupportedLocalesIterator * it = provider->IterateSupportedLocales(); + + if (it) + { + err = aEncoder.EncodeList([&it](const auto & encoder) -> CHIP_ERROR { + CharSpan activeLocale; + + while (it->Next(activeLocale)) + { + ReturnErrorOnFailure(encoder.Encode(activeLocale)); + } - return CHIP_NO_ERROR; - }); + return CHIP_NO_ERROR; + }); + + it->Release(); + } + else + { + err = aEncoder.EncodeEmptyList(); + } } else { @@ -102,17 +117,23 @@ using Status = Protocols::InteractionModel::Status; static Protocols::InteractionModel::Status emberAfPluginLocalizationConfigurationOnActiveLocaleChange(EndpointId EndpointId, CharSpan newLangtag) { - DeviceLayer::AttributeList supportedLocales; + DeviceLayer::DeviceInfoProvider * provider = DeviceLayer::GetDeviceInfoProvider(); + DeviceLayer::DeviceInfoProvider::SupportedLocalesIterator * it; - if (DeviceLayer::PlatformMgr().GetSupportedLocales(supportedLocales) == CHIP_NO_ERROR) + if (provider && (it = provider->IterateSupportedLocales())) { - for (const Span & locale : supportedLocales) + CharSpan outLocale; + + while (it->Next(outLocale)) { - if (locale.data_equal(newLangtag)) + if (outLocale.data_equal(newLangtag)) { + it->Release(); return Status::Success; } } + + it->Release(); } return Status::InvalidValue; @@ -141,32 +162,58 @@ Protocols::InteractionModel::Status MatterLocalizationConfigurationClusterServer void emberAfLocalizationConfigurationClusterServerInitCallback(EndpointId endpoint) { - DeviceLayer::AttributeList supportedLocales; - CharSpan validLocale; - - char outBuffer[Attributes::ActiveLocale::TypeInfo::MaxLength()]; - MutableCharSpan activeLocale(outBuffer); + char outBuf[Attributes::ActiveLocale::TypeInfo::MaxLength()]; + MutableCharSpan activeLocale(outBuf); EmberAfStatus status = ActiveLocale::Get(endpoint, activeLocale); VerifyOrReturn(EMBER_ZCL_STATUS_SUCCESS == status, ChipLogError(Zcl, "Failed to read ActiveLocale with error: 0x%02x", status)); - // We could have an invalid ActiveLocale value if an OTA update removed support for the value we were using. - if (DeviceLayer::PlatformMgr().GetSupportedLocales(supportedLocales) == CHIP_NO_ERROR) + DeviceLayer::DeviceInfoProvider * provider = DeviceLayer::GetDeviceInfoProvider(); + + VerifyOrReturn(provider != nullptr, ChipLogError(Zcl, "DeviceInfoProvider is not registered")); + + DeviceLayer::DeviceInfoProvider::SupportedLocalesIterator * it = provider->IterateSupportedLocales(); + + if (it) { - for (const Span & locale : supportedLocales) + CHIP_ERROR err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND; + + char tempBuf[Attributes::ActiveLocale::TypeInfo::MaxLength()]; + MutableCharSpan validLocale(tempBuf); + CharSpan outLocale; + bool validLocaleCached = false; + + while (it->Next(outLocale)) { - if (locale.data_equal(activeLocale)) + if (outLocale.data_equal(activeLocale)) { - return; + err = CHIP_NO_ERROR; + break; } - validLocale = locale; + if (!validLocaleCached) + { + if (CopyCharSpanToMutableCharSpan(outLocale, validLocale) != CHIP_NO_ERROR) + { + err = CHIP_ERROR_WRITE_FAILED; + break; + } + else + { + validLocaleCached = true; + } + } } - // If initial value is not one of the allowed values, pick one valid value and write it. - status = ActiveLocale::Set(endpoint, validLocale); - VerifyOrReturn(EMBER_ZCL_STATUS_SUCCESS == status, - ChipLogError(Zcl, "Failed to write active locale with error: 0x%02x", status)); + it->Release(); + + if (err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND) + { + // If initial value is not one of the allowed values, write the valid value it. + status = ActiveLocale::Set(endpoint, validLocale); + VerifyOrReturn(EMBER_ZCL_STATUS_SUCCESS == status, + ChipLogError(Zcl, "Failed to write active locale with error: 0x%02x", status)); + } } } diff --git a/src/include/platform/DeviceInfoProvider.h b/src/include/platform/DeviceInfoProvider.h index cb23e97bb36164..cfe371e48aa97b 100644 --- a/src/include/platform/DeviceInfoProvider.h +++ b/src/include/platform/DeviceInfoProvider.h @@ -31,6 +31,7 @@ namespace DeviceLayer { static constexpr size_t kMaxUserLabelListLength = 10; static constexpr size_t kMaxLabelNameLength = 16; static constexpr size_t kMaxLabelValueLength = 16; +static constexpr size_t kMaxActiveLocaleLength = 35; class DeviceInfoProvider { @@ -66,8 +67,9 @@ class DeviceInfoProvider using FixedLabelType = app::Clusters::FixedLabel::Structs::LabelStruct::Type; using UserLabelType = app::Clusters::UserLabel::Structs::LabelStruct::Type; - using FixedLabelIterator = Iterator; - using UserLabelIterator = Iterator; + using FixedLabelIterator = Iterator; + using UserLabelIterator = Iterator; + using SupportedLocalesIterator = Iterator; DeviceInfoProvider() = default; @@ -91,6 +93,14 @@ class DeviceInfoProvider virtual FixedLabelIterator * IterateFixedLabel(EndpointId endpoint) = 0; virtual UserLabelIterator * IterateUserLabel(EndpointId endpoint) = 0; + /** + * Creates an iterator that may be used to obtain the list of supported locales of the device. + * In order to release the allocated memory, the Release() method must be called after the iteration is finished. + * @retval An instance of EndpointIterator on success + * @retval nullptr if no iterator instances are available. + */ + virtual SupportedLocalesIterator * IterateSupportedLocales() = 0; + protected: /** * @brief Set the UserLabel at the specified index of the UserLabelList on a given endpoint diff --git a/src/include/platform/PlatformManager.h b/src/include/platform/PlatformManager.h index 8e78209698c2cf..b657a3c1d63ff1 100644 --- a/src/include/platform/PlatformManager.h +++ b/src/include/platform/PlatformManager.h @@ -37,7 +37,6 @@ class DiscoveryImplPlatform; namespace DeviceLayer { -static constexpr size_t kMaxLanguageTags = 254; // Maximum number of entry type 'ARRAY' supports static constexpr size_t kMaxCalendarTypes = 12; class PlatformManagerImpl; @@ -193,7 +192,6 @@ class PlatformManager bool IsChipStackLockedByCurrentThread() const; #endif - CHIP_ERROR GetSupportedLocales(AttributeList & supportedLocales); CHIP_ERROR GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes); @@ -449,11 +447,6 @@ inline CHIP_ERROR PlatformManager::StartChipTimer(System::Clock::Timeout duratio return static_cast(this)->_StartChipTimer(duration); } -inline CHIP_ERROR PlatformManager::GetSupportedLocales(AttributeList & supportedLocales) -{ - return static_cast(this)->_GetSupportedLocales(supportedLocales); -} - inline CHIP_ERROR PlatformManager::GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes) { diff --git a/src/include/platform/internal/GenericPlatformManagerImpl.h b/src/include/platform/internal/GenericPlatformManagerImpl.h index afe0e3d2d85edd..c89b2bc9ebdebd 100644 --- a/src/include/platform/internal/GenericPlatformManagerImpl.h +++ b/src/include/platform/internal/GenericPlatformManagerImpl.h @@ -60,7 +60,6 @@ class GenericPlatformManagerImpl void _ScheduleWork(AsyncWorkFunct workFunct, intptr_t arg); void _DispatchEvent(const ChipDeviceEvent * event); - CHIP_ERROR _GetSupportedLocales(AttributeList & supportedLocales); CHIP_ERROR _GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes); @@ -79,13 +78,6 @@ class GenericPlatformManagerImpl // Instruct the compiler to instantiate the template only when explicitly told to do so. extern template class GenericPlatformManagerImpl; -template -inline CHIP_ERROR -GenericPlatformManagerImpl::_GetSupportedLocales(AttributeList & supportedLocales) -{ - return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; -} - template inline CHIP_ERROR GenericPlatformManagerImpl::_GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes) diff --git a/src/lib/support/Span.h b/src/lib/support/Span.h index edcdd8ed353a62..a5dabe9084c74d 100644 --- a/src/lib/support/Span.h +++ b/src/lib/support/Span.h @@ -274,4 +274,15 @@ inline CHIP_ERROR CopySpanToMutableSpan(ByteSpan span_to_copy, MutableByteSpan & return CHIP_NO_ERROR; } +inline CHIP_ERROR CopyCharSpanToMutableCharSpan(CharSpan cspan_to_copy, MutableCharSpan & out_buf) +{ + VerifyOrReturnError(IsSpanUsable(cspan_to_copy), CHIP_ERROR_INVALID_ARGUMENT); + VerifyOrReturnError(out_buf.size() >= cspan_to_copy.size(), CHIP_ERROR_BUFFER_TOO_SMALL); + + memcpy(out_buf.data(), cspan_to_copy.data(), cspan_to_copy.size()); + out_buf.reduce_size(cspan_to_copy.size()); + + return CHIP_NO_ERROR; +} + } // namespace chip diff --git a/src/platform/Darwin/PlatformManagerImpl.cpp b/src/platform/Darwin/PlatformManagerImpl.cpp index b598ea0c4aa864..1b340e5e0fc875 100644 --- a/src/platform/Darwin/PlatformManagerImpl.cpp +++ b/src/platform/Darwin/PlatformManagerImpl.cpp @@ -136,23 +136,6 @@ CHIP_ERROR PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * event) return CHIP_NO_ERROR; } -CHIP_ERROR -PlatformManagerImpl::_GetSupportedLocales(AttributeList & supportedLocales) -{ - // In Darwin simulation, return following hardcoded list of Strings that are valid values for the ActiveLocale. - supportedLocales.add(CharSpan::fromCharString("Test")); - supportedLocales.add(CharSpan::fromCharString("en-US")); - supportedLocales.add(CharSpan::fromCharString("de-DE")); - supportedLocales.add(CharSpan::fromCharString("fr-FR")); - supportedLocales.add(CharSpan::fromCharString("en-GB")); - supportedLocales.add(CharSpan::fromCharString("es-ES")); - supportedLocales.add(CharSpan::fromCharString("zh-CN")); - supportedLocales.add(CharSpan::fromCharString("it-IT")); - supportedLocales.add(CharSpan::fromCharString("ja-JP")); - - return CHIP_NO_ERROR; -} - CHIP_ERROR PlatformManagerImpl::_GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes) diff --git a/src/platform/Darwin/PlatformManagerImpl.h b/src/platform/Darwin/PlatformManagerImpl.h index 7132ae50cab6ba..642f68cc3e3a89 100644 --- a/src/platform/Darwin/PlatformManagerImpl.h +++ b/src/platform/Darwin/PlatformManagerImpl.h @@ -64,7 +64,6 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener CHIP_ERROR _StartEventLoopTask(); CHIP_ERROR _StopEventLoopTask(); - CHIP_ERROR _GetSupportedLocales(AttributeList & supportedLocales); CHIP_ERROR _GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes); diff --git a/src/platform/Linux/DeviceInfoProviderImpl.cpp b/src/platform/Linux/DeviceInfoProviderImpl.cpp index dd75dc047db5d4..32987b0b1f962e 100644 --- a/src/platform/Linux/DeviceInfoProviderImpl.cpp +++ b/src/platform/Linux/DeviceInfoProviderImpl.cpp @@ -196,5 +196,76 @@ bool DeviceInfoProviderImpl::UserLabelIteratorImpl::Next(UserLabelType & output) } } +DeviceInfoProvider::SupportedLocalesIterator * DeviceInfoProviderImpl::IterateSupportedLocales() +{ + return new SupportedLocalesIteratorImpl(); +} + +size_t DeviceInfoProviderImpl::SupportedLocalesIteratorImpl::Count() +{ + // In Linux Simulation, return the size of the hardcoded list of Strings that are valid values for the ActiveLocale. + // {("en-US"), ("de-DE"), ("fr-FR"), ("en-GB"), ("es-ES"), ("zh-CN"), ("it-IT"), ("ja-JP")} + + return 8; +} + +bool DeviceInfoProviderImpl::SupportedLocalesIteratorImpl::Next(CharSpan & output) +{ + // In Linux simulation, return following hardcoded list of Strings that are valid values for the ActiveLocale. + CHIP_ERROR err = CHIP_NO_ERROR; + + const char * activeLocalePtr = nullptr; + + VerifyOrReturnError(mIndex < 8, false); + + switch (mIndex) + { + case 0: + activeLocalePtr = "en-US"; + break; + case 1: + activeLocalePtr = "de-DE"; + break; + case 2: + activeLocalePtr = "fr-FR"; + break; + case 3: + activeLocalePtr = "en-GB"; + break; + case 4: + activeLocalePtr = "es-ES"; + break; + case 5: + activeLocalePtr = "zh-CN"; + break; + case 6: + activeLocalePtr = "it-IT"; + break; + case 7: + activeLocalePtr = "ja-JP"; + break; + default: + err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND; + break; + } + + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(std::strlen(activeLocalePtr) <= kMaxActiveLocaleLength, false); + + Platform::CopyString(mActiveLocaleBuf, kMaxActiveLocaleLength + 1, activeLocalePtr); + + output = CharSpan::fromCharString(mActiveLocaleBuf); + + mIndex++; + + return true; + } + else + { + return false; + } +} + } // namespace DeviceLayer } // namespace chip diff --git a/src/platform/Linux/DeviceInfoProviderImpl.h b/src/platform/Linux/DeviceInfoProviderImpl.h index a9ddfb44521ccb..19d19c198625f6 100644 --- a/src/platform/Linux/DeviceInfoProviderImpl.h +++ b/src/platform/Linux/DeviceInfoProviderImpl.h @@ -30,6 +30,7 @@ class DeviceInfoProviderImpl : public DeviceInfoProvider // Iterators FixedLabelIterator * IterateFixedLabel(EndpointId endpoint) override; UserLabelIterator * IterateUserLabel(EndpointId endpoint) override; + SupportedLocalesIterator * IterateSupportedLocales() override; static DeviceInfoProviderImpl & GetDefaultInstance(); @@ -66,6 +67,19 @@ class DeviceInfoProviderImpl : public DeviceInfoProvider char mUserLabelValueBuf[kMaxLabelValueLength + 1]; }; + class SupportedLocalesIteratorImpl : public SupportedLocalesIterator + { + public: + SupportedLocalesIteratorImpl() = default; + size_t Count() override; + bool Next(CharSpan & output) override; + void Release() override { delete this; } + + private: + size_t mIndex = 0; + char mActiveLocaleBuf[kMaxActiveLocaleLength + 1]; + }; + CHIP_ERROR SetUserLabelLength(EndpointId endpoint, size_t val) override; CHIP_ERROR GetUserLabelLength(EndpointId endpoint, size_t & val) override; CHIP_ERROR SetUserLabelAt(EndpointId endpoint, size_t index, const UserLabelType & userLabel) override; diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp index 4fd08f16e3c557..97db3058c9fd64 100644 --- a/src/platform/Linux/PlatformManagerImpl.cpp +++ b/src/platform/Linux/PlatformManagerImpl.cpp @@ -242,22 +242,6 @@ CHIP_ERROR PlatformManagerImpl::_Shutdown() return Internal::GenericPlatformManagerImpl_POSIX::_Shutdown(); } -CHIP_ERROR -PlatformManagerImpl::_GetSupportedLocales(AttributeList & supportedLocales) -{ - // In Linux simulation, return following hardcoded list of Strings that are valid values for the ActiveLocale. - supportedLocales.add(CharSpan::fromCharString("en-US")); - supportedLocales.add(CharSpan::fromCharString("de-DE")); - supportedLocales.add(CharSpan::fromCharString("fr-FR")); - supportedLocales.add(CharSpan::fromCharString("en-GB")); - supportedLocales.add(CharSpan::fromCharString("es-ES")); - supportedLocales.add(CharSpan::fromCharString("zh-CN")); - supportedLocales.add(CharSpan::fromCharString("it-IT")); - supportedLocales.add(CharSpan::fromCharString("ja-JP")); - - return CHIP_NO_ERROR; -} - CHIP_ERROR PlatformManagerImpl::_GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes) diff --git a/src/platform/Linux/PlatformManagerImpl.h b/src/platform/Linux/PlatformManagerImpl.h index 5d29bb997cf0ee..b3f1a8cde0a688 100644 --- a/src/platform/Linux/PlatformManagerImpl.h +++ b/src/platform/Linux/PlatformManagerImpl.h @@ -65,7 +65,6 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener CHIP_ERROR _InitChipStack(); CHIP_ERROR _Shutdown(); - CHIP_ERROR _GetSupportedLocales(AttributeList & supportedLocales); CHIP_ERROR _GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes); diff --git a/src/platform/fake/PlatformManagerImpl.h b/src/platform/fake/PlatformManagerImpl.h index d020908a5d6305..ff2be4162fc6d7 100644 --- a/src/platform/fake/PlatformManagerImpl.h +++ b/src/platform/fake/PlatformManagerImpl.h @@ -100,11 +100,6 @@ class PlatformManagerImpl final : public PlatformManager CHIP_ERROR _StartChipTimer(System::Clock::Timeout duration) { return CHIP_ERROR_NOT_IMPLEMENTED; } - CHIP_ERROR _GetSupportedLocales(AttributeList & supportedLocales) - { - return CHIP_ERROR_NOT_IMPLEMENTED; - } - CHIP_ERROR _GetSupportedCalendarTypes( AttributeList & supportedCalendarTypes) {