From 6547bedf7bd402a4829e9ca74bf13e3a74d0e53e Mon Sep 17 00:00:00 2001 From: Matt Hazley Date: Thu, 31 Aug 2023 14:18:01 +0100 Subject: [PATCH] Improvements to API for Concentration Measurement Clusters (#28965) * Improved the use of the template args to compile out the entire function * Added a detail namespace with attribute classes that can be inherited based on feature settings * Fixed the hard to read constraints functions to be a bit more logical and better documented * Using overload of MatterReportingAttributeChangeCallback so as to avoid using ConcreteAttributePath * made the checking code in the constraints functions more succinct and removed the need for an old variable in the reporting check --- .../concentration-measurement-server.h | 425 ++++++++---------- 1 file changed, 182 insertions(+), 243 deletions(-) diff --git a/src/app/clusters/concentration-measurement-server/concentration-measurement-server.h b/src/app/clusters/concentration-measurement-server/concentration-measurement-server.h index 9c4bbeae6bf613..391674bb844d95 100644 --- a/src/app/clusters/concentration-measurement-server/concentration-measurement-server.h +++ b/src/app/clusters/concentration-measurement-server/concentration-measurement-server.h @@ -32,10 +32,56 @@ namespace app { namespace Clusters { namespace ConcentrationMeasurement { -struct DummyType +namespace Detail { + +struct DummyNumericMeasurementMembers +{ +}; + +struct DummyPeakMeasurementMembers +{ +}; + +struct DummyAverageMeasurementMembers +{ +}; + +struct DummyLevelIndicationMembers +{ +}; + +class NumericMeasurementMembers +{ +protected: + DataModel::Nullable mMeasuredValue; + DataModel::Nullable mMinMeasuredValue; + DataModel::Nullable mMaxMeasuredValue; + MeasurementUnitEnum mMeasurementUnit; + float mUncertainty; +}; + +class PeakMeasurementMembers +{ +protected: + DataModel::Nullable mPeakMeasuredValue; + uint32_t mPeakMeasuredValueWindow; +}; + +class AverageMeasurementMembers +{ +protected: + DataModel::Nullable mAverageMeasuredValue; + uint32_t mAverageMeasuredValueWindow; +}; + +class LevelIndicationMembers { +protected: + LevelValueEnum mLevel; }; +} // namespace Detail + /** * This class provides the base implementation for the server side of the Concentration Measurement cluster as well as an API for * setting the values of the attributes. @@ -49,7 +95,14 @@ struct DummyType */ template -class Instance : public AttributeAccessInterface +class Instance + : public AttributeAccessInterface, + protected std::conditional_t, + protected std::conditional_t, + protected std::conditional_t, + protected std::conditional_t { private: static const int WINDOW_MAX = 604800; @@ -57,18 +110,7 @@ class Instance : public AttributeAccessInterface EndpointId mEndpointId{}; ClusterId mClusterId{}; - // Attribute data store MeasurementMediumEnum mMeasurementMedium; - std::conditional_t, DummyType> mMeasuredValue; - std::conditional_t, DummyType> mMinMeasuredValue; - std::conditional_t, DummyType> mMaxMeasuredValue; - std::conditional_t mMeasurementUnit; - std::conditional_t mUncertainty; - std::conditional_t, DummyType> mPeakMeasuredValue; - std::conditional_t mPeakMeasuredValueWindow; - std::conditional_t, DummyType> mAverageMeasuredValue; - std::conditional_t mAverageMeasuredValueWindow; - std::conditional_t mLevel; uint32_t mFeature = 0; @@ -80,67 +122,70 @@ class Instance : public AttributeAccessInterface case Attributes::MeasuredValue::Id: if constexpr (NumericMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mMeasuredValue)); + ReturnErrorOnFailure(aEncoder.Encode(this->mMeasuredValue)); } break; case Attributes::MinMeasuredValue::Id: if constexpr (NumericMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mMinMeasuredValue)); + ReturnErrorOnFailure(aEncoder.Encode(this->mMinMeasuredValue)); } break; case Attributes::MaxMeasuredValue::Id: if constexpr (NumericMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mMaxMeasuredValue)); + ReturnErrorOnFailure(aEncoder.Encode(this->mMaxMeasuredValue)); } break; case Attributes::Uncertainty::Id: if constexpr (NumericMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mUncertainty)); + ReturnErrorOnFailure(aEncoder.Encode(this->mUncertainty)); } break; case Attributes::MeasurementUnit::Id: - ReturnErrorOnFailure(aEncoder.Encode(mMeasurementUnit)); - break; + if constexpr (NumericMeasurementEnabled) + { + ReturnErrorOnFailure(aEncoder.Encode(this->mMeasurementUnit)); + break; + } case Attributes::PeakMeasuredValue::Id: if constexpr (PeakMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mPeakMeasuredValue)); + ReturnErrorOnFailure(aEncoder.Encode(this->mPeakMeasuredValue)); } break; case Attributes::PeakMeasuredValueWindow::Id: if constexpr (PeakMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mPeakMeasuredValueWindow)); + ReturnErrorOnFailure(aEncoder.Encode(this->mPeakMeasuredValueWindow)); } break; case Attributes::AverageMeasuredValue::Id: if constexpr (AverageMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mAverageMeasuredValue)); + ReturnErrorOnFailure(aEncoder.Encode(this->mAverageMeasuredValue)); } break; case Attributes::AverageMeasuredValueWindow::Id: if constexpr (AverageMeasurementEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mAverageMeasuredValueWindow)); + ReturnErrorOnFailure(aEncoder.Encode(this->mAverageMeasuredValueWindow)); } break; case Attributes::LevelValue::Id: if constexpr (LevelIndicationEnabled) { - ReturnErrorOnFailure(aEncoder.Encode(mLevel)); + ReturnErrorOnFailure(aEncoder.Encode(this->mLevel)); } break; @@ -210,58 +255,42 @@ class Instance : public AttributeAccessInterface }; /** - * This checks is a given value is within the min and max constraints. + * This checks if a given nullable float is within the min and max constraints or two other nullable floats. * @param value The value to check. * @param minValue The minimum value. * @param maxValue The maximum value. - * @return true if the value is within the min and max constraints. + * @return true if the value is within the min and max constraints. If either of the pair of values being compared is null, + * that's considered to be within the constraint. */ static bool CheckConstraintMinMax(DataModel::Nullable value, DataModel::Nullable minValue, DataModel::Nullable maxValue) { - if (!minValue.IsNull() && !value.IsNull() && (minValue.Value() > value.Value())) - { - return false; - } - - if (!maxValue.IsNull() && !value.IsNull() && (maxValue.Value() < value.Value())) - { - return false; - } - - return true; + return (minValue.IsNull() || value.IsNull() || (value.Value() >= minValue.Value())) && + (maxValue.IsNull() || value.IsNull() || (value.Value() <= maxValue.Value())); }; /** - * This checks is a given value is greater than a given value. + * This checks if a given nullable float is less than or equal to another given nullable float. * @param value The value to check. - * @param valueToBeGreaterThan The value to be greater than. - * @return true if the value is greater than the given value. + * @param valueToBeLessThanOrEqualTo The value to be less than or equal to. + * @return true if value is less than or equal to valueToBeLessThanOrEqualTo, or if either of the values is Null. */ - static bool CheckConstraintsGreaterThan(DataModel::Nullable value, DataModel::Nullable valueToBeGreaterThan) + static bool CheckConstraintsLessThanOrEqualTo(DataModel::Nullable value, + DataModel::Nullable valueToBeLessThanOrEqualTo) { - if (!valueToBeGreaterThan.IsNull() && !value.IsNull() && (valueToBeGreaterThan.Value() > value.Value())) - { - return false; - } - - return true; + return valueToBeLessThanOrEqualTo.IsNull() || value.IsNull() || (value.Value() <= valueToBeLessThanOrEqualTo.Value()); }; /** - * This checks is a given value is less than a given value. + * This checks if a given nullable float is greater than or equal to another given nullable float. * @param value The value to check. - * @param valueToBeLessThan The value to be less than. - * @return true if the value is less than the given value. + * @param valueToBeGreaterThanOrEqualTo The value to be greater than or equal to. + * @return true if value is greater than or equal to valueToBeGreaterThanOrEqualTo, or if either of the values is Null. */ - static bool CheckConstraintsLessThan(DataModel::Nullable value, DataModel::Nullable valueToBeLessThan) + static bool CheckConstraintsGreaterThanOrEqualTo(DataModel::Nullable value, + DataModel::Nullable valueToBeGreaterThanOrEqualTo) { - if (!valueToBeLessThan.IsNull() && !value.IsNull() && (valueToBeLessThan.Value() < value.Value())) - { - return false; - } - - return true; + return valueToBeGreaterThanOrEqualTo.IsNull() || value.IsNull() || (value.Value() >= valueToBeGreaterThanOrEqualTo.Value()); }; public: @@ -289,8 +318,10 @@ class Instance : public AttributeAccessInterface Instance(EndpointId aEndpointId, ClusterId aClusterId, MeasurementMediumEnum aMeasurementMedium, MeasurementUnitEnum aMeasurementUnit) : AttributeAccessInterface(Optional(aEndpointId), aClusterId), - mEndpointId(aEndpointId), mClusterId(aClusterId), mMeasurementMedium(aMeasurementMedium), - mMeasurementUnit(aMeasurementUnit){}; + mEndpointId(aEndpointId), mClusterId(aClusterId), mMeasurementMedium(aMeasurementMedium) + { + this->mMeasurementUnit = aMeasurementUnit; + }; ~Instance() override { unregisterAttributeAccessOverride(this); }; @@ -325,256 +356,164 @@ class Instance : public AttributeAccessInterface return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetMeasuredValue(DataModel::Nullable aMeasuredValue) { - if constexpr (NumericMeasurementEnabled) + if (!CheckConstraintMinMax(aMeasuredValue, this->mMinMeasuredValue, this->mMaxMeasuredValue)) { - if (!CheckConstraintMinMax(aMeasuredValue, mMinMeasuredValue, mMaxMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } + return CHIP_ERROR_INVALID_ARGUMENT; + } - DataModel::Nullable oldValue = mMeasuredValue; - mMeasuredValue = aMeasuredValue; + // Check to see if a change has ocurred + VerifyOrReturnError(this->mMeasuredValue != aMeasuredValue, CHIP_NO_ERROR); + this->mMeasuredValue = aMeasuredValue; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::MeasuredValue::Id); - if (oldValue != mMeasuredValue) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::MeasuredValue::Id); - MatterReportingAttributeChangeCallback(path); - } - - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetMinMeasuredValue(DataModel::Nullable aMinMeasuredValue) { - if constexpr (NumericMeasurementEnabled) + if (!CheckConstraintsLessThanOrEqualTo(aMinMeasuredValue, this->mMaxMeasuredValue)) { - if (!CheckConstraintsLessThan(aMinMeasuredValue, mMaxMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - if (!CheckConstraintsLessThan(aMinMeasuredValue, mMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - DataModel::Nullable oldValue = mMinMeasuredValue; - mMinMeasuredValue = aMinMeasuredValue; - - if (oldValue != mMinMeasuredValue) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::MinMeasuredValue::Id); - MatterReportingAttributeChangeCallback(path); - } - - return CHIP_NO_ERROR; + return CHIP_ERROR_INVALID_ARGUMENT; } - else + + if (!CheckConstraintsLessThanOrEqualTo(aMinMeasuredValue, this->mMeasuredValue)) { - return CHIP_ERROR_INCORRECT_STATE; + return CHIP_ERROR_INVALID_ARGUMENT; } + + // Check to see if a change has ocurred + VerifyOrReturnError(this->mMinMeasuredValue != aMinMeasuredValue, CHIP_NO_ERROR); + this->mMinMeasuredValue = aMinMeasuredValue; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::MinMeasuredValue::Id); + + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetMaxMeasuredValue(DataModel::Nullable aMaxMeasuredValue) { - if constexpr (NumericMeasurementEnabled) + if (!CheckConstraintsGreaterThanOrEqualTo(aMaxMeasuredValue, this->mMinMeasuredValue)) { - if (!CheckConstraintsGreaterThan(aMaxMeasuredValue, mMinMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - if (!CheckConstraintsGreaterThan(aMaxMeasuredValue, mMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - DataModel::Nullable oldValue = mMaxMeasuredValue; - mMaxMeasuredValue = aMaxMeasuredValue; - - if (oldValue != mMaxMeasuredValue) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::MaxMeasuredValue::Id); - MatterReportingAttributeChangeCallback(path); - } - - return CHIP_NO_ERROR; + return CHIP_ERROR_INVALID_ARGUMENT; } - else + + if (!CheckConstraintsGreaterThanOrEqualTo(aMaxMeasuredValue, this->mMeasuredValue)) { - return CHIP_ERROR_INCORRECT_STATE; + return CHIP_ERROR_INVALID_ARGUMENT; } + + // Check to see if a change has ocurred + VerifyOrReturnError(this->mMaxMeasuredValue != aMaxMeasuredValue, CHIP_NO_ERROR); + this->mMaxMeasuredValue = aMaxMeasuredValue; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::MaxMeasuredValue::Id); + + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetUncertainty(float aUncertainty) { - if constexpr (NumericMeasurementEnabled) - { - float oldValue = mUncertainty; - mUncertainty = aUncertainty; + // Check to see if a change has ocurred + VerifyOrReturnError(this->mUncertainty != aUncertainty, CHIP_NO_ERROR); + this->mUncertainty = aUncertainty; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::Uncertainty::Id); - if (oldValue != mUncertainty) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::Uncertainty::Id); - MatterReportingAttributeChangeCallback(path); - } - - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetPeakMeasuredValue(DataModel::Nullable aPeakMeasuredValue) { - if constexpr (PeakMeasurementEnabled) + if (!CheckConstraintMinMax(aPeakMeasuredValue, this->mMinMeasuredValue, this->mMaxMeasuredValue)) { - if (!CheckConstraintMinMax(aPeakMeasuredValue, mMinMeasuredValue, mMaxMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - DataModel::Nullable oldValue = mPeakMeasuredValue; - mPeakMeasuredValue = aPeakMeasuredValue; + return CHIP_ERROR_INVALID_ARGUMENT; + } - if (oldValue != mPeakMeasuredValue) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::PeakMeasuredValue::Id); - MatterReportingAttributeChangeCallback(path); - } + // Check to see if a change has ocurred + VerifyOrReturnError(this->mPeakMeasuredValue != aPeakMeasuredValue, CHIP_NO_ERROR); + this->mPeakMeasuredValue = aPeakMeasuredValue; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::PeakMeasuredValue::Id); - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetPeakMeasuredValueWindow(uint32_t aPeakMeasuredValueWindow) { - if constexpr (PeakMeasurementEnabled) + if (aPeakMeasuredValueWindow > WINDOW_MAX) { - if (aPeakMeasuredValueWindow > WINDOW_MAX) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } + return CHIP_ERROR_INVALID_ARGUMENT; + } - uint32_t oldValue = mPeakMeasuredValueWindow; - mPeakMeasuredValueWindow = aPeakMeasuredValueWindow; + // Check to see if a change has ocurred + VerifyOrReturnError(this->mPeakMeasuredValueWindow != aPeakMeasuredValueWindow, CHIP_NO_ERROR); + this->mPeakMeasuredValueWindow = aPeakMeasuredValueWindow; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::PeakMeasuredValueWindow::Id); - if (oldValue != mPeakMeasuredValueWindow) - { - ConcreteAttributePath path = - ConcreteAttributePath(mEndpointId, mClusterId, Attributes::PeakMeasuredValueWindow::Id); - MatterReportingAttributeChangeCallback(path); - } - - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetAverageMeasuredValue(DataModel::Nullable aAverageMeasuredValue) { - if constexpr (AverageMeasurementEnabled) + if (!CheckConstraintMinMax(aAverageMeasuredValue, this->mMinMeasuredValue, this->mMaxMeasuredValue)) { - if (!CheckConstraintMinMax(aAverageMeasuredValue, mMinMeasuredValue, mMaxMeasuredValue)) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } - - DataModel::Nullable oldValue = mAverageMeasuredValue; - mAverageMeasuredValue = aAverageMeasuredValue; + return CHIP_ERROR_INVALID_ARGUMENT; + } - if (oldValue != mAverageMeasuredValue) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::AverageMeasuredValue::Id); - MatterReportingAttributeChangeCallback(path); - } + // Check to see if a change has ocurred + VerifyOrReturnError(this->mAverageMeasuredValue != aAverageMeasuredValue, CHIP_NO_ERROR); + this->mAverageMeasuredValue = aAverageMeasuredValue; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::AverageMeasuredValue::Id); - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetAverageMeasuredValueWindow(uint32_t aAverageMeasuredValueWindow) { - if constexpr (AverageMeasurementEnabled) + if (aAverageMeasuredValueWindow > WINDOW_MAX) { - if (aAverageMeasuredValueWindow > WINDOW_MAX) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } + return CHIP_ERROR_INVALID_ARGUMENT; + } - uint32_t oldValue = mAverageMeasuredValueWindow; - mAverageMeasuredValueWindow = aAverageMeasuredValueWindow; + // Check to see if a change has ocurred + VerifyOrReturnError(this->mAverageMeasuredValueWindow != aAverageMeasuredValueWindow, CHIP_NO_ERROR); + this->mAverageMeasuredValueWindow = aAverageMeasuredValueWindow; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::AverageMeasuredValueWindow::Id); - if (oldValue != mAverageMeasuredValueWindow) - { - ConcreteAttributePath path = - ConcreteAttributePath(mEndpointId, mClusterId, Attributes::AverageMeasuredValueWindow::Id); - MatterReportingAttributeChangeCallback(path); - } - - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; + template > CHIP_ERROR SetLevelValue(LevelValueEnum aLevel) { - if constexpr (LevelIndicationEnabled) + if constexpr (!MediumLevelEnabled) { - if constexpr (!MediumLevelEnabled) + if (aLevel == LevelValueEnum::kMedium) { - if (aLevel == LevelValueEnum::kMedium) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } + return CHIP_ERROR_INVALID_ARGUMENT; } + } - if constexpr (!CriticalLevelEnabled) + if constexpr (!CriticalLevelEnabled) + { + if (aLevel == LevelValueEnum::kCritical) { - if (aLevel == LevelValueEnum::kCritical) - { - return CHIP_ERROR_INVALID_ARGUMENT; - } + return CHIP_ERROR_INVALID_ARGUMENT; } + } - LevelValueEnum oldValue = mLevel; - mLevel = aLevel; - - if (oldValue != mLevel) - { - ConcreteAttributePath path = ConcreteAttributePath(mEndpointId, mClusterId, Attributes::LevelValue::Id); - MatterReportingAttributeChangeCallback(path); - } + // Check to see if a change has ocurred + VerifyOrReturnError(this->mLevel != aLevel, CHIP_NO_ERROR); + this->mLevel = aLevel; + MatterReportingAttributeChangeCallback(mEndpointId, mClusterId, Attributes::LevelValue::Id); - return CHIP_NO_ERROR; - } - else - { - return CHIP_ERROR_INCORRECT_STATE; - } + return CHIP_NO_ERROR; }; };