[OPSTATE] Fix: change in type for OperationalState attribute (project…

…-chip#27958) * OperationalState attribute simply to Enum * temporary remove some test step in TestOperationalState * zap regen all * modify the code related of the OperationalState attribute type * update OperationalState type to Enum in TestOperationalState.yaml * Restyled by clang-format * modify the api of GetCurrentOperationalState in class Delegate * modify the api of SetOperationalState in class Delegate * optimize the Operational State cluster definition * zap regen all * Restyled by clang-format * modify the note for RVC Operational State cluster definition --------- Co-authored-by: Restyled.io <[email protected]>
Rightpoint · Jul 17, 2023 · 6441b46 · 6441b46
1 parent a3d5a9d
commit 6441b46
Show file tree

Hide file tree

Showing 19 changed files with 147 additions and 160 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -2692,7 +2692,7 @@ server cluster OperationalState = 96 {
   readonly attribute nullable int8u currentPhase = 1;
   readonly attribute nullable elapsed_s countdownTime = 2;
   readonly attribute OperationalStateStruct operationalStateList[] = 3;
-  readonly attribute OperationalStateStruct operationalState = 4;
+  readonly attribute OperationalStateEnum operationalState = 4;
   readonly attribute ErrorStateStruct operationalError = 5;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;

diff --git a/examples/all-clusters-app/all-clusters-common/include/operational-state-delegate-impl.h b/examples/all-clusters-app/all-clusters-common/include/operational-state-delegate-impl.h
@@ -34,11 +34,10 @@ class OperationalStateDelegate : public Delegate
 
 public:
     /**
-     * Get current operational state.
-     * @param op The GenericOperationalState to fill with the current operational state value.
-     * @return void.
+     * Get the current operational state.
+     * @return The current operational state value
      */
-    void GetCurrentOperationalState(GenericOperationalState & op) override;
+    uint8_t GetCurrentOperationalState() override;
 
     /**
      * Get the list of supported operational states.
@@ -86,7 +85,7 @@ class OperationalStateDelegate : public Delegate
      * Set current operational state.
      * @param opState The operational state that should now be the current one.
      */
-    CHIP_ERROR SetOperationalState(const GenericOperationalState & opState) override;
+    CHIP_ERROR SetOperationalState(uint8_t opState) override;
 
     /**
      * Set operational phase.
@@ -125,7 +124,7 @@ class OperationalStateDelegate : public Delegate
      */
     void HandleStopStateCallback(GenericOperationalError & err) override;
 
-    OperationalStateDelegate(GenericOperationalState aOperationalState, GenericOperationalError aOperationalError,
+    OperationalStateDelegate(uint8_t aOperationalState, GenericOperationalError aOperationalError,
                              Span<const GenericOperationalState> aOperationalStateList,
                              Span<const GenericOperationalPhase> aOperationalPhaseList,
                              app::DataModel::Nullable<uint8_t> aPhase          = DataModel::Nullable<uint8_t>(),
@@ -137,7 +136,7 @@ class OperationalStateDelegate : public Delegate
     ~OperationalStateDelegate() = default;
 
 private:
-    GenericOperationalState mOperationalState;
+    uint8_t mOperationalState;
     GenericOperationalError mOperationalError;
     app::DataModel::List<const GenericOperationalState> mOperationalStateList;
     Span<const GenericOperationalPhase> mOperationalPhaseList;

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
@@ -24,7 +24,7 @@ namespace OperationalState {
 
 using chip::Protocols::InteractionModel::Status;
 
-CHIP_ERROR OperationalStateDelegate::SetOperationalState(const GenericOperationalState & opState)
+CHIP_ERROR OperationalStateDelegate::SetOperationalState(uint8_t opState)
 {
     mOperationalState = opState;
     return CHIP_NO_ERROR;
@@ -42,9 +42,9 @@ CHIP_ERROR OperationalStateDelegate::SetCountdownTime(const app::DataModel::Null
     return CHIP_NO_ERROR;
 }
 
-void OperationalStateDelegate::GetCurrentOperationalState(GenericOperationalState & op)
+uint8_t OperationalStateDelegate::GetCurrentOperationalState()
 {
-    op = mOperationalState;
+    return mOperationalState;
 }
 
 CHIP_ERROR OperationalStateDelegate::SetOperationalError(const GenericOperationalError & opErrState)
@@ -91,28 +91,28 @@ CHIP_ERROR OperationalStateDelegate::GetOperationalPhaseAtIndex(size_t index, Ge
 void OperationalStateDelegate::HandlePauseStateCallback(GenericOperationalError & err)
 {
     // placeholder implementation
-    mOperationalState.Set(to_underlying(OperationalStateEnum::kPaused));
+    mOperationalState = to_underlying(OperationalStateEnum::kPaused);
     err.Set(to_underlying(ErrorStateEnum::kNoError));
 }
 
 void OperationalStateDelegate::HandleResumeStateCallback(GenericOperationalError & err)
 {
     // placeholder implementation
-    mOperationalState.Set(to_underlying(OperationalStateEnum::kRunning));
+    mOperationalState = to_underlying(OperationalStateEnum::kRunning);
     err.Set(to_underlying(ErrorStateEnum::kNoError));
 }
 
 void OperationalStateDelegate::HandleStartStateCallback(GenericOperationalError & err)
 {
     // placeholder implementation
-    mOperationalState.Set(to_underlying(OperationalStateEnum::kRunning));
+    mOperationalState = to_underlying(OperationalStateEnum::kRunning);
     err.Set(to_underlying(ErrorStateEnum::kNoError));
 }
 
 void OperationalStateDelegate::HandleStopStateCallback(GenericOperationalError & err)
 {
     // placeholder implementation
-    mOperationalState.Set(to_underlying(OperationalStateEnum::kStopped));
+    mOperationalState = to_underlying(OperationalStateEnum::kStopped);
     err.Set(to_underlying(ErrorStateEnum::kNoError));
 }
 

diff --git a/examples/all-clusters-app/all-clusters-common/src/operational-state-delegates.cpp b/examples/all-clusters-app/all-clusters-common/src/operational-state-delegates.cpp
@@ -77,7 +77,7 @@ static const GenericOperationalPhase opPhaseList[] = {
  * Operational State Delegate
  * Note: User Define
  */
-static OperationalStateDelegate opStateDelegate(GenericOperationalState(to_underlying(OperationalStateEnum::kStopped)),
+static OperationalStateDelegate opStateDelegate(to_underlying(OperationalStateEnum::kStopped),
                                                 GenericOperationalError(to_underlying(ErrorStateEnum::kNoError)),
                                                 Span<const GenericOperationalState>(opStateList),
                                                 Span<const GenericOperationalPhase>(opPhaseList));

diff --git a/src/app/clusters/operational-state-server/operational-state-delegate.h b/src/app/clusters/operational-state-server/operational-state-delegate.h
@@ -213,10 +213,9 @@ class Delegate
 public:
     /**
      * Get the current operational state.
-     * @param op The GenericOperationalState to fill with the current operational state value.
-     * @return void.
+     * @return The current operational state value
      */
-    virtual void GetCurrentOperationalState(GenericOperationalState & op) = 0;
+    virtual uint8_t GetCurrentOperationalState() = 0;
 
     /**
      * Get the list of supported operational states.
@@ -258,7 +257,7 @@ class Delegate
      * Set current operational state.
      * @param opState The operational state that should now be the current one.
      */
-    virtual CHIP_ERROR SetOperationalState(const GenericOperationalState & opState) = 0;
+    virtual CHIP_ERROR SetOperationalState(uint8_t opState) = 0;
 
     /**
      * Set operational error.

diff --git a/src/app/clusters/operational-state-server/operational-state-server.cpp b/src/app/clusters/operational-state-server/operational-state-server.cpp
@@ -106,12 +106,11 @@ void OperationalStateServer::HandlePauseState(HandlerContext & ctx, const Comman
     Commands::OperationalCommandResponse::Type response;
     Delegate * delegate = OperationalState::GetOperationalStateDelegate(mEndpointId, mClusterId);
     GenericOperationalError err(to_underlying(ErrorStateEnum::kNoError));
-    GenericOperationalState opState;
 
     VerifyOrReturn(delegate != nullptr, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure));
-    delegate->GetCurrentOperationalState(opState);
+    uint8_t opState = delegate->GetCurrentOperationalState();
 
-    if (opState.operationalStateID != to_underlying(OperationalStateEnum::kPaused))
+    if (opState != to_underlying(OperationalStateEnum::kPaused))
     {
         delegate->HandlePauseStateCallback(err);
     }
@@ -126,18 +125,15 @@ void OperationalStateServer::HandleResumeState(HandlerContext & ctx, const Comma
     Commands::OperationalCommandResponse::Type response;
     Delegate * delegate = OperationalState::GetOperationalStateDelegate(mEndpointId, mClusterId);
     GenericOperationalError err(to_underlying(ErrorStateEnum::kNoError));
-    GenericOperationalState opState;
 
     VerifyOrReturn(delegate != nullptr, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure));
+    uint8_t opState = delegate->GetCurrentOperationalState();
 
-    delegate->GetCurrentOperationalState(opState);
-
-    if (opState.operationalStateID != to_underlying(OperationalStateEnum::kPaused) &&
-        opState.operationalStateID != to_underlying(OperationalStateEnum::kRunning))
+    if (opState != to_underlying(OperationalStateEnum::kPaused) && opState != to_underlying(OperationalStateEnum::kRunning))
     {
         err.Set(to_underlying(ErrorStateEnum::kCommandInvalidInState));
     }
-    else if (opState.operationalStateID == to_underlying(OperationalStateEnum::kPaused))
+    else if (opState == to_underlying(OperationalStateEnum::kPaused))
     {
         delegate->HandleResumeStateCallback(err);
     }
@@ -152,13 +148,11 @@ void OperationalStateServer::HandleStartState(HandlerContext & ctx, const Comman
     Commands::OperationalCommandResponse::Type response;
     Delegate * delegate = OperationalState::GetOperationalStateDelegate(mEndpointId, mClusterId);
     GenericOperationalError err(to_underlying(ErrorStateEnum::kNoError));
-    GenericOperationalState opState;
 
     VerifyOrReturn(delegate != nullptr, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure));
+    uint8_t opState = delegate->GetCurrentOperationalState();
 
-    delegate->GetCurrentOperationalState(opState);
-
-    if (opState.operationalStateID != to_underlying(OperationalStateEnum::kRunning))
+    if (opState != to_underlying(OperationalStateEnum::kRunning))
     {
         delegate->HandleStartStateCallback(err);
     }
@@ -173,13 +167,11 @@ void OperationalStateServer::HandleStopState(HandlerContext & ctx, const Command
     Commands::OperationalCommandResponse::Type response;
     Delegate * delegate = OperationalState::GetOperationalStateDelegate(mEndpointId, mClusterId);
     GenericOperationalError err(to_underlying(ErrorStateEnum::kNoError));
-    GenericOperationalState opState;
 
     VerifyOrReturn(delegate != nullptr, ctx.mCommandHandler.AddStatus(ctx.mRequestPath, Status::Failure));
+    uint8_t opState = delegate->GetCurrentOperationalState();
 
-    delegate->GetCurrentOperationalState(opState);
-
-    if (opState.operationalStateID != to_underlying(OperationalStateEnum::kStopped))
+    if (opState != to_underlying(OperationalStateEnum::kStopped))
     {
         delegate->HandleStopStateCallback(err);
     }
@@ -243,9 +235,8 @@ CHIP_ERROR OperationalStateServer::Read(const ConcreteReadAttributePath & aPath,
     case OperationalState::Attributes::OperationalState::Id: {
 
         Delegate * delegate = OperationalState::GetOperationalStateDelegate(mEndpointId, mClusterId);
-        GenericOperationalState opState;
         VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INCORRECT_STATE, ChipLogError(Zcl, "Delegate is nullptr"));
-        delegate->GetCurrentOperationalState(opState);
+        uint8_t opState = delegate->GetCurrentOperationalState();
         return aEncoder.Encode(opState);
     }
     break;

diff --git a/src/app/tests/suites/TestOperationalState.yaml b/src/app/tests/suites/TestOperationalState.yaml
@@ -68,7 +68,7 @@ tests:
       command: "readAttribute"
       attribute: "OperationalState"
       response:
-          value: { OperationalStateID: 0 }
+          value: 0
 
     - label: "Start Command"
       command: "Start"
@@ -81,7 +81,7 @@ tests:
       command: "readAttribute"
       attribute: "OperationalState"
       response:
-          value: { OperationalStateID: 1 }
+          value: 1
 
     - label: "Pause Command"
       command: "Pause"
@@ -94,7 +94,7 @@ tests:
       command: "readAttribute"
       attribute: "OperationalState"
       response:
-          value: { OperationalStateID: 2 }
+          value: 2
 
     - label: "Resume Command"
       command: "Resume"
@@ -107,7 +107,7 @@ tests:
       command: "readAttribute"
       attribute: "OperationalState"
       response:
-          value: { OperationalStateID: 1 }
+          value: 1
 
     - label: "Stop Command"
       command: "Stop"
@@ -120,4 +120,4 @@ tests:
       command: "readAttribute"
       attribute: "OperationalState"
       response:
-          value: { OperationalStateID: 0 }
+          value: 0
diff --git a/src/app/zap-templates/zcl/data-model/chip/operational-state-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/operational-state-cluster.xml
@@ -63,7 +63,7 @@ limitations under the License.
     <attribute side="server" code="0x0001" define="CURRENT_PHASE"           type="INT8U"                                    min="0x00" max="0x1F"    writable="false" isNullable="true"  optional="false">CurrentPhase</attribute>
     <attribute side="server" code="0x0002" define="COUNTDOWN_TIME"          type="elapsed_s"                                min="0x00" max="259200"  writable="false" isNullable="true"  optional="true">CountdownTime</attribute>
     <attribute side="server" code="0x0003" define="OPERATIONAL_STATE_LIST"  type="ARRAY" entryType="OperationalStateStruct"                          writable="false"                    optional="false">OperationalStateList</attribute>
-    <attribute side="server" code="0x0004" define="OPERATIONAL_STATE"       type="OperationalStateStruct"                                            writable="false"                    optional="false">OperationalState</attribute>
+    <attribute side="server" code="0x0004" define="OPERATIONAL_STATE"       type="OperationalStateEnum"                                              writable="false"                    optional="false">OperationalState</attribute>
     <attribute side="server" code="0x0005" define="OPERATIONAL_ERROR"       type="ErrorStateStruct"                                                  writable="false"                    optional="false">OperationalError</attribute>
 
     <command source="client" code="0x00" name="Pause" response="OperationalCommandResponse" optional="true">

diff --git a/src/app/zap-templates/zcl/data-model/chip/operational-state-rvc-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/operational-state-rvc-cluster.xml
@@ -51,7 +51,13 @@ limitations under the License.
     <attribute side="server" code="0x0001" define="CURRENT_PHASE"           type="INT8U"                                    min="0x00" max="0x1F"    writable="false" isNullable="true"  optional="false">CurrentPhase</attribute>
     <attribute side="server" code="0x0002" define="COUNTDOWN_TIME"          type="elapsed_s"                                min="0"    max="259200"  writable="false" isNullable="true"  optional="true">CountdownTime</attribute>
     <attribute side="server" code="0x0003" define="OPERATIONAL_STATE_LIST"  type="ARRAY" entryType="OperationalStateStruct"                          writable="false"                    optional="false">OperationalStateList</attribute>
-    <attribute side="server" code="0x0004" define="OPERATIONAL_STATE"       type="OperationalStateStruct"                                            writable="false"                    optional="false">OperationalState</attribute>
+
+<!--
+The type of this attribute in the spec is OperationalStateEnum, but the OperationalStateEnum in our XML
+excludes the values from the base OperationalState cluster.  Just use ENUM8 for the type, so it allows
+both values from this cluster and from the base cluster.
+-->
+    <attribute side="server" code="0x0004" define="OPERATIONAL_STATE"       type="ENUM8"                                                             writable="false"                    optional="false">OperationalState</attribute>
     <attribute side="server" code="0x0005" define="OPERATIONAL_ERROR"       type="ErrorStateStruct"                                                  writable="false"                    optional="false">OperationalError</attribute>
 
     <command source="client" code="0x00" name="Pause" response="OperationalCommandResponse" optional="true">

diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -3354,7 +3354,7 @@ client cluster OperationalState = 96 {
   readonly attribute nullable int8u currentPhase = 1;
   readonly attribute optional nullable elapsed_s countdownTime = 2;
   readonly attribute OperationalStateStruct operationalStateList[] = 3;
-  readonly attribute OperationalStateStruct operationalState = 4;
+  readonly attribute OperationalStateEnum operationalState = 4;
   readonly attribute ErrorStateStruct operationalError = 5;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;
@@ -3421,7 +3421,7 @@ client cluster RvcOperationalState = 97 {
   readonly attribute nullable int8u currentPhase = 1;
   readonly attribute optional nullable elapsed_s countdownTime = 2;
   readonly attribute OperationalStateStruct operationalStateList[] = 3;
-  readonly attribute OperationalStateStruct operationalState = 4;
+  readonly attribute enum8 operationalState = 4;
   readonly attribute ErrorStateStruct operationalError = 5;
   readonly attribute command_id generatedCommandList[] = 65528;
   readonly attribute command_id acceptedCommandList[] = 65529;

diff --git a/src/controller/java/generated/java/chip/devicecontroller/ClusterReadMapping.java b/src/controller/java/generated/java/chip/devicecontroller/ClusterReadMapping.java
@@ -7432,6 +7432,17 @@ private static Map<String, InteractionInfo> readOperationalStateInteractionInfo(
           readOperationalStateOperationalStateListCommandParams
         );
         result.put("readOperationalStateListAttribute", readOperationalStateOperationalStateListAttributeInteractionInfo);
+     Map<String, CommandParameterInfo> readOperationalStateOperationalStateCommandParams = new LinkedHashMap<String, CommandParameterInfo>();
+        InteractionInfo readOperationalStateOperationalStateAttributeInteractionInfo = new InteractionInfo(
+          (cluster, callback, commandArguments) -> {
+            ((ChipClusters.OperationalStateCluster) cluster).readOperationalStateAttribute(
+              (ChipClusters.IntegerAttributeCallback) callback
+            );
+          },
+          () -> new ClusterInfoMapping.DelegatedIntegerAttributeCallback(),
+          readOperationalStateOperationalStateCommandParams
+        );
+        result.put("readOperationalStateAttribute", readOperationalStateOperationalStateAttributeInteractionInfo);
      Map<String, CommandParameterInfo> readOperationalStateGeneratedCommandListCommandParams = new LinkedHashMap<String, CommandParameterInfo>();
         InteractionInfo readOperationalStateGeneratedCommandListAttributeInteractionInfo = new InteractionInfo(
           (cluster, callback, commandArguments) -> {
@@ -7546,6 +7557,17 @@ private static Map<String, InteractionInfo> readRvcOperationalStateInteractionIn
           readRvcOperationalStateOperationalStateListCommandParams
         );
         result.put("readOperationalStateListAttribute", readRvcOperationalStateOperationalStateListAttributeInteractionInfo);
+     Map<String, CommandParameterInfo> readRvcOperationalStateOperationalStateCommandParams = new LinkedHashMap<String, CommandParameterInfo>();
+        InteractionInfo readRvcOperationalStateOperationalStateAttributeInteractionInfo = new InteractionInfo(
+          (cluster, callback, commandArguments) -> {
+            ((ChipClusters.RvcOperationalStateCluster) cluster).readOperationalStateAttribute(
+              (ChipClusters.IntegerAttributeCallback) callback
+            );
+          },
+          () -> new ClusterInfoMapping.DelegatedIntegerAttributeCallback(),
+          readRvcOperationalStateOperationalStateCommandParams
+        );
+        result.put("readOperationalStateAttribute", readRvcOperationalStateOperationalStateAttributeInteractionInfo);
      Map<String, CommandParameterInfo> readRvcOperationalStateGeneratedCommandListCommandParams = new LinkedHashMap<String, CommandParameterInfo>();
         InteractionInfo readRvcOperationalStateGeneratedCommandListAttributeInteractionInfo = new InteractionInfo(
           (cluster, callback, commandArguments) -> {