Skip to content

Commit

Permalink
[OPSTATE] Fix: change in type for OperationalState attribute (#27958)
Browse files Browse the repository at this point in the history 
* 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]>
  • Loading branch information
@mideayanghui @restyled-commits @pull
2 people authored and pull[bot] committed Sep 13, 2023
1 parent 8a7c07d commit de3215d
Show file tree
Hide file tree
Showing 19 changed files with 147 additions and 160 deletions.
2 changes: 1 addition & 1 deletion examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 6 additions & 7 deletions examples/all-clusters-app/all-clusters-common/include/operational-state-delegate-impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>(),
Expand All @@ -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;
Expand Down
14 changes: 7 additions & 7 deletions examples/all-clusters-app/all-clusters-common/src/operational-state-delegate-impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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));
}

Expand Down
2 changes: 1 addition & 1 deletion examples/all-clusters-app/all-clusters-common/src/operational-state-delegates.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
7 changes: 3 additions & 4 deletions src/app/clusters/operational-state-server/operational-state-delegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 10 additions & 19 deletions src/app/clusters/operational-state-server/operational-state-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions src/app/tests/suites/TestOperationalState.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ tests:
command: "readAttribute"
attribute: "OperationalState"
response:
value: { OperationalStateID: 0 }
value: 0

- label: "Start Command"
command: "Start"
Expand All @@ -81,7 +81,7 @@ tests:
command: "readAttribute"
attribute: "OperationalState"
response:
value: { OperationalStateID: 1 }
value: 1

- label: "Pause Command"
command: "Pause"
Expand All @@ -94,7 +94,7 @@ tests:
command: "readAttribute"
attribute: "OperationalState"
response:
value: { OperationalStateID: 2 }
value: 2

- label: "Resume Command"
command: "Resume"
Expand All @@ -107,7 +107,7 @@ tests:
command: "readAttribute"
attribute: "OperationalState"
response:
value: { OperationalStateID: 1 }
value: 1

- label: "Stop Command"
command: "Stop"
Expand All @@ -120,4 +120,4 @@ tests:
command: "readAttribute"
attribute: "OperationalState"
response:
value: { OperationalStateID: 0 }
value: 0
2 changes: 1 addition & 1 deletion src/app/zap-templates/zcl/data-model/chip/operational-state-cluster.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
8 changes: 7 additions & 1 deletion src/app/zap-templates/zcl/data-model/chip/operational-state-rvc-cluster.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
4 changes: 2 additions & 2 deletions src/controller/data_model/controller-clusters.matter
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 22 additions & 0 deletions src/controller/java/generated/java/chip/devicecontroller/ClusterReadMapping.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> {
Expand Down Expand Up @@ -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) -> {
Expand Down
Loading

0 comments on commit de3215d

Please sign in to comment.