Skip to content

Commit bfe646f

Browse files
Address some minor comments from code review.
1 parent 6113905 commit bfe646f

File tree

2 files changed

+19
-27
lines changed

2 files changed

+19
-27
lines changed

src/app/clusters/actions-server/actions-server.cpp

+16-25
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,16 @@ Delegate * GetDelegate(EndpointId aEndpoint)
4545
return (aEndpoint >= kActionsDelegateTableSize ? nullptr : gDelegateTable[aEndpoint]);
4646
}
4747

48+
CHIP_ERROR IsValid(Delegate * delegate)
49+
{
50+
if (delegate == nullptr)
51+
{
52+
ChipLogError(Zcl, "Actions delegate is null!!!");
53+
return CHIP_ERROR_INCORRECT_STATE;
54+
}
55+
return CHIP_NO_ERROR;
56+
}
57+
4858
} // namespace
4959

5060
ActionsServer ActionsServer::sInstance;
@@ -117,11 +127,7 @@ CHIP_ERROR ActionsServer::ReadActionListAttribute(const ConcreteReadAttributePat
117127
const AttributeValueEncoder::ListEncodeHelper & aEncoder)
118128
{
119129
Delegate * delegate = GetDelegate(aPath.mEndpointId);
120-
if (delegate == nullptr)
121-
{
122-
ChipLogError(Zcl, "Actions delegate is null!!!");
123-
return CHIP_ERROR_INCORRECT_STATE;
124-
}
130+
VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
125131

126132
for (uint16_t i = 0; i < kMaxActionListLength; i++)
127133
{
@@ -142,11 +148,8 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP
142148
const AttributeValueEncoder::ListEncodeHelper & aEncoder)
143149
{
144150
Delegate * delegate = GetDelegate(aPath.mEndpointId);
145-
if (delegate == nullptr)
146-
{
147-
ChipLogError(Zcl, "Actions delegate is null!!!");
148-
return CHIP_ERROR_INCORRECT_STATE;
149-
}
151+
VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
152+
150153
for (uint16_t i = 0; i < kMaxEndpointListLength; i++)
151154
{
152155
EndpointListStorage epList;
@@ -165,11 +168,7 @@ CHIP_ERROR ActionsServer::ReadEndpointListAttribute(const ConcreteReadAttributeP
165168
bool ActionsServer::HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId)
166169
{
167170
Delegate * delegate = GetDelegate(aEndpointId);
168-
if (delegate == nullptr)
169-
{
170-
ChipLogError(Zcl, "Actions delegate is null!!!");
171-
return false;
172-
}
171+
VerifyOrReturnValue(IsValid(delegate) == CHIP_NO_ERROR, false);
173172
return delegate->HaveActionWithId(aActionId);
174173
}
175174

@@ -405,11 +404,7 @@ void ActionsServer::InvokeCommand(HandlerContext & handlerContext)
405404
CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStructStorage & aAction)
406405
{
407406
Delegate * delegate = GetDelegate(aEndpoint);
408-
if (delegate == nullptr)
409-
{
410-
ChipLogError(Zcl, "Actions delegate is null!");
411-
return CHIP_ERROR_INCORRECT_STATE;
412-
}
407+
VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
413408

414409
// Read through the list to find and update the existing action that matches the passed-in action's ID.
415410
for (uint16_t i = 0; i < kMaxActionListLength; i++)
@@ -437,11 +432,7 @@ CHIP_ERROR ActionsServer::ModifyActionList(EndpointId aEndpoint, const ActionStr
437432
CHIP_ERROR ActionsServer::ModifyEndpointList(EndpointId aEndpoint, const EndpointListStorage & aEpList)
438433
{
439434
Delegate * delegate = GetDelegate(aEndpoint);
440-
if (delegate == nullptr)
441-
{
442-
ChipLogError(Zcl, "Actions delegate is null!");
443-
return CHIP_ERROR_INCORRECT_STATE;
444-
}
435+
VerifyOrReturnError(IsValid(delegate) == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
445436

446437
// Read through the list to find and update the existing action that matches the passed-in endpoint-list's ID
447438
for (uint16_t i = 0; i < kMaxEndpointListLength; i++)

src/app/clusters/actions-server/actions-server.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@ struct EndpointListStorage : public Structs::EndpointListStruct::Type
9999
{
100100
mEpList[index] = aEndpointList[index];
101101
}
102-
endpoints = DataModel::List<const EndpointId>(Span(mEpList, epListSize));
103-
MutableCharSpan mEpListName = Span(mBuffer, sizeof(mBuffer));
102+
endpoints = DataModel::List<const EndpointId>(Span(mEpList, epListSize));
103+
MutableCharSpan mEpListName(mBuffer);
104104
CopyCharSpanToMutableCharSpanWithTruncation(aEpListName, mEpListName);
105105
name = mEpListName;
106106
}
@@ -172,6 +172,7 @@ class ActionsServer : public AttributeAccessInterface, public CommandHandlerInte
172172
const AttributeValueEncoder::ListEncodeHelper & aEncoder);
173173
bool HaveActionWithId(EndpointId aEndpointId, uint16_t aActionId);
174174

175+
// TODO: We should move to non-global dirty marker.
175176
void MarkDirty(EndpointId aEndpointId, AttributeId aAttributeId)
176177
{
177178
MatterReportingAttributeChangeCallback(aEndpointId, Id, aAttributeId);

0 commit comments

Comments
 (0)