Enable op state pause and resume for compatible states (#30795)
* Updated the OpState HandlePauseState method to enable new pause-compatible state to receive this command.
* Made OpState unchangable internal variables const.
* Updated the OpState HandleResumeState method to enable new resume-compatible states to receive this command.
* Restyled by clang-format
* Skip resume callback call if state is Running.
* Changed the OpState server API such that all derived clusters have their own class. This separetes out the cluster specific logic, reducing the footprint cost.
* Added a helper readme that explains how to use and extend the operational state server.
* Fixed permissions of the OpState derived cluster methods.
* Updated the rvc-app example following the changes to the operational state server API
* Updated the all-clusters--app example following the changes to the operational state server API
* Restyled by whitespace
* Restyled by clang-format
* Restyled by prettier-markdown
* Updated the dishwasher-app example following the changes to the operational state server API
* Restyled by clang-format
* Fixed the ameba build of all-clusters-app.
* Restyled by clang-format
* Fixed the ameba build of all-clusters-app.
* Fixed a typo in the operational state server doc.
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/examples/all-clusters-app/all-clusters-common/src/operational-state-delegate-impl.cpp b/examples/all-clusters-app/all-clusters-common/src/operational-state-delegate-impl.cpp
index f8a465e..ec3e392 100644
--- a/examples/all-clusters-app/all-clusters-common/src/operational-state-delegate-impl.cpp
+++ b/examples/all-clusters-app/all-clusters-common/src/operational-state-delegate-impl.cpp
@@ -125,7 +125,7 @@
gOperationalStateDelegate = new OperationalStateDelegate;
EndpointId operationalStateEndpoint = 0x01;
- gOperationalStateInstance = new Instance(gOperationalStateDelegate, operationalStateEndpoint, Clusters::OperationalState::Id);
+ gOperationalStateInstance = new OperationalState::Instance(gOperationalStateDelegate, operationalStateEndpoint);
gOperationalStateInstance->SetOperationalState(to_underlying(OperationalState::OperationalStateEnum::kStopped));
@@ -158,8 +158,7 @@
gRvcOperationalStateDelegate = new RvcOperationalStateDelegate;
EndpointId operationalStateEndpoint = 0x01;
- gRvcOperationalStateInstance =
- new Instance(gRvcOperationalStateDelegate, operationalStateEndpoint, Clusters::RvcOperationalState::Id);
+ gRvcOperationalStateInstance = new RvcOperationalState::Instance(gRvcOperationalStateDelegate, operationalStateEndpoint);
gRvcOperationalStateInstance->SetOperationalState(to_underlying(OperationalState::OperationalStateEnum::kStopped));
diff --git a/examples/all-clusters-app/ameba/main/OperationalStateManager.cpp b/examples/all-clusters-app/ameba/main/OperationalStateManager.cpp
index 9e4001e..68ee456 100644
--- a/examples/all-clusters-app/ameba/main/OperationalStateManager.cpp
+++ b/examples/all-clusters-app/ameba/main/OperationalStateManager.cpp
@@ -126,7 +126,7 @@
gOperationalStateDelegate = new OperationalStateDelegate;
EndpointId operationalStateEndpoint = 0x01;
- gOperationalStateInstance = new Instance(gOperationalStateDelegate, operationalStateEndpoint, Clusters::OperationalState::Id);
+ gOperationalStateInstance = new OperationalState::Instance(gOperationalStateDelegate, operationalStateEndpoint);
gOperationalStateInstance->SetOperationalState(to_underlying(OperationalState::OperationalStateEnum::kStopped));
@@ -135,10 +135,10 @@
// Init RVC Operational State cluster
-static OperationalState::Instance * gRvcOperationalStateInstance = nullptr;
-static RvcOperationalStateDelegate * gRvcOperationalStateDelegate = nullptr;
+static RvcOperationalState::Instance * gRvcOperationalStateInstance = nullptr;
+static RvcOperationalStateDelegate * gRvcOperationalStateDelegate = nullptr;
-OperationalState::Instance * OperationalState::GetRVCOperationalStateInstance()
+RvcOperationalState::Instance * RvcOperationalState::GetRvcOperationalStateInstance()
{
return gRvcOperationalStateInstance;
}
@@ -164,8 +164,7 @@
gRvcOperationalStateDelegate = new RvcOperationalStateDelegate;
EndpointId operationalStateEndpoint = 0x01;
- gRvcOperationalStateInstance =
- new Instance(gRvcOperationalStateDelegate, operationalStateEndpoint, Clusters::RvcOperationalState::Id);
+ gRvcOperationalStateInstance = new RvcOperationalState::Instance(gRvcOperationalStateDelegate, operationalStateEndpoint);
gRvcOperationalStateInstance->SetOperationalState(to_underlying(OperationalState::OperationalStateEnum::kStopped));
diff --git a/examples/all-clusters-app/ameba/main/include/ManualOperationalStateCommand.h b/examples/all-clusters-app/ameba/main/include/ManualOperationalStateCommand.h
index 1f6d899..5d43735 100644
--- a/examples/all-clusters-app/ameba/main/include/ManualOperationalStateCommand.h
+++ b/examples/all-clusters-app/ameba/main/include/ManualOperationalStateCommand.h
@@ -137,7 +137,7 @@
uint32_t state = atoi(argv[0]);
CHIP_ERROR err;
- err = GetRVCOperationalStateInstance()->SetOperationalState(state);
+ err = RvcOperationalState::GetRvcOperationalStateInstance()->SetOperationalState(state);
if (err != CHIP_NO_ERROR)
{
@@ -178,7 +178,7 @@
break;
}
- GetRVCOperationalStateInstance()->OnOperationalErrorDetected(err);
+ RvcOperationalState::GetRvcOperationalStateInstance()->OnOperationalErrorDetected(err);
return CHIP_NO_ERROR;
}
diff --git a/examples/all-clusters-app/ameba/main/include/OperationalStateManager.h b/examples/all-clusters-app/ameba/main/include/OperationalStateManager.h
index e98d321..875534c 100644
--- a/examples/all-clusters-app/ameba/main/include/OperationalStateManager.h
+++ b/examples/all-clusters-app/ameba/main/include/OperationalStateManager.h
@@ -32,7 +32,6 @@
namespace OperationalState {
Instance * GetOperationalStateInstance();
-Instance * GetRVCOperationalStateInstance();
// This is an application level delegate to handle operational state commands according to the specific business logic.
class GenericOperationalStateDelegateImpl : public Delegate
@@ -124,6 +123,8 @@
namespace RvcOperationalState {
+Instance * GetRvcOperationalStateInstance();
+
// This is an application level delegate to handle operational state commands according to the specific business logic.
class RvcOperationalStateDelegate : public OperationalState::GenericOperationalStateDelegateImpl
{
diff --git a/examples/dishwasher-app/dishwasher-common/src/operational-state-delegate-impl.cpp b/examples/dishwasher-app/dishwasher-common/src/operational-state-delegate-impl.cpp
index 0b57ece..815ab69 100644
--- a/examples/dishwasher-app/dishwasher-common/src/operational-state-delegate-impl.cpp
+++ b/examples/dishwasher-app/dishwasher-common/src/operational-state-delegate-impl.cpp
@@ -122,7 +122,7 @@
gOperationalStateDelegate = new OperationalStateDelegate;
EndpointId operationalStateEndpoint = 0x01;
- gOperationalStateInstance = new Instance(gOperationalStateDelegate, operationalStateEndpoint, Clusters::OperationalState::Id);
+ gOperationalStateInstance = new Instance(gOperationalStateDelegate, operationalStateEndpoint);
gOperationalStateInstance->SetOperationalState(to_underlying(OperationalState::OperationalStateEnum::kStopped));
diff --git a/examples/rvc-app/rvc-common/include/rvc-device.h b/examples/rvc-app/rvc-common/include/rvc-device.h
index 6064bce..79d114a 100644
--- a/examples/rvc-app/rvc-common/include/rvc-device.h
+++ b/examples/rvc-app/rvc-common/include/rvc-device.h
@@ -19,7 +19,7 @@
ModeBase::Instance mCleanModeInstance;
RvcOperationalState::RvcOperationalStateDelegate mOperationalStateDelegate;
- OperationalState::Instance mOperationalStateInstance;
+ RvcOperationalState::Instance mOperationalStateInstance;
bool mDocked = false;
bool mCharging = false;
@@ -33,7 +33,7 @@
explicit RvcDevice(EndpointId aRvcClustersEndpoint) :
mRunModeDelegate(), mRunModeInstance(&mRunModeDelegate, aRvcClustersEndpoint, RvcRunMode::Id, 0), mCleanModeDelegate(),
mCleanModeInstance(&mCleanModeDelegate, aRvcClustersEndpoint, RvcCleanMode::Id, 0), mOperationalStateDelegate(),
- mOperationalStateInstance(&mOperationalStateDelegate, aRvcClustersEndpoint, RvcOperationalState::Id)
+ mOperationalStateInstance(&mOperationalStateDelegate, aRvcClustersEndpoint)
{
// set the current-mode at start-up
mRunModeInstance.UpdateCurrentMode(RvcRunMode::ModeIdle);
diff --git a/src/app/clusters/operational-state-server/README.md b/src/app/clusters/operational-state-server/README.md
new file mode 100644
index 0000000..8dd6d59
--- /dev/null
+++ b/src/app/clusters/operational-state-server/README.md
@@ -0,0 +1,36 @@
+# The Operational State cluster and its derived clusters
+
+The Operational State cluster is a normal cluster with ID 0x0060. It is also a
+base cluster from with other operational state clusters are derived.
+
+## How to use an Operational State cluster
+
+All Operational State derived clusters have their own `Instance` class within
+their respective namespace. This class is used to manage the SDK side of the
+cluster. This class requires an `OperationalState::Delegate` where the
+application specific logic is implemented.
+
+To use an Operational State cluster
+
+- Create a class that inherits the `OperationalState::Delegate` class.
+- For this class, implement the necessary virtual methods.
+- In some translation unit (.c or .cpp file), instantiate the delegate class.
+- Instantiate the `Instance` class for your Operational State cluster using
+ the delegate instance.
+- Call the `Init()` method of your instance after the root `Server::Init()`.
+- Alternatively, the last two steps can be done in the
+ `emberAf<ClusterName>ClusterInitCallback` function.
+
+**Note** Zap accessor functions for these clusters do not exist. Use the
+instance's `Set...` and `Get...` functions to access the attributes.
+
+## How to add new derived clusters
+
+Once an Operational State derived cluster has been defined in the spec, add the
+implementation using the following steps
+
+1. Translate the spec as an XML in `src/app/zap-templates/zcl/data-model/chip`.
+ You can look at similar files on how to do this.
+2. Regenerate the zap code.
+3. Implement an `Instance` class in your cluster's namespace.
+4. Extend the all-clusters-app example to include your new cluster.
diff --git a/src/app/clusters/operational-state-server/operational-state-server.cpp b/src/app/clusters/operational-state-server/operational-state-server.cpp
index 0c8c4f9..c7b7cd1 100644
--- a/src/app/clusters/operational-state-server/operational-state-server.cpp
+++ b/src/app/clusters/operational-state-server/operational-state-server.cpp
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-/****************************************************************************
+/****************************************************************************'
* @file
* @brief Implementation for the Operational State Server Cluster
***************************************************************************/
@@ -42,6 +42,8 @@
mDelegate->SetInstance(this);
}
+Instance::Instance(Delegate * aDelegate, EndpointId aEndpointId) : Instance(aDelegate, aEndpointId, OperationalState::Id) {}
+
Instance::~Instance()
{
InteractionModelEngine::GetInstance()->UnregisterCommandHandler(this);
@@ -337,11 +339,24 @@
GenericOperationalError err(to_underlying(ErrorStateEnum::kNoError));
uint8_t opState = GetCurrentOperationalState();
- if (opState != to_underlying(OperationalStateEnum::kPaused) && opState != to_underlying(OperationalStateEnum::kRunning))
+ // Handle Operational State Pause-incompatible states.
+ if (opState == to_underlying(OperationalStateEnum::kStopped) || opState == to_underlying(OperationalStateEnum::kError))
{
err.Set(to_underlying(ErrorStateEnum::kCommandInvalidInState));
}
- else if (opState == to_underlying(OperationalStateEnum::kRunning))
+
+ // Handle Pause-incompatible states for derived clusters.
+ if (opState >= DerivedClusterNumberSpaceStart && opState < VendorNumberSpaceStart)
+ {
+ if (!IsDerivedClusterStatePauseCompatible(opState))
+ {
+ err.Set(to_underlying(ErrorStateEnum::kCommandInvalidInState));
+ }
+ }
+
+ // If the error is still NoError, we can call the delegate's handle function.
+ // If the current state is Paused we can skip this call.
+ if (err.errorStateID == 0 && opState != to_underlying(OperationalStateEnum::kPaused))
{
mDelegate->HandlePauseStateCallback(err);
}
@@ -395,11 +410,24 @@
GenericOperationalError err(to_underlying(ErrorStateEnum::kNoError));
uint8_t opState = GetCurrentOperationalState();
- if (opState != to_underlying(OperationalStateEnum::kPaused) && opState != to_underlying(OperationalStateEnum::kRunning))
+ // Handle Operational State Resume-incompatible states.
+ if (opState == to_underlying(OperationalStateEnum::kStopped) || opState == to_underlying(OperationalStateEnum::kError))
{
err.Set(to_underlying(ErrorStateEnum::kCommandInvalidInState));
}
- else if (opState == to_underlying(OperationalStateEnum::kPaused))
+
+ // Handle Resume-incompatible states for derived clusters.
+ if (opState >= DerivedClusterNumberSpaceStart && opState < VendorNumberSpaceStart)
+ {
+ if (!IsDerivedClusterStateResumeCompatible(opState))
+ {
+ err.Set(to_underlying(ErrorStateEnum::kCommandInvalidInState));
+ }
+ }
+
+ // If the error is still NoError, we can call the delegate's handle function.
+ // If the current state is Running we can skip this call.
+ if (err.errorStateID == 0 && opState != to_underlying(OperationalStateEnum::kRunning))
{
mDelegate->HandleResumeStateCallback(err);
}
@@ -409,3 +437,16 @@
ctx.mCommandHandler.AddResponse(ctx.mRequestPath, response);
}
+
+// RvcOperationalState
+
+bool RvcOperationalState::Instance::IsDerivedClusterStatePauseCompatible(uint8_t aState)
+{
+ return aState == to_underlying(RvcOperationalState::OperationalStateEnum::kSeekingCharger);
+}
+
+bool RvcOperationalState::Instance::IsDerivedClusterStateResumeCompatible(uint8_t aState)
+{
+ return (aState == to_underlying(RvcOperationalState::OperationalStateEnum::kCharging) ||
+ aState == to_underlying(RvcOperationalState::OperationalStateEnum::kDocked));
+}
diff --git a/src/app/clusters/operational-state-server/operational-state-server.h b/src/app/clusters/operational-state-server/operational-state-server.h
index 2e0875d..88725a1 100644
--- a/src/app/clusters/operational-state-server/operational-state-server.h
+++ b/src/app/clusters/operational-state-server/operational-state-server.h
@@ -29,6 +29,9 @@
namespace Clusters {
namespace OperationalState {
+const uint8_t DerivedClusterNumberSpaceStart = 0x40;
+const uint8_t VendorNumberSpaceStart = 0x80;
+
class Uncopyable
{
protected:
@@ -50,15 +53,15 @@
{
public:
/**
- * Creates an operational state cluster instance. The Init() function needs to be called for this instance
- * to be registered and called by the interaction model at the appropriate times.
+ * Creates an operational state cluster instance.
+ * The Init() function needs to be called for this instance to be registered and called by the
+ * interaction model at the appropriate times.
* It is possible to set the CurrentPhase and OperationalState via the Set... methods before calling Init().
* @param aDelegate A pointer to the delegate to be used by this server.
* Note: the caller must ensure that the delegate lives throughout the instance's lifetime.
* @param aEndpointId The endpoint on which this cluster exists. This must match the zap configuration.
- * @param aClusterId The ID of the operational state derived cluster to be instantiated.
*/
- Instance(Delegate * aDelegate, EndpointId aEndpointId, ClusterId aClusterId);
+ Instance(Delegate * aDelegate, EndpointId aEndpointId);
~Instance() override;
@@ -148,11 +151,40 @@
*/
bool IsSupportedOperationalState(uint8_t aState);
+protected:
+ /**
+ * Creates an operational state cluster instance for a given cluster ID.
+ * The Init() function needs to be called for this instance to be registered and called by the
+ * interaction model at the appropriate times.
+ * It is possible to set the CurrentPhase and OperationalState via the Set... methods before calling Init().
+ * @param aDelegate A pointer to the delegate to be used by this server.
+ * Note: the caller must ensure that the delegate lives throughout the instance's lifetime.
+ * @param aEndpointId The endpoint on which this cluster exists. This must match the zap configuration.
+ * @param aClusterId The ID of the operational state derived cluster to be instantiated.
+ */
+ Instance(Delegate * aDelegate, EndpointId aEndpointId, ClusterId aClusterId);
+
+ /**
+ * Given a state in the derived cluster number-space (from 0x40 to 0x7f), this method checks if the state is pause-compatible.
+ * Note: if a state outside the derived cluster number-space is given, this method returns false.
+ * @param aState The state to check.
+ * @return true if aState is pause-compatible, false otherwise.
+ */
+ virtual bool IsDerivedClusterStatePauseCompatible(uint8_t aState) { return false; };
+
+ /**
+ * Given a state in the derived cluster number-space (from 0x40 to 0x7f), this method checks if the state is resume-compatible.
+ * Note: if a state outside the derived cluster number-space is given, this method returns false.
+ * @param aState The state to check.
+ * @return true if aState is pause-compatible, false otherwise.
+ */
+ virtual bool IsDerivedClusterStateResumeCompatible(uint8_t aState) { return false; };
+
private:
Delegate * mDelegate;
- EndpointId mEndpointId;
- ClusterId mClusterId;
+ const EndpointId mEndpointId;
+ const ClusterId mClusterId;
// Attribute Data Store
app::DataModel::Nullable<uint8_t> mCurrentPhase;
@@ -174,6 +206,9 @@
/**
* Handle Command: Pause.
+ * If the current state is not pause-compatible, this method responds with an ErrorStateId of CommandInvalidInState.
+ * If the current state is paused, this method responds with an ErrorStateId of NoError but takes no action.
+ * Otherwise, this method calls the delegate's HandlePauseStateCallback.
*/
void HandlePauseState(HandlerContext & ctx, const Commands::Pause::DecodableType & req);
@@ -189,6 +224,8 @@
/**
* Handle Command: Resume.
+ * If the current state is not resume-compatible, this method responds with an ErrorStateId of CommandInvalidInState.
+ * Otherwise, this method calls the delegate's HandleResumeStateCallback.
*/
void HandleResumeState(HandlerContext & ctx, const Commands::Resume::DecodableType & req);
};
@@ -275,6 +312,66 @@
};
} // namespace OperationalState
+
+namespace RvcOperationalState {
+
+class Instance : public OperationalState::Instance
+{
+public:
+ /**
+ * Creates an RVC operational state cluster instance.
+ * The Init() function needs to be called for this instance to be registered and called by the
+ * interaction model at the appropriate times.
+ * It is possible to set the CurrentPhase and OperationalState via the Set... methods before calling Init().
+ * @param aDelegate A pointer to the delegate to be used by this server.
+ * Note: the caller must ensure that the delegate lives throughout the instance's lifetime.
+ * @param aEndpointId The endpoint on which this cluster exists. This must match the zap configuration.
+ */
+ Instance(OperationalState::Delegate * aDelegate, EndpointId aEndpointId) :
+ OperationalState::Instance(aDelegate, aEndpointId, Id)
+ {}
+
+protected:
+ /**
+ * Given a state in the derived cluster number-space (from 0x40 to 0x7f), this method checks if the state is pause-compatible.
+ * Note: if a state outside the derived cluster number-space is given, this method returns false.
+ * @param aState The state to check.
+ * @return true if aState is pause-compatible, false otherwise.
+ */
+ bool IsDerivedClusterStatePauseCompatible(uint8_t aState) override;
+
+ /**
+ * Given a state in the derived cluster number-space (from 0x40 to 0x7f), this method checks if the state is resume-compatible.
+ * Note: if a state outside the derived cluster number-space is given, this method returns false.
+ * @param aState The state to check.
+ * @return true if aState is pause-compatible, false otherwise.
+ */
+ bool IsDerivedClusterStateResumeCompatible(uint8_t aState) override;
+};
+
+} // namespace RvcOperationalState
+
+namespace OvenCavityOperationalState {
+
+class Instance : public OperationalState::Instance
+{
+public:
+ /**
+ * Creates an oven cavity operational state cluster instance.
+ * The Init() function needs to be called for this instance to be registered and called by the
+ * interaction model at the appropriate times.
+ * It is possible to set the CurrentPhase and OperationalState via the Set... methods before calling Init().
+ * @param aDelegate A pointer to the delegate to be used by this server.
+ * Note: the caller must ensure that the delegate lives throughout the instance's lifetime.
+ * @param aEndpointId The endpoint on which this cluster exists. This must match the zap configuration.
+ */
+ Instance(OperationalState::Delegate * aDelegate, EndpointId aEndpointId) :
+ OperationalState::Instance(aDelegate, aEndpointId, Id)
+ {}
+};
+
+} // namespace OvenCavityOperationalState
+
} // namespace Clusters
} // namespace app
} // namespace chip
