Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history 
Will updated the code to make things work with suggestion soon.

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@lpbeliveau-silabs @bzbarsky-apple
lpbeliveau-silabs and bzbarsky-apple authored Nov 3, 2023
1 parent 35f6b8a commit 29c7f27
Showing 1 changed file with 18 additions and 17 deletions.
35 changes: 18 additions & 17 deletions src/app/clusters/scenes-server/scenes-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ CHIP_ERROR AddResponseOnError(CommandHandlerInterface::HandlerContext & ctx, Res
/// value for an existing one
/// @return
CHIP_ERROR updateFabricSceneInfo(EndpointId endpoint, FabricIndex fabric, Optional<GroupId> group, Optional<SceneId> scene,
Optional<bool> SceneValid)
Optional<bool> sceneValid)
{
VerifyOrReturnError(kInvalidEndpointId != endpoint, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(kUndefinedFabricIndex != fabric, CHIP_ERROR_INVALID_ARGUMENT);

SceneTable * sceneTable = scenes::GetSceneTableImpl(endpoint);
Structs::SceneInfoStruct::Type * SceneInfo = ScenesServer::Instance().GetSceneInfoStruct(endpoint, fabric);
Structs::SceneInfoStruct::Type * sceneInfo = ScenesServer::Instance().GetSceneInfoStruct(endpoint, fabric);
if (nullptr != SceneInfo)
{
if (group.HasValue())
Expand All @@ -125,9 +125,9 @@ CHIP_ERROR updateFabricSceneInfo(EndpointId endpoint, FabricIndex fabric, Option
Structs::SceneInfoStruct::Type newSceneInfo;
newSceneInfo.fabricIndex = fabric;

newSceneInfo.currentGroup = (group.HasValue()) ? group.Value() : 0;
newSceneInfo.currentScene = (scene.HasValue()) ? scene.Value() : 0;
newSceneInfo.sceneValid = (SceneValid.HasValue()) ? SceneValid.Value() : false;
newSceneInfo.currentGroup = group.ValueOr(0);
newSceneInfo.currentScene = scene.ValueOr(0);
newSceneInfo.sceneValid = sceneValid.ValueOr(false);

ReturnErrorOnFailure(sceneTable->GetFabricSceneCount(fabric, newSceneInfo.sceneCount));
ReturnErrorOnFailure(sceneTable->GetRemainingCapacity(fabric, newSceneInfo.remainingCapacity));
Expand Down Expand Up @@ -178,7 +178,7 @@ Structs::SceneInfoStruct::Type * ScenesServer::FabricSceneInfo::GetFabricSceneIn
return mSceneInfoStructs[endpointIndex];
}

/// @brief Gets the SceneInfoStruct () for a specific fabric for a specific endpoint
/// @brief Gets the SceneInfoStruct for a specific fabric for a specific endpoint
/// @param endpoint target endpoint
/// @param fabric target fabric
/// @param index
Expand All @@ -193,11 +193,11 @@ Structs::SceneInfoStruct::Type * ScenesServer::FabricSceneInfo::GetSceneInfoStru
return &mSceneInfoStructs[endpointIndex][sceneInfoStructIndex];
}

///@brief Sets the SceneInfoStruct () for a specific fabric for a specific endpoint
/// @brief Sets the SceneInfoStruct for a specific fabric for a specific endpoint
/// @param endpoint target endpoint
/// @param fabric target fabric
/// @param [in] sceneInfoStruct SceneInfoStruct to set
/// @return CHIP_NO_ERROR, CHIP_ERROR_NOT_FOUND if the endpoint is not found, CHIP_ERROR_NO_MEMORY if the number of fabric is
/// @return CHIP_NO_ERROR, CHIP_ERROR_NOT_FOUND if the endpoint is not found, CHIP_ERROR_NO_MEMORY if the number of fabrics is
/// exceeded, CHIP_ERROR_INVALID_ARGUMENT if invalid fabric or endpoint
CHIP_ERROR ScenesServer::FabricSceneInfo::SetSceneInfoStruct(EndpointId endpoint, FabricIndex fabric,
Structs::SceneInfoStruct::Type & sceneInfoStruct)
Expand All @@ -221,7 +221,7 @@ CHIP_ERROR ScenesServer::FabricSceneInfo::SetSceneInfoStruct(EndpointId endpoint
return CHIP_NO_ERROR;
}

/// @brief Clears the SceneInfoStruct () associated to a fabric and compresses the array to leave uninitialised structs at the end
/// @brief Clears the SceneInfoStruct associated to a fabric and compresses the array to leave uninitialised structs at the end
/// @param[in] endpoint target endpoint
/// @param[in] fabric target fabric
void ScenesServer::FabricSceneInfo::ClearSceneInfoStruct(EndpointId endpoint, FabricIndex fabric)
Expand Down Expand Up @@ -270,8 +270,8 @@ CHIP_ERROR ScenesServer::FabricSceneInfo::FindFabricSceneInfoIndex(EndpointId en
return CHIP_ERROR_NOT_FOUND;
}

/// @brief Returns the SceneInfoStruct () associated to a fabric
/// @param[in] fabric target fabric
/// @brief Returns the SceneInfoStruct associated to a fabric
/// @param[in] fabric target fabric index
/// @param[in] endpointIndex index of the corresponding FabricSceneInfo for an endpoint, corresponds to a row in the
/// mSceneInfoStructs array
/// @param[out] index index of the corresponding SceneInfoStruct if found, mFabricSceneInfo[endpoint] number of fabrics otherwise
Expand Down Expand Up @@ -628,7 +628,7 @@ CHIP_ERROR StoreSceneParse(const FabricIndex & fabricIdx, const EndpointId & end

// Update SceneInfo Attribute
ReturnErrorOnFailure(
updateFabricSceneInfo(endpointID, fabricIdx, Optional<GroupId>(groupID), Optional<SceneId>(sceneID), Optional<bool>(true)));
updateFabricSceneInfo(endpointID, fabricIdx, MakeOptional(groupID), MakeOptional(sceneID), MakeOptional(true)));

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -770,7 +770,7 @@ void ScenesServer::GroupWillBeRemoved(FabricIndex aFabricIx, EndpointId aEndpoin
SceneTable * sceneTable = scenes::GetSceneTableImpl(aEndpointId);
VerifyOrReturn(nullptr != sceneTable);

Structs::SceneInfoStruct::Type * SceneInfo = mFabricSceneInfo.GetSceneInfoStruct(aEndpointId, aFabricIx);
Structs::SceneInfoStruct::Type * sceneInfo = mFabricSceneInfo.GetSceneInfoStruct(aEndpointId, aFabricIx);
chip::GroupId currentGroup = (nullptr != SceneInfo) ? SceneInfo->currentGroup : 0x0000;

// If currentGroup is what is being removed, we can't possibly still have a valid scene,
Expand Down Expand Up @@ -896,10 +896,11 @@ void ScenesServer::HandleRemoveScene(HandlerContext & ctx, const Commands::Remov
Structs::SceneInfoStruct::Type * sceneInfo =
GetSceneInfoStruct(ctx.mRequestPath.mEndpointId, ctx.mCommandHandler.GetAccessingFabricIndex());
Optional<bool> sceneValid;
if (nullptr != sceneInfo)
{
sceneValid = (req.groupID == sceneInfo->currentGroup && req.sceneID == sceneInfo->currentScene) ? Optional<bool>(false)
: Optional<bool>();
if (nullptr != sceneInfo &&
req.groupID == sceneInfo->currentGroup &&
req.sceneID == sceneInfo->currentScene)
{
sceneValid.emplace(false);
}

ReturnOnFailure(
Expand Down

0 comments on commit 29c7f27

Please sign in to comment.