Retry wakeup — more consistent communications for x22 pumps #113
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…p message when first waking pump. This, combined with turning on my 722's 'remote' option, makes communication stable. May help others using x22 pumps. Warning: likely full of swift faux pas. Hacking around swift for first time. Apologies. Test plan: install rileylink_ios on phone, tap to manual 'get pump model', verify that pump model is returned and you don't see RileyLinkTimeout. ...then wait ~2 minutes (for pump's RF to turn off) and do it again. Do it a few times, verify that RileyLinkTimeout never shows up.
Test plan: build and run rileylink_ios on phone tap to manual 'get pump model' verify that pump model is properly retrieved/displayed shut off bluetooth connection in rileylink app, close rileylink app wait 2 minutes open rileylink app, turn on bluetooth connection physically separate pump and rileylink by quite a bit to make sure RF comms won't work well tap to manually 'get pump model' verify that RileyLinkTimeout properly shows up on screen.
| lastError = PumpCommsError.UnknownResponse("Wakeup shortResponse: \(shortResponse.txData)") | ||
| } | ||
| } catch let error { | ||
| print("Wakeup failure on attempt #\(attempt): \(error)") |
There was a problem hiding this comment.
This is probably unnecessary logging, since it's not something we previously logged.
Eventual stretch goal would be to remove all direct NSLog calls and let the framework's client choose how to handle logging, but that's a long way off.
Test plan: none
|
Changes look good to me. I want to wait for @ps2's blessing to merge. |
|
Looks good to me. I wanted to try it out, but haven't found time. I think we should go ahead and merge, since it's helping others. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Apologies if I should base this PR off of a branch other than dev. Lemme know the process you'd prefer.)
As discussed in gitter/slack, this PR adds retry logic to the initial short wakeup attempt, which has been verified by me and aemazaheri to increase likelihood of RL<=>pump communication success.
You and @loudnate know far more than I do about why that is the case ;). Happy to add more findings/context from my experience here if helpful.