Skip to content

Commit

Permalink
Fix/post pr 28707 comments (#28993)
Browse files Browse the repository at this point in the history
* AitQuality: Changed the feature attribute type to a BitMask of the Feature type. Fixed bug in the UpdateAirQuality method where checks where made against an incorrect enum.

* Added an out-of-band API to set the AirQuality attribute in the linux all-clusters-app.

* Added a check to ensure that the value given to UpdateAirQuality is not beyond the list of acceptable enums.

* Restyled by clang-format

* AirQuality: Renamed the out-of-band message's key

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

* AirQuality: Moved the check for the valid enum value in the switch statement.

* Restyled by clang-format

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
3 people authored and pull[bot] committed Sep 1, 2023
1 parent 5274e97 commit 1238848
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ namespace app {
namespace Clusters {
namespace AirQuality {

Instance * GetInstance();

void Shutdown();

} // namespace AirQuality
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ using namespace chip::app::Clusters::AirQuality;

Instance * gAirQualityCluster = nullptr;

Instance * AirQuality::GetInstance()
{
return gAirQualityCluster;
}

void AirQuality::Shutdown()
{
if (gAirQualityCluster != nullptr)
Expand All @@ -20,6 +25,6 @@ void emberAfAirQualityClusterInitCallback(chip::EndpointId endpointId)
VerifyOrDie(gAirQualityCluster == nullptr);
chip::BitMask<Feature, uint32_t> airQualityFeatures(Feature::kModerate, Feature::kFair, Feature::kVeryPoor,
Feature::kExtremelyPoor);
gAirQualityCluster = new Instance(1, airQualityFeatures.Raw());
gAirQualityCluster = new Instance(1, airQualityFeatures);
gAirQualityCluster->Init();
}
26 changes: 26 additions & 0 deletions examples/all-clusters-app/linux/AllClustersCommandDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <app/server/Server.h>
#include <platform/PlatformManager.h>

#include <air-quality-instance.h>
#include <dishwasher-mode.h>
#include <laundry-washer-mode.h>
#include <rvc-modes.h>
Expand Down Expand Up @@ -164,6 +165,19 @@ void AllClustersAppCommandHandler::HandleCommand(intptr_t context)
}
self->OnModeChangeHandler(device, type, mode);
}
else if (name == "SetAirQuality")
{
Json::Value jsonAirQualityEnum = self->mJsonValue["NewValue"];

if (jsonAirQualityEnum.isNull())
{
ChipLogError(NotSpecified, "The SetAirQuality command requires the NewValue key.");
}
else
{
self->OnAirQualityChange(static_cast<uint32_t>(jsonAirQualityEnum.asUInt()));
}
}
else
{
ChipLogError(NotSpecified, "Unhandled command: Should never happens");
Expand Down Expand Up @@ -414,6 +428,18 @@ void AllClustersAppCommandHandler::OnModeChangeHandler(std::string device, std::
}
}

void AllClustersAppCommandHandler::OnAirQualityChange(uint32_t aNewValue)
{
AirQuality::Instance * airQualityInstance = AirQuality::GetInstance();
Protocols::InteractionModel::Status status =
airQualityInstance->UpdateAirQuality(static_cast<AirQuality::AirQualityEnum>(aNewValue));

if (status != Protocols::InteractionModel::Status::Success)
{
ChipLogDetail(NotSpecified, "Invalid value: %u", aNewValue);
}
}

void AllClustersCommandDelegate::OnEventCommandReceived(const char * json)
{
auto handler = AllClustersAppCommandHandler::FromJSON(json);
Expand Down
5 changes: 5 additions & 0 deletions examples/all-clusters-app/linux/AllClustersCommandDelegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ class AllClustersAppCommandHandler
* Should be called when it is necessary to change the mode to manual operation.
*/
void OnModeChangeHandler(std::string device, std::string type, chip::app::DataModel::Nullable<uint8_t> mode);

/**
* Should be called when it is necessary to change the air quality attribute.
*/
void OnAirQualityChange(uint32_t aEnum);
};

class AllClustersCommandDelegate : public NamedPipeCommandDelegate
Expand Down
23 changes: 14 additions & 9 deletions src/app/clusters/air-quality-server/air-quality-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace app {
namespace Clusters {
namespace AirQuality {

Instance::Instance(EndpointId aEndpointId, uint32_t aFeature) :
Instance::Instance(EndpointId aEndpointId, BitMask<Feature> aFeature) :
AttributeAccessInterface(Optional<EndpointId>(aEndpointId), Id), mEndpointId(aEndpointId), mFeature(aFeature)
{}

Expand All @@ -51,9 +51,9 @@ CHIP_ERROR Instance::Init()
return CHIP_NO_ERROR;
}

bool Instance::HasFeature(AirQualityEnum aFeature) const
bool Instance::HasFeature(Feature aFeature) const
{
return mFeature & to_underlying(aFeature);
return mFeature.Has(aFeature);
}

Protocols::InteractionModel::Status Instance::UpdateAirQuality(AirQualityEnum aNewAirQuality)
Expand All @@ -62,35 +62,40 @@ Protocols::InteractionModel::Status Instance::UpdateAirQuality(AirQualityEnum aN
switch (aNewAirQuality)
{
case AirQualityEnum::kFair: {
if (!HasFeature(AirQualityEnum::kFair))
if (!HasFeature(Feature::kFair))
{
return Protocols::InteractionModel::Status::ConstraintError;
}
}
break;
case AirQualityEnum::kModerate: {
if (!HasFeature(AirQualityEnum::kModerate))
if (!HasFeature(Feature::kModerate))
{
return Protocols::InteractionModel::Status::ConstraintError;
}
}
break;
case AirQualityEnum::kPoor: {
if (!HasFeature(AirQualityEnum::kPoor))
case AirQualityEnum::kVeryPoor: {
if (!HasFeature(Feature::kVeryPoor))
{
return Protocols::InteractionModel::Status::ConstraintError;
}
}
break;
case AirQualityEnum::kExtremelyPoor: {
if (!HasFeature(AirQualityEnum::kExtremelyPoor))
if (!HasFeature(Feature::kExtremelyPoor))
{
return Protocols::InteractionModel::Status::ConstraintError;
}
}
break;
default:
case AirQualityEnum::kUnknown:
case AirQualityEnum::kGood:
case AirQualityEnum::kPoor:
break;
default: {
return Protocols::InteractionModel::Status::InvalidValue;
}
}

mAirQuality = aNewAirQuality;
Expand Down
6 changes: 3 additions & 3 deletions src/app/clusters/air-quality-server/air-quality-server.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class Instance : public AttributeAccessInterface
* @param aEndpointId The endpoint on which this cluster exists. This must match the zap configuration.
* @param aFeature The bitmask value that identifies which features are supported by this instance.
*/
Instance(EndpointId aEndpointId, uint32_t eFeature);
Instance(EndpointId aEndpointId, BitMask<Feature> aFeature);

~Instance() override;

Expand All @@ -49,7 +49,7 @@ class Instance : public AttributeAccessInterface
* Returns true if the feature is supported.
* @param feature the feature to check.
*/
bool HasFeature(AirQualityEnum) const;
bool HasFeature(Feature aFeature) const;

/**
* Sets the AirQuality attribute.
Expand All @@ -66,7 +66,7 @@ class Instance : public AttributeAccessInterface
private:
EndpointId mEndpointId;
AirQualityEnum mAirQuality = AirQualityEnum::kUnknown;
uint32_t mFeature;
BitMask<Feature> mFeature;

// AttributeAccessInterface
CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
Expand Down

0 comments on commit 1238848

Please sign in to comment.