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.

Sign up for GitHub

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

Jump to bottom

Check bluetooth state prior to connecting #291

Merged
merged 11 commits into from
Apr 9, 2022
Merged

Check bluetooth state prior to connecting #291

merged 11 commits into from
Apr 9, 2022

Conversation

burnhamd
Copy link
Contributor

Refs #290

@twyatt twyatt self-requested a review March 29, 2022 23:51
twyatt
twyatt reviewed Mar 30, 2022
View reviewed changes
core/src/appleMain/kotlin/Peripheral.kt Outdated Show resolved Hide resolved
core/src/appleMain/kotlin/Peripheral.kt Outdated Show resolved Hide resolved
core/src/appleMain/kotlin/Peripheral.kt Outdated Show resolved Hide resolved
@twyatt twyatt added this to the 0.17.0 milestone Apr 1, 2022
@davertay-j davertay-j requested a review from twyatt April 1, 2022 20:53
twyatt
twyatt previously approved these changes Apr 1, 2022
View reviewed changes
core/src/appleMain/kotlin/CentralManagerDelegate.kt Outdated Show resolved Hide resolved
core/src/appleMain/kotlin/Peripheral.kt Outdated Show resolved Hide resolved
@twyatt twyatt self-requested a review April 1, 2022 21:30
@twyatt twyatt dismissed their stale review April 1, 2022 21:31

Realized that we can update to be more consistent with Android implementation.

@twyatt twyatt linked an issue Apr 6, 2022 that may be closed by this pull request
Connecting to device with bluetooth powered off results in device showing connecting status which doesn't change #290
Closed
@twyatt twyatt added the apple label Apr 6, 2022
@twyatt twyatt changed the title Added connect to peripheral while Bluetooth turned off in IOS Check bluetooth state prior to connecting Apr 6, 2022
davertay-j
davertay-j approved these changes Apr 8, 2022
View reviewed changes
Copy link
Contributor

@davertay-j davertay-j left a comment

Choose a reason for hiding this comment

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

Wondering why we are using the deprecated CBCentralManagerState over the newer CBManagerState? Not a blocker as they are defined identically and probably won't go away anytime soon, but am curious if there is a technical reason.

@twyatt
Copy link
Member

twyatt commented Apr 8, 2022

Wondering why we are using the deprecated CBCentralManagerState over the newer CBManagerState? Not a blocker as they are defined identically and probably won't go away anytime soon, but am curious if there is a technical reason.

I didn't comment on it because we were already using CBCentralManagerState in the codebase, so wanted to remain consistent with adding additional constants in this PR. 🤷

I figured we'd replace all the constants in one pass (#299), so: not due to technical reasons.

@twyatt twyatt enabled auto-merge (squash) April 9, 2022 05:08
@twyatt twyatt merged commit aea9303 into JuulLabs:main Apr 9, 2022
@twyatt twyatt mentioned this pull request Apr 9, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davertay-j davertay-j davertay-j approved these changes

@QuantumRand QuantumRand QuantumRand approved these changes

@twyatt twyatt twyatt approved these changes

@Phoenix7351 Phoenix7351 Awaiting requested review from Phoenix7351

@cedrickcooke cedrickcooke Awaiting requested review from cedrickcooke

@sdonn3 sdonn3 Awaiting requested review from sdonn3

Assignees
No one assigned
Labels
apple
Projects
None yet
Milestone
0.16.1
5 participants
@burnhamd @twyatt @lammertw @QuantumRand @davertay-j