Skip to content

Commit

Permalink
Stop calling OnPairingComplete on CASE session establishment.
Browse files Browse the repository at this point in the history
OnPairingComplete is documented as indicating the end of PASE
establishment, but our implementation calls it for _some_ CASE
sessions as well: only the ones we kick off from UpdateDevice.

This is unexpected by API consumers, and we should not be doing it.

Fixes project-chip#18230
  • Loading branch information
bzbarsky-apple committed May 17, 2022
1 parent d79a0e3 commit 4d761a3
Showing 1 changed file with 15 additions and 27 deletions.
42 changes: 15 additions & 27 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1510,32 +1510,24 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDevicePr
DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
VerifyOrReturn(commissioner != nullptr, ChipLogProgress(Controller, "Device connected callback with null context. Ignoring"));

if (commissioner->mCommissioningStage == CommissioningStage::kFindOperational)
if (commissioner->mCommissioningStage != CommissioningStage::kFindOperational)
{
if (commissioner->mDeviceBeingCommissioned != nullptr &&
commissioner->mDeviceBeingCommissioned->GetDeviceId() == device->GetDeviceId() &&
commissioner->mCommissioningDelegate != nullptr)
{
CommissioningDelegate::CommissioningReport report;
report.Set<OperationalNodeFoundData>(OperationalNodeFoundData(device));
commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
}
// This call is definitely not us finding our commissionee device.
// This is presumably us trying to re-establish CASE on MRP failure.
return;
}
else

if (commissioner->mDeviceBeingCommissioned == nullptr ||
commissioner->mDeviceBeingCommissioned->GetDeviceId() != device->GetDeviceId()) {
// Not the device we are trying to commission.
return;
}

if (commissioner->mCommissioningDelegate != nullptr)
{
if (commissioner->mPairingDelegate != nullptr)
{
commissioner->mPairingDelegate->OnPairingComplete(CHIP_NO_ERROR);
}
// Only release the PASE session if we're not commissioning. If we're commissioning, we're going to hold onto that PASE
// session until we send the commissioning complete command just in case it fails and we need to go back to the PASE
// connection to re-setup the network. This is unlikely, given that we just connected over the operational network, but is
// required by the spec.
CommissioneeDeviceProxy * commissionee = commissioner->FindCommissioneeDevice(device->GetDeviceId());
if (commissionee != nullptr)
{
commissioner->ReleaseCommissioneeDevice(commissionee);
}
CommissioningDelegate::CommissioningReport report;
report.Set<OperationalNodeFoundData>(OperationalNodeFoundData(device));
commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
}
}

Expand All @@ -1562,10 +1554,6 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer
{
commissioner->CommissioningStageComplete(error);
}
else
{
commissioner->mPairingDelegate->OnPairingComplete(error);
}
commissioner->mSystemState->CASESessionMgr()->ReleaseSession(peerId);
}

Expand Down

0 comments on commit 4d761a3

Please sign in to comment.