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

refactor: Rename CloseSession to MarkSessionDefunct and add MarkSessi… #37903

Merged
Show file tree
Hide file tree
Changes from all 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
18 changes: 16 additions & 2 deletions src/controller/python/ChipDeviceController-ScriptBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,8 @@ PyChipError pychip_DeviceController_SetCheckMatchingFabric(bool check);
struct IcdRegistrationParameters;
PyChipError pychip_DeviceController_SetIcdRegistrationParameters(bool enabled, const IcdRegistrationParameters * params);
PyChipError pychip_DeviceController_ResetCommissioningParameters();
PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
PyChipError pychip_DeviceController_MarkSessionDefunct(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
PyChipError pychip_DeviceController_MarkSessionForEviction(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid);
PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::DeviceCommissioner * devCtrl, const char * peerAddrStr,
uint32_t setupPINCode, chip::NodeId nodeid, uint16_t port);
PyChipError pychip_DeviceController_EstablishPASESessionBLE(chip::Controller::DeviceCommissioner * devCtrl, uint32_t setupPINCode,
Expand Down Expand Up @@ -661,7 +662,7 @@ PyChipError pychip_DeviceController_ResetCommissioningParameters()
return ToPyChipError(CHIP_NO_ERROR);
}

PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
PyChipError pychip_DeviceController_MarkSessionDefunct(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
{
//
// Since we permit multiple controllers per fabric and each is associated with a unique fabric index, closing a session
Expand All @@ -678,6 +679,19 @@ PyChipError pychip_DeviceController_CloseSession(chip::Controller::DeviceCommiss
return ToPyChipError(CHIP_NO_ERROR);
}

PyChipError pychip_DeviceController_MarkSessionForEviction(chip::Controller::DeviceCommissioner * devCtrl, chip::NodeId nodeid)
{
devCtrl->SessionMgr()->ForEachMatchingSessionOnLogicalFabric(ScopedNodeId(nodeid, devCtrl->GetFabricIndex()),
[](auto * session) {
if (session->IsActiveSession())
{
session->MarkForEviction();
}
});

return ToPyChipError(CHIP_NO_ERROR);
}

PyChipError pychip_DeviceController_EstablishPASESessionIP(chip::Controller::DeviceCommissioner * devCtrl, const char * peerAddrStr,
uint32_t setupPINCode, chip::NodeId nodeid, uint16_t port)
{
Expand Down
58 changes: 51 additions & 7 deletions src/controller/python/chip/ChipDeviceCtrl.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#
# Copyright (c) 2020-2022 Project CHIP Authors
# Copyright (c) 2020-2025 Project CHIP Authors
# Copyright (c) 2019-2020 Google, LLC.
# Copyright (c) 2013-2018 Nest Labs, Inc.
# All rights reserved.
Expand Down Expand Up @@ -702,12 +702,52 @@ def ExpireSessions(self, nodeid):

self._ChipStack.Call(lambda: self._dmLib.pychip_ExpireSessions(self.devCtrl, nodeid)).raise_on_error()

# TODO: This needs to be called MarkSessionDefunct
def CloseSession(self, nodeid):
def MarkSessionDefunct(self, nodeid):
'''
Marks a previously active session with the specified node as defunct to temporarily prevent it
from being used with new exchanges to send or receive messages. This should be called when there
is suspicion of a loss-of-sync with the session state on the associated peer, such as evidence
of transport failure.

If messages are received thereafter on this session, the session will be put back into the Active state.

This function should only be called on an active session.
This will NOT detach any existing SessionHolders.

Parameters:
nodeid (int): The node ID of the device whose session should be marked as defunct.

Raises:
RuntimeError: If the controller is not active.
PyChipError: If the operation fails.
'''
self.CheckIsActive()

self._ChipStack.Call(
lambda: self._dmLib.pychip_DeviceController_MarkSessionDefunct(
self.devCtrl, nodeid)
).raise_on_error()

def MarkSessionForEviction(self, nodeid):
'''
Marks the session with the specified node for eviction. It will first detach all SessionHolders
attached to this session by calling 'OnSessionReleased' on each of them. This will force them
to release their reference to the session. If there are no more references left, the session
will then be de-allocated.

Once marked for eviction, the session SHALL NOT ever become active again.

Parameters:
nodeid (int): The node ID of the device whose session should be marked for eviction.

Raises:
RuntimeError: If the controller is not active.
PyChipError: If the operation fails.
'''
self.CheckIsActive()

self._ChipStack.Call(
lambda: self._dmLib.pychip_DeviceController_CloseSession(
lambda: self._dmLib.pychip_DeviceController_MarkSessionForEviction(
self.devCtrl, nodeid)
).raise_on_error()

Expand Down Expand Up @@ -1873,9 +1913,13 @@ def _InitLib(self):
c_void_p, c_uint64, _DeviceUnpairingCompleteFunct]
self._dmLib.pychip_DeviceController_UnpairDevice.restype = PyChipError

self._dmLib.pychip_DeviceController_CloseSession.argtypes = [
self._dmLib.pychip_DeviceController_MarkSessionDefunct.argtypes = [
c_void_p, c_uint64]
self._dmLib.pychip_DeviceController_CloseSession.restype = PyChipError
self._dmLib.pychip_DeviceController_MarkSessionDefunct.restype = PyChipError

self._dmLib.pychip_DeviceController_MarkSessionForEviction.argtypes = [
c_void_p, c_uint64]
self._dmLib.pychip_DeviceController_MarkSessionForEviction.restype = PyChipError

self._dmLib.pychip_DeviceController_GetAddressAndPort.argtypes = [
c_void_p, c_uint64, c_char_p, c_uint64, POINTER(c_uint16)]
Expand Down Expand Up @@ -2261,7 +2305,7 @@ async def CommissionOnNetwork(self, nodeId: int, setupPinCode: int,
return await asyncio.futures.wrap_future(ctx.future)

def get_rcac(self):
'''
'''
Passes captured RCAC data back to Python test modules for validation
- Setting buffer size to max size mentioned in spec:
- Ref: https://github.com/CHIP-Specifications/connectedhomeip-spec/blob/06c4d55962954546ecf093c221fe1dab57645028/policies/matter_certificate_policy.adoc#615-key-sizes
Expand Down
2 changes: 1 addition & 1 deletion src/controller/python/chip/commissioning/pase.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def __enter__(self) -> Session:
device=self.devCtrl.GetConnectedDeviceSync(self.node_id, allowPASE=True, timeoutMs=1000))

def __exit__(self, type, value, traceback):
self.devCtrl.CloseSession(self.node_id)
self.devCtrl.MarkSessionDefunct(self.node_id)
if self.is_ble:
self.devCtrl.CloseBLEConnection(self.is_ble)

Expand Down
12 changes: 6 additions & 6 deletions src/controller/python/test/test_scripts/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ async def TestCaseEviction(self, nodeid: int):
# the controller and target.
#
for x in range(minimumSupportedFabrics * minimumCASESessionsPerFabric * 2):
self.devCtrl.CloseSession(nodeid)
self.devCtrl.MarkSessionDefunct(nodeid)
await self.devCtrl.ReadAttribute(nodeid, [(Clusters.BasicInformation.Attributes.ClusterRevision)])

self.logger.info("Testing CASE defunct logic")
Expand Down Expand Up @@ -700,7 +700,7 @@ def OnValueChange(path: Attribute.TypedAttributePath, transaction: Attribute.Sub
#
# This marks the session defunct.
#
self.devCtrl.CloseSession(nodeid)
self.devCtrl.MarkSessionDefunct(nodeid)

#
# Now write the attribute from fabric2, give it some time before checking if the report
Expand Down Expand Up @@ -732,7 +732,7 @@ def OnValueChange(path: Attribute.TypedAttributePath, transaction: Attribute.Sub
sub.SetAttributeUpdateCallback(OnValueChange)

for x in range(minimumSupportedFabrics * minimumCASESessionsPerFabric * 2):
self.devCtrl2.CloseSession(nodeid)
self.devCtrl2.MarkSessionDefunct(nodeid)
await self.devCtrl2.ReadAttribute(nodeid, [(Clusters.BasicInformation.Attributes.ClusterRevision)])

#
Expand Down Expand Up @@ -760,7 +760,7 @@ def OnValueChange(path: Attribute.TypedAttributePath, transaction: Attribute.Sub
sub.SetAttributeUpdateCallback(OnValueChange)

for x in range(minimumSupportedFabrics * minimumCASESessionsPerFabric * 2):
self.devCtrl.CloseSession(nodeid)
self.devCtrl.MarkSessionDefunct(nodeid)
await self.devCtrl.ReadAttribute(nodeid, [(Clusters.BasicInformation.Attributes.ClusterRevision)])

await self.devCtrl.WriteAttribute(nodeid, [(1, Clusters.UnitTesting.Attributes.Int8u(6))])
Expand Down Expand Up @@ -1072,7 +1072,7 @@ async def OnResubscriptionSucceeded(transaction):
def TestCloseSession(self, nodeid: int):
self.logger.info(f"Closing sessions with device {nodeid}")
try:
self.devCtrl.CloseSession(nodeid)
self.devCtrl.MarkSessionDefunct(nodeid)
return True
except Exception as ex:
self.logger.exception(
Expand Down Expand Up @@ -1476,7 +1476,7 @@ async def TestSubscriptionResumptionCapacityStep2(self, nodeid: int, endpoint: i

self.logger.info("Send a new subscription request from the second controller")
# Close previous session so that the second controller will res-establish the session with the remote device
self.devCtrl.CloseSession(nodeid)
self.devCtrl.MarkSessionDefunct(nodeid)
await self.devCtrl.ReadAttribute(nodeid, [(endpoint, Clusters.BasicInformation.Attributes.NodeLabel)], None,
False, reportInterval=(1, 50),
keepSubscriptions=True, autoResubscribe=False)
Expand Down
2 changes: 1 addition & 1 deletion src/python_testing/TC_CGEN_2_8.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ async def test_TC_CGEN_2_8(self):
self.wait_for_user_input(prompt_msg="Manually trigger factory reset on the DUT, then continue")

# Close the commissioner session with the device to clean up resources
commissioner.CloseSession(nodeid=self.dut_node_id)
commissioner.MarkSessionDefunct(nodeid=self.dut_node_id)

# Step 6: Put device in commissioning mode (requiring user input, so skip in CI)
self.step(6)
Expand Down
2 changes: 1 addition & 1 deletion src/python_testing/TC_CGEN_2_9.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ async def test_TC_CGEN_2_9(self):
await self.remove_commissioner_fabric()

# Close the commissioner session with the device to clean up resources
commissioner.CloseSession(nodeid=self.dut_node_id)
commissioner.MarkSessionDefunct(nodeid=self.dut_node_id)

# Step 6: Put device in commissioning mode (requiring user input, so skip in CI)
self.step(6)
Expand Down
2 changes: 1 addition & 1 deletion src/python_testing/TC_ICDM_3_4.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ async def test_TC_ICDM_3_4(self):
paaTrustStorePath=str(self.config.paa_trust_store_path),
catTags=self.config.controller_cat_tags
)
devCtrl.CloseSession(self.dut_node_id)
devCtrl.MarkSessionDefunct(self.dut_node_id)
icdCounter2 = await self._read_icdm_attribute_expect_success(attribute=attributes.ICDCounter)
asserts.assert_greater_equal(icdCounter2, icdCounter1,
"ICDCounter have reboot is not greater or equal to the ICDCounter read before the reboot.")
Expand Down
2 changes: 1 addition & 1 deletion src/test_driver/mbed/integration_tests/common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ def close_connection(devCtrl, nodeId):
:return: true if successful, otherwise false
"""
try:
devCtrl.CloseSession(nodeId)
devCtrl.MarkSessionDefunct(nodeId)
except exceptions.ChipStackException as ex:
log.error("Close session failed: {}".format(str(ex)))
return False
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def disconnect_device(devCtrl, nodeId):
:return: node ID if connection successful or None if failed
"""
try:
devCtrl.CloseSession(nodeId)
devCtrl.MarkSessionDefunct(nodeId)
except exceptions.ChipStackException as ex:
log.error("CloseSession failed {}".format(str(ex)))
return False
Expand Down
Loading