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>

examples/all-clusters-app/all-clusters-common/src/operational-state-delegate-impl.cpp

@@ -125,7 +125,7 @@ void emberAfOperationalStateClusterInitCallback(chip::EndpointId endpointId)
 
     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 @@ void emberAfRvcOperationalStateClusterInitCallback(chip::EndpointId endpointId)
 
     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));
 

examples/all-clusters-app/ameba/main/OperationalStateManager.cpp

@@ -126,7 +126,7 @@ void emberAfOperationalStateClusterInitCallback(chip::EndpointId endpointId)
 
     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 @@ void emberAfOperationalStateClusterInitCallback(chip::EndpointId endpointId)
 
 // 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 @@ void emberAfRvcOperationalStateClusterInitCallback(chip::EndpointId endpointId)
 
     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));
 

examples/all-clusters-app/ameba/main/include/ManualOperationalStateCommand.h

@@ -137,7 +137,7 @@ CHIP_ERROR ManualRVCOperationalStateSetStateCommandHandler(int argc, char ** arg
     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 @@ CHIP_ERROR ManualRVCOperationalStateSetErrorCommandHandler(int argc, char ** arg
         break;
     }
 
-    GetRVCOperationalStateInstance()->OnOperationalErrorDetected(err);
+    RvcOperationalState::GetRvcOperationalStateInstance()->OnOperationalErrorDetected(err);
 
     return CHIP_NO_ERROR;
 }

examples/all-clusters-app/ameba/main/include/OperationalStateManager.h

@@ -32,7 +32,6 @@ namespace Clusters {
 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 @@ void Shutdown();
 
 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
 {

examples/dishwasher-app/dishwasher-common/src/operational-state-delegate-impl.cpp

@@ -122,7 +122,7 @@ void emberAfOperationalStateClusterInitCallback(chip::EndpointId endpointId)
 
     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));
 

examples/rvc-app/rvc-common/include/rvc-device.h

@@ -19,7 +19,7 @@ class RvcDevice
     ModeBase::Instance mCleanModeInstance;
 
     RvcOperationalState::RvcOperationalStateDelegate mOperationalStateDelegate;
-    OperationalState::Instance mOperationalStateInstance;
+    RvcOperationalState::Instance mOperationalStateInstance;
 
     bool mDocked   = false;
     bool mCharging = false;
@@ -33,7 +33,7 @@ class RvcDevice
     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);

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.

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 @@ Instance::Instance(Delegate * aDelegate, EndpointId aEndpointId, ClusterId aClus
     mDelegate->SetInstance(this);
 }
 
+Instance::Instance(Delegate * aDelegate, EndpointId aEndpointId) : Instance(aDelegate, aEndpointId, OperationalState::Id) {}
+
 Instance::~Instance()
 {
     InteractionModelEngine::GetInstance()->UnregisterCommandHandler(this);
@@ -337,11 +339,24 @@ void Instance::HandlePauseState(HandlerContext & ctx, const Commands::Pause::Dec
     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 @@ void Instance::HandleResumeState(HandlerContext & ctx, const Commands::Resume::D
     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 @@ void Instance::HandleResumeState(HandlerContext & ctx, const Commands::Resume::D
 
     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));
+}

src/app/clusters/operational-state-server/operational-state-server.h

@@ -29,6 +29,9 @@ namespace app {
 namespace Clusters {
 namespace OperationalState {
 
+const uint8_t DerivedClusterNumberSpaceStart = 0x40;
+const uint8_t VendorNumberSpaceStart         = 0x80;
+
 class Uncopyable
 {
 protected:
@@ -50,15 +53,15 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface
 {
 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 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface
      */
     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 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface
 
     /**
      * 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 @@ class Instance : public CommandHandlerInterface, public AttributeAccessInterface
 
     /**
      * 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 @@ class Delegate
 };
 
 } // 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