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

Fix re-sending config data #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cookie777
Copy link

Issue

The current ble connection freezes if you try to connect (establish the session) to the device again.

Details

  • The first time you use connect function in. the ESPDevice, it works.
  • However, if the completion status is error and if we try to use this function again, the app will freeze.
espDevice.connect(delegate: self) { status in
}

Reason

The DispatchSemaphore doesn't call signal() when it catches errors.

Details

  • Whenever we call connect or sendData from ESPDevice, internally we all func SendConfigData
  • This function locks the process by transportToken.wait()and releases it when the process finishes.
  • However, transportToken.signal() is not called when the process ends with error.
    • When we call espressifPeripheral.writeValue in the function, delegate functions didWriteValueFor or didUpdateValueFor is called.
    • When it catches errors, it completes with only handler.(without signal())
        guard error == nil else {
            currentRequestCompletionHandler?(nil, error)
            return
        }

Solution

Set transportToken.signal() in delegate functions didWriteValueFor and didUpdateValueFor when it catches error.

        guard error == nil else {
            transportToken.signal() // This is needed to release Semaphore
            currentRequestCompletionHandler?(nil, error)
            return
        }

@CLAassistant
Copy link

CLAassistant commented Apr 28, 2022

CLA assistant check
All committers have signed the CLA.

@vikas-chandra-mnnit
Copy link
Contributor

Hi @cookie777, we have removed DispatchSemaphore in our latest version of SDK I.e. 2.0.15. However, if you think there is some scenario in which use of the Semaphore is essential then do let us know so that we can quickly revert our changes and merge your PR for the next build.

@cookie777
Copy link
Author

Hello @vikas-chandra-mnnit .
I'm happy with removing DispatchSemaphore 👍 Thank you for your support!

@puneet-arora15
Copy link

@cookie777 Did the issue get resolved for you with the latest SDK ? As I'm still seeing this issue in the app

@cookie777
Copy link
Author

I havn't faced this issue after all transportToken.signal() are removed from the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants