Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Group] Fix group messaging after test were disabled #13505

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion examples/chip-tool/commands/common/CommandInvoker.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,11 @@ class CommandInvoker final : public ResponseReceiver<typename RequestType::Respo
{
return CHIP_ERROR_NO_MEMORY;
}
ReturnErrorOnFailure(commandSender->SendCommandRequest(session));
ReturnErrorOnFailure(commandSender->SendCommandRequest(session.Value()));

commandSender.release();
exchangeManager->GetSessionManager()->RemoveGroupSession(session.Value()->AsGroupSession());
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved

return CHIP_NO_ERROR;
}
};
Expand Down
12 changes: 11 additions & 1 deletion src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,17 @@ TLV::TLVWriter * CommandHandler::GetCommandDataIBTLVWriter()

FabricIndex CommandHandler::GetAccessingFabricIndex() const
{
return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
FabricIndex fabric = kUndefinedFabricIndex;
if (mpExchangeCtx->GetSessionHandle()->IsGroupSession())
{
fabric = mpExchangeCtx->GetSessionHandle()->AsGroupSession()->GetFabricIndex();
}
else
{
fabric = mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
}
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved

return fabric;
}

CommandHandler * CommandHandler::Handle::Get()
Expand Down
12 changes: 11 additions & 1 deletion src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,17 @@ CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathPar

FabricIndex WriteHandler::GetAccessingFabricIndex() const
{
return mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
FabricIndex fabric = kUndefinedFabricIndex;
if (mpExchangeCtx->GetSessionHandle()->IsGroupSession())
{
fabric = mpExchangeCtx->GetSessionHandle()->AsGroupSession()->GetFabricIndex();
}
else
{
fabric = mpExchangeCtx->GetSessionHandle()->AsSecureSession()->GetFabricIndex();
}

return fabric;
}

const char * WriteHandler::GetStateStr() const
Expand Down
2 changes: 1 addition & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint
// for (iterate through all GroupDataProvider multicast Address)
// {
#ifdef CHIP_ENABLE_GROUP_MESSAGING_TESTS
err = mTransports.MulticastGroupJoinLeave(Transport::PeerAddress::Multicast(1, 1234), true);
err = mTransports.MulticastGroupJoinLeave(Transport::PeerAddress::Multicast(0, 1234), true);
SuccessOrExit(err);
#endif
//}
Expand Down
19 changes: 14 additions & 5 deletions src/app/tests/suites/TestGroupMessaging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# !!!!!!!!!! TEST INFORMATION !!!!!!!!!!!!!!!!!!
# This test file tests Group Multicast Messaging.
# For this test to work, A Group Write/Command and a unicast read need to occur to be able to confirm that the group Communication works. Every test comes in a pair
# Only Works on Linux machines
# When building chip-tool, chip_enable_group_messaging_tests needs to be set to true in the build command for the test to pass
# ./scripts/examples/gn_build_example.sh examples/all-clusters-app/linux out/debug/standalone/ chip_config_network_layer_ble=false is_tsan=true chip_enable_group_messaging_tests=true
name: Basic Group Messaging Tests

config:
Expand All @@ -26,41 +32,44 @@ tests:
#TODO : Add Group membership command when implemented Issue #11077
# - label: "Add device to Group"
# command: "TODO"

# Test Pair 1 : Sends a Group Write Attribute
- label: "Group Write Attribute"
command: "writeAttribute"
attribute: "location"
groupId: "1234"
arguments:
value: "us"
#Disabled due to issue-12983

# Test Pair 1 : Validates previous group write attribute with a unicast to read
- label: "Read back Attribute"
disabled: true
command: "readAttribute"
attribute: "location"
response:
value: "us"

# Test Pair 2 : Sends a Group Write Attribute
- label: "Restore initial location value"
command: "writeAttribute"
attribute: "location"
groupId: "1234"
arguments:
value: ""
#Disabled due to issue-12983

# Test Pair 2 : Validates previous group write attribute with a unicast to read
- label: "Read back Attribute"
disabled: true
command: "readAttribute"
attribute: "location"
response:
value: ""

# Test Pair 3 : Sends a Group command
- label: "Turn On the light to see attribute change"
cluster: "On/Off"
command: "On"
groupId: "1234"
disabled: true

# Test Pair 3 : Validates previous group command with a unicast to read
- label: "Check on/off attribute value is true after on command"
cluster: "On/Off"
command: "readAttribute"
Expand Down
13 changes: 8 additions & 5 deletions src/controller/CHIPCluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class DLL_EXPORT ClusterBase
WriteResponseSuccessCallback successCb, WriteResponseFailureCallback failureCb,
const Optional<uint16_t> & aTimedWriteTimeoutMs, WriteResponseDoneCallback doneCb = nullptr)
{
CHIP_ERROR err = CHIP_NO_ERROR;
VerifyOrReturnError(mDevice != nullptr, CHIP_ERROR_INCORRECT_STATE);

auto onSuccessCb = [context, successCb](const app::ConcreteAttributePath & commandPath) {
Expand All @@ -141,15 +142,17 @@ class DLL_EXPORT ClusterBase

if (mGroupSession)
{
return chip::Controller::WriteAttribute<AttrType>(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData,
onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
err = chip::Controller::WriteAttribute<AttrType>(mGroupSession.Get(), mEndpoint, clusterId, attributeId, requestData,
onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
mDevice->GetExchangeManager()->GetSessionManager()->RemoveGroupSession(mGroupSession->AsGroupSession());
}
else
{
return chip::Controller::WriteAttribute<AttrType>(mDevice->GetSecureSession().Value(), mEndpoint, clusterId,
attributeId, requestData, onSuccessCb, onFailureCb,
aTimedWriteTimeoutMs, onDoneCb);
err = chip::Controller::WriteAttribute<AttrType>(mDevice->GetSecureSession().Value(), mEndpoint, clusterId, attributeId,
requestData, onSuccessCb, onFailureCb, aTimedWriteTimeoutMs, onDoneCb);
}

return err;
}

template <typename AttributeInfo>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,8 @@ void GenericThreadStackManagerImpl_OpenThread_LwIP<ImplClass>::UpdateThreadInter
it->Release();
}
}

err = transportMgr->MulticastGroupJoinLeave(Transport::PeerAddress::Multicast(0, 1234), true);
}
#endif // CHIP_SYSTEM_CONFIG_USE_OPEN_THREAD_UDP
}
Expand Down
7 changes: 7 additions & 0 deletions src/transport/GroupSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,13 @@ class GroupSessionTable
}
}

/**
* @brief Deletes an entry from the
mkardous-silabs marked this conversation as resolved.
Show resolved Hide resolved
*
* @param entry
*/
void DeleteEntry(GroupSession * entry) { mEntries.ReleaseObject(entry); }

private:
BitMapObjectPool<GroupSession, kMaxSessionCount> mEntries;
};
Expand Down
11 changes: 5 additions & 6 deletions src/transport/SessionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,12 +540,6 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade
return; // malformed packet
}

Optional<SessionHandle> session = FindGroupSession(packetHeader.GetDestinationGroupId().Value());
if (!session.HasValue())
{
return;
}

if (msg.IsNull())
{
ChipLogError(Inet, "Secure transport received Groupcast NULL packet, discarding");
Expand Down Expand Up @@ -603,7 +597,12 @@ void SessionManager::SecureGroupMessageDispatch(const PacketHeader & packetHeade

if (mCB != nullptr)
{
Optional<SessionHandle> session = CreateGroupSession(packetHeader.GetDestinationGroupId().Value());
VerifyOrReturn(session.HasValue(), ChipLogError(Inet, "Error when creating group session handle."));

mCB->OnMessageReceived(packetHeader, payloadHeader, session.Value(), peerAddress, isDuplicate, std::move(msg));

RemoveGroupSession(session.Value()->AsGroupSession());
}
}

Expand Down
1 change: 1 addition & 0 deletions src/transport/SessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ class DLL_EXPORT SessionManager : public TransportMgrDelegate
// TODO: implements group sessions
Optional<SessionHandle> CreateGroupSession(GroupId group) { return mGroupSessions.AllocEntry(group, kUndefinedFabricIndex); }
Optional<SessionHandle> FindGroupSession(GroupId group) { return mGroupSessions.FindEntry(group, kUndefinedFabricIndex); }
void RemoveGroupSession(Transport::GroupSession * session) { mGroupSessions.DeleteEntry(session); }

// TODO: this is a temporary solution for legacy tests which use nodeId to send packets
// and tv-casting-app that uses the TV's node ID to find the associated secure session
Expand Down
90 changes: 79 additions & 11 deletions zzz_generated/chip-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.