Skip to content

Commit

Permalink
Fix Smoke CO Alarm Cluster SDK (#28961)
Browse files Browse the repository at this point in the history 
* Remove redundant pics values

* Avoid unsupported attributes affecting other

---------

Co-authored-by: Hare <[email protected]>
  • Loading branch information
@ericzijian1994 @hare-siterwell
ericzijian1994 and hare-siterwell authored Sep 11, 2023
1 parent e29cd6a commit 458ffe2
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 75 deletions.
60 changes: 34 additions & 26 deletions src/app/clusters/smoke-co-alarm-server/smoke-co-alarm-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,43 +46,44 @@ SmokeCoAlarmServer & SmokeCoAlarmServer::Instance()
void SmokeCoAlarmServer::SetExpressedStateByPriority(EndpointId endpointId,
const std::array<ExpressedStateEnum, kPriorityOrderLength> & priorityOrder)
{
AlarmStateEnum alarmState = AlarmStateEnum::kNormal;
EndOfServiceEnum endOfServiceState = EndOfServiceEnum::kNormal;
bool active = false;

for (ExpressedStateEnum priority : priorityOrder)
{
AlarmStateEnum alarmState = AlarmStateEnum::kNormal;
EndOfServiceEnum endOfServiceState = EndOfServiceEnum::kNormal;
bool active = false;
bool success = false;

switch (priority)
{
case ExpressedStateEnum::kSmokeAlarm:
VerifyOrReturn(GetSmokeState(endpointId, alarmState));
success = GetSmokeState(endpointId, alarmState);
break;
case ExpressedStateEnum::kCOAlarm:
VerifyOrReturn(GetCOState(endpointId, alarmState));
success = GetCOState(endpointId, alarmState);
break;
case ExpressedStateEnum::kBatteryAlert:
VerifyOrReturn(GetBatteryAlert(endpointId, alarmState));
success = GetBatteryAlert(endpointId, alarmState);
break;
case ExpressedStateEnum::kTesting:
VerifyOrReturn(GetTestInProgress(endpointId, active));
success = GetTestInProgress(endpointId, active);
break;
case ExpressedStateEnum::kHardwareFault:
VerifyOrReturn(GetHardwareFaultAlert(endpointId, active));
success = GetHardwareFaultAlert(endpointId, active);
break;
case ExpressedStateEnum::kEndOfService:
VerifyOrReturn(GetEndOfServiceAlert(endpointId, endOfServiceState));
success = GetEndOfServiceAlert(endpointId, endOfServiceState);
break;
case ExpressedStateEnum::kInterconnectSmoke:
VerifyOrReturn(GetInterconnectSmokeAlarm(endpointId, alarmState));
success = GetInterconnectSmokeAlarm(endpointId, alarmState);
break;
case ExpressedStateEnum::kInterconnectCO:
VerifyOrReturn(GetInterconnectCOAlarm(endpointId, alarmState));
success = GetInterconnectCOAlarm(endpointId, alarmState);
break;
default:
break;
}

if ((alarmState != AlarmStateEnum::kNormal) || (endOfServiceState != EndOfServiceEnum::kNormal) || active)
if (success && ((alarmState != AlarmStateEnum::kNormal) || (endOfServiceState != EndOfServiceEnum::kNormal) || active))
{
SetExpressedState(endpointId, priority);
return;
Expand Down Expand Up @@ -186,20 +187,22 @@ bool SmokeCoAlarmServer::SetDeviceMuted(EndpointId endpointId, MuteStateEnum new
{
AlarmStateEnum alarmState;

VerifyOrReturnValue(GetAttribute(endpointId, SmokeState::Id, SmokeState::Get, alarmState), false);
VerifyOrReturnValue(alarmState != AlarmStateEnum::kCritical, false);
// If the attribute has been read and the attribute is Critical, return false

VerifyOrReturnValue(GetAttribute(endpointId, COState::Id, COState::Get, alarmState), false);
VerifyOrReturnValue(alarmState != AlarmStateEnum::kCritical, false);
bool success = GetSmokeState(endpointId, alarmState);
VerifyOrReturnValue(!success || alarmState != AlarmStateEnum::kCritical, false);

VerifyOrReturnValue(GetAttribute(endpointId, BatteryAlert::Id, BatteryAlert::Get, alarmState), false);
VerifyOrReturnValue(alarmState != AlarmStateEnum::kCritical, false);
success = GetCOState(endpointId, alarmState);
VerifyOrReturnValue(!success || alarmState != AlarmStateEnum::kCritical, false);

VerifyOrReturnValue(GetAttribute(endpointId, InterconnectSmokeAlarm::Id, InterconnectSmokeAlarm::Get, alarmState), false);
VerifyOrReturnValue(alarmState != AlarmStateEnum::kCritical, false);
success = GetBatteryAlert(endpointId, alarmState);
VerifyOrReturnValue(!success || alarmState != AlarmStateEnum::kCritical, false);

VerifyOrReturnValue(GetAttribute(endpointId, InterconnectCOAlarm::Id, InterconnectCOAlarm::Get, alarmState), false);
VerifyOrReturnValue(alarmState != AlarmStateEnum::kCritical, false);
success = GetInterconnectSmokeAlarm(endpointId, alarmState);
VerifyOrReturnValue(!success || alarmState != AlarmStateEnum::kCritical, false);

success = GetInterconnectCOAlarm(endpointId, alarmState);
VerifyOrReturnValue(!success || alarmState != AlarmStateEnum::kCritical, false);
}

VerifyOrReturnValue(SetAttribute(endpointId, DeviceMuted::Id, DeviceMuted::Set, newDeviceMuted), false);
Expand Down Expand Up @@ -466,10 +469,15 @@ template <typename T>
bool SmokeCoAlarmServer::GetAttribute(EndpointId endpointId, AttributeId attributeId,
EmberAfStatus (*getFn)(EndpointId endpointId, T * value), T & value) const
{
EmberAfStatus status = getFn(endpointId, &value);
bool success = (EMBER_ZCL_STATUS_SUCCESS == status);
EmberAfStatus status = getFn(endpointId, &value);
bool success = (EMBER_ZCL_STATUS_SUCCESS == status);
bool unsupportedStatus = (EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE == status);

if (!success)
if (unsupportedStatus)
{
ChipLogProgress(Zcl, "Read unsupported SmokeCOAlarm attribute: attribute=" ChipLogFormatMEI, ChipLogValueMEI(attributeId));
}
else if (!success)
{
ChipLogError(Zcl, "Failed to read SmokeCOAlarm attribute: attribute=" ChipLogFormatMEI ", status=0x%x",
ChipLogValueMEI(attributeId), to_underlying(status));
Expand Down
11 changes: 1 addition & 10 deletions src/app/tests/suites/certification/PICS.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8901,15 +8901,6 @@ PICS:
- label: "Does the device implement the InterconnectCOAlarm attribute?"
id: SMOKECO.S.A0009

- label: "Does the device implement the ContaminationState attribute?"
id: SMOKECO.S.A0010

- label: "Does the device implement the SensitivityLevel attribute?"
id: SMOKECO.S.A0011

- label: "Does the device implement the ExpiryDate attribute?"
id: SMOKECO.S.A0012

- label: "Does the device implement the ContaminationState attribute?"
id: SMOKECO.S.A000a

Expand Down Expand Up @@ -8953,7 +8944,7 @@ PICS:
id: SMOKECO.S.E09

- label: "Does the device implement the AllClear event?"
id: SMOKECO.S.E10
id: SMOKECO.S.E0a

#
# server / manually
Expand Down
41 changes: 2 additions & 39 deletions src/app/tests/suites/certification/ci-pics-values
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,40 +700,6 @@ S.C.AM-WRITE=0
S.C.AO-READ=0
S.C.AO-WRITE=0

# Smoke CO Alarm Cluster
SMOKECO.S=1
SMOKECO.S.F00=1
SMOKECO.S.F01=1
SMOKECO.S.A0000=1
SMOKECO.S.A0001=1
SMOKECO.S.A0002=1
SMOKECO.S.A0003=1
SMOKECO.S.A0004=1
SMOKECO.S.A0005=1
SMOKECO.S.A0006=1
SMOKECO.S.A0007=1
SMOKECO.S.A0008=1
SMOKECO.S.A0009=1
SMOKECO.S.A000a=1
SMOKECO.S.A000b=1
SMOKECO.S.A000c=1
SMOKECO.S.E00=1
SMOKECO.S.E01=1
SMOKECO.S.E02=1
SMOKECO.S.E03=1
SMOKECO.S.E04=1
SMOKECO.S.E05=1
SMOKECO.S.E06=1
SMOKECO.S.E07=1
SMOKECO.S.E08=1
SMOKECO.S.E09=1
SMOKECO.S.E0a=1
SMOKECO.M.ManuallyControlledTest=1
SMOKECO.M.ManuallyControlledMute=1
SMOKECO.S.C00.Rsp=1

SMOKECO.C=1

# Switch Cluster
SWTCH.S=1
SWTCH.S.F00=1
Expand Down Expand Up @@ -2607,6 +2573,7 @@ ICDM.S.C01.Tx=1

# Smoke CO Alarm Cluster
SMOKECO.S=1
SMOKECO.C=1
SMOKECO.S.F00=1
SMOKECO.S.F01=1
SMOKECO.S.A0000=1
Expand All @@ -2619,9 +2586,6 @@ SMOKECO.S.A0006=1
SMOKECO.S.A0007=1
SMOKECO.S.A0008=1
SMOKECO.S.A0009=1
SMOKECO.S.A0010=1
SMOKECO.S.A0011=1
SMOKECO.S.A0012=1
SMOKECO.S.A000a=1
SMOKECO.S.A000b=1
SMOKECO.S.A000c=1
Expand All @@ -2635,11 +2599,10 @@ SMOKECO.S.E06=1
SMOKECO.S.E07=1
SMOKECO.S.E08=1
SMOKECO.S.E09=1
SMOKECO.S.E10=1
SMOKECO.S.E0a=1
SMOKECO.M.ManuallyControlledTest=1
SMOKECO.M.ManuallyControlledMute=1
SMOKECO.S.C00.Rsp=1
SMOKECO.C=1

#Temperature Controlled Cabinet Mode Cluster
TCCM.S=1
Expand Down

0 comments on commit 458ffe2

Please sign in to comment.