Skip to content

Conversation

bzbarsky-apple
Copy link
Contributor

This reverts commit ad5403e.

There are two issues:

We need to sort out why CHIPDeviceController is canceling BLE connections when starting PASE over BLE (!).

Fixes #28139

…) is called (project-chip#27304)"

This reverts commit ad5403e.

There are two issues:

* project-chip#27607 which has had no
  response for weeks.
* project-chip#28139 which breaks
  commissioning on Mac, and would break it on Linux if it implemented BLE
  connection cancellation.

We need to sort out why CHIPDeviceController is canceling BLE connections when
starting PASE over BLE (!).
@woody-apple
Copy link
Contributor

@bzbarsky-apple The change makes logical sense though, to cancel any connections being established though? Why is this causing issues?

@woody-apple woody-apple removed hotfix urgent fix needed, can bypass review fast track labels Jul 21, 2023
@bzbarsky-apple
Copy link
Contributor Author

@woody-apple Because of this stack:

  * frame #0: 0x0000000101bf916c chip-tool`chip::Ble::BleLayer::CancelBleIncompleteConnection(this=0x0000000102beb5a0) at BleLayer.cpp:372:5
    frame #1: 0x0000000101bf8cd4 chip-tool`chip::Ble::BleLayer::CloseAllBleConnections(this=0x0000000102beb5a0) at BleLayer.cpp:312:22
    frame #2: 0x0000000102260b80 chip-tool`chip::Controller::DeviceCommissioner::ReleaseCommissioneeDevice(this=0x0000000108d11900, device=0x0000000107915e80) at CHIPDeviceController.cpp:560:35
    frame #3: 0x0000000102265d64 chip-tool`chip::Controller::DeviceCommissioner::OnDiscoveredDeviceOverBleSuccess(appState=0x0000000108d11900, connObj=0x000000010bf953c0) at CHIPDeviceController.cpp:795:15

Now why we end up doing CloseAllBleConnections from establishing a BLE connection I don't know, but until we stop doing that, we can't cancel the actual connection that got established from there.... For now this gets us back to the known-good state we were in since 1.0. Once we figure out what's going on with that bit and the shutdown issues #27607 describes, we can put this back.

Copy link
Contributor

@vivien-apple vivien-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should revert as you suggest. The original patch may probably be retargeted to call mSystemState->BleLayer()->CancelBleIncompleteConnection(); in DeviceCommissioner::OnSessionEstablishmentError.

Something like:

void DeviceCommissioner::OnSessionEstablishmentError(CHIP_ERROR err)
{
   ...
   if (mPairingDelegate != nullptr)
   {
        mPairingDelegate->OnStatusUpdate(DevicePairingDelegate::SecurePairingFailed);
    }

    auto device = self->mDeviceInPASEEstablishment;
    if (nullptr != device && device->GetDeviceTransportType() == Transport::Type::kBle)
    {
        mSystemState->BleLayer()->CancelBleIncompleteConnection();
    }

    RendezvousCleanup(err);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Unable to pair M5 board / nordic using chip tool

5 participants