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

Exponential Auto-Reconnect #287

Closed
wants to merge 2 commits into from

Conversation

sachinvas
Copy link
Contributor

Hi @HJianBo, As discussed in #286.
Below is the PR, can you please review and merge?

Copy link

@Abhijith-net Abhijith-net left a comment

Choose a reason for hiding this comment

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

Thanks for UT.
LGTM 👍

@Abhijith-net
Copy link

@HJianBo, Could you pls help merge. This will save battery for iOS devices, especially who have auto-reconnect ON!

Thanks in advance!

autoReconnTimer = CocoaMQTTTimer.every(Double(autoReconnectTimeInterval), { [weak self] in
printInfo("Try reconnect")
_ = self?.connect()
guard let weakSelf = self else {

Choose a reason for hiding this comment

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

This is not a weakSelf, instead, when creating this let it turns into a strongSelf.
What's the reasoning for this naming convention?
I have at least seen it once other time in the codebase

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @halonsoluis, sorry for wrong convention, I will be updating it. Thanks. https://github.com/apple/swift-evolution/blob/master/proposals/0079-upgrade-self-from-weak-to-strong.md is a swift proposal, I meant to use it.

@Abhijith-net
Copy link

@HJianBo We have 2 approvals now, need your help now!

@HJianBo
Copy link
Member

HJianBo commented Jul 25, 2019

Sorry guys, I am very busy in recently. I'll check it now

@HJianBo
Copy link
Member

HJianBo commented Jul 25, 2019

Hi guys, I have reviewed these commits but it has some potential problems and incorrect implement for reconnecting mechanism by Timer.every(_, _) So, I make some changes to improve it.

Unfortunately I have to open a new PR to commit it. Please to see #290

@HJianBo HJianBo closed this Jul 25, 2019
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.

None yet

5 participants