Closure Dimension server cluster code #2
Conversation
src/app/clusters/closure-dimension-server/closure-dimension-server.h
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-server.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-server.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-server.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-server.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-server.cpp
Outdated
Show resolved
Hide resolved
|
Updated the dimension server code as per new template |
…on' into closure_dimension_cluster_server_code
src/app/clusters/closure-dimension-server/closure-dimension-delegate.h
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
| { | ||
| inline bool HasFeature(Feature feature) const { return featureMap & to_underlying(feature); } | ||
| uint32_t featureMap; | ||
| bool supportsOverflow; |
There was a problem hiding this comment.
why do you keep a var supportsOverflow but not the others, HasFeature can be used for all of them no?
There was a problem hiding this comment.
The overflow attribute is mandatory with the Rotation feature and optional with MotionLatching feature.
It is the device's responsibility to specify whether this attribute is supported. The values will be set using Clusterconformance struct.
src/app/clusters/closure-dimension-server/closure-dimension-matter-context.h
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-matter-context.h
Outdated
Show resolved
Hide resolved
| // { | ||
| // } | ||
|
|
||
| CHIP_ERROR ClusterStateAttributes::SetCurrent(GenericCurrentStruct & current) |
There was a problem hiding this comment.
Current attribute has the Quiet Reporting quality
Changes to this attribute SHALL only be marked as reportable in the following cases:
At most once every 5 seconds on changes when only the Position changes, or
When it changes from null to any other value and vice versa.
When Target.Position is reached.
When Target.Speed changes.
When Target.Latching changes.
There was a problem hiding this comment.
sure , will change it to quiet reporting.
| using namespace Protocols::InteractionModel; | ||
| namespace { | ||
|
|
||
| CHIP_ERROR TranslateErrorToIMStatus(CHIP_ERROR err) |
There was a problem hiding this comment.
This should be a switch case. Please add function comments that also documents the mapping between the chip error and the im error code.
src/app/clusters/closure-dimension-server/closure-dimension-server.cpp
Outdated
Show resolved
Hide resolved
| switch (aPath.mAttributeId) | ||
| { | ||
| case Current::Id: { | ||
| typedef GenericCurrentStruct T; |
There was a problem hiding this comment.
Splitting the Typedef from the function call doesn't server any purpose. This applies to all following cases.
src/app/clusters/closure-dimension-server/closure-dimension-server.h
Outdated
Show resolved
Hide resolved
| #include <app-common/zap-generated/ids/Clusters.h> | ||
| #include <app/AttributeAccessInterface.h> | ||
| #include <app/AttributeAccessInterfaceRegistry.h> | ||
| #include <app/CommandHandlerInterface.h> | ||
| #include <app/ConcreteCommandPath.h> | ||
| #include <app/data-model/Encode.h> | ||
| #include <app/util/config.h> | ||
| #include <lib/core/CHIPError.h> |
| uint32_t featureMap; | ||
| bool supportsOverflow; |
There was a problem hiding this comment.
We should always set a default value.
| { | ||
| bool supportsRotation = HasFeature(Feature::kRotation); | ||
| bool supportsMotionLatching = HasFeature(Feature::kMotionLatching); | ||
| if (supportsOverflow && !(supportsRotation || supportsMotionLatching)) |
There was a problem hiding this comment.
supportsOverflow isn't set any where and the default value isn't set. How does this behave?
src/app/clusters/closure-dimension-server/closure-dimension-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
Closure dimension cluster server code
Testing
added closurebase endpoint on all-cluster-app to include closure dimension cluster as server and built app locally.