Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

KeepAliveGatt EventHandler #74

Merged
merged 35 commits into from
Jul 17, 2020
Merged

KeepAliveGatt EventHandler #74

merged 35 commits into from
Jul 17, 2020

Conversation

sdonn3
Copy link
Contributor

@sdonn3 sdonn3 commented Jul 9, 2020

Allows the consumer of this API to provide lambdas to run on either connection or disconnection events.

Rendered markdown of the updated documentation re: Events is available here.

sdonn3 added 2 commits July 9, 2020 15:45
…this API to set lambdas to be ran either onConnect or onDisconnect events
@sdonn3 sdonn3 requested a review from twyatt July 9, 2020 22:52
Copy link
Member

@twyatt twyatt left a comment

Choose a reason for hiding this comment

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

Did a quick review. I'll find a bit more time to give more feedback.

You can update API via: ./gradlew apiDump

@twyatt
Copy link
Member

twyatt commented Jul 10, 2020

Unfortunately Android Studio and Kotlinter do not agree on import ordering. I think JetBrains has a fix coming so Android Studio can play nicer with Kotlinter.

Until then, I've opted to have Kotlinter enforce it's import ordering. It's slightly cumbersome but at least makes ordering enforceable by CI. So you'll have to run ./gradlew formatKotlin whenever your CI build fails due to import ordering. Small price to pay for consistency? ¯\_(ツ)_/¯

@sdonn3 sdonn3 changed the title KeepAliveGatt onEventAction KeepAliveGatt onEvent Jul 10, 2020
@sdonn3 sdonn3 requested a review from twyatt July 14, 2020 00:20
@twyatt twyatt changed the title KeepAliveGatt onEvent KeepAliveGatt EventHandler Jul 14, 2020
@sdonn3 sdonn3 requested a review from twyatt July 14, 2020 19:47
* @return `true` if connection was established (then dropped), or `false` if connection attempt failed.
* @throws ConnectionRejected if the operating system rejects the connection request.
*/
private suspend fun establishConnection(): Boolean {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this new naming way better

Copy link
Member

@twyatt twyatt left a comment

Choose a reason for hiding this comment

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

Pushed the fixes we discussed. Please add tests around event handling and re-request review. Thanks!

@twyatt twyatt requested a review from Chris-Corea July 16, 2020 06:46
@twyatt
Copy link
Member

twyatt commented Jul 16, 2020

@Chris-Corea re-requested your review as we had to make substantial changes since you reviewed.

@twyatt twyatt merged commit a6e47cd into develop Jul 17, 2020
@twyatt twyatt deleted the steve/esc-965_onDisconnect branch July 17, 2020 05:53
@twyatt twyatt added this to the 1.0.0 milestone Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants