forked from randallknutson/OmniBLE
-
Notifications
You must be signed in to change notification settings - Fork 53
Update OmniBLE tidepool-merge with improvements from dev #142
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
Merged
Conversation
This file contains hidden or 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
+ Refactored for more uniform date and time display for all cases + New optional -v command line option to enable all print options + Enable new pod disconnect/reconnect message printing with -v command line option + Enable message address and seq #'s printing with -v command line option + Disable new unacknowledged message error printing with -q command line option + Disable date printing with timestamps with -q command line option
+ Remove unneeded & redundant didSend() & didReceive() message logging + Improved and additional PodState debugDescription display for insulin values + Improved pod suspend testing when updating delivery status + Update suspended, bolusing, tempBasalRunning, extendedBolusRunning definitions for better efficency and clarity using @dnzxy suggestion on OmniBLE PR #123 + Use supportedTempBasalRates var for better OmniKit consistency
+ Don't append an optional beep block to a getStatus with a pending unacknowledged command + Handle unacknowledged commands cases in acknowledgeAlerts(), beepConfig() & configureAlert() + Use checkCommandAgainstStatus() for seq # mismatch to determine insulin delivery status + Remove unneeded @discardResult attribute on various PodCommsSessions functions + Comment improvements and better source code synchronization between OmniKit and OmniBLE
…sions + Add new PodCommsSessions.getStatus parameter to specify noSeqGetStatus + Use new getStatus for post-connect, resuming setup and standalone getStatus + Update PodCommsSessions.send to handle alternative noSeqStatus msg # handling + Rename type 7 PodInfoRequestSubType to noSeqStatus for better clarity
Various OmniBLEParser updates and improvements
Miscellaneous Omnipod code improvements & cleanup
Unacknowledged command handling fixes and PodCommsSession improvements
…or-handling Don't fail on ack comms error if validated response has been received
Use alternate type 7 getStatus call as per DASH PDM
New resolveUnacknowledgedCommand() attempts a GetStatusCommand to resolve any unacknowledged command or throws .unacknowledgedCommandPending on failure. Have all PodCommSession funcs call resolveUnacknowledgedCommand() instead of failing if called with a pending unacknowledged command.
… code Fix deactivatePod() logic error to correctly handle an unacknowledged command Simplify logging by not including the calling function's name
…recovery Improved unacknowledged command recovery
+ Fixes Trio issue #455, see for more details + Tweaks to runTemporaryBasalProgram() to avoid using stale podState + Change enactBolus() to use same updated pod suspended testing logic
Logic fix for 049 pod fault with concurrent temp basal commands
add self.tryToValidateComms to handle uncertain comms; ensure podSuspended is only return when actually suspended
Fix for pump manager returns bogus podSuspended
ps2
approved these changes
May 7, 2025
ps2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
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.
Purpose
Bring in improvements added to the dev branch since the tidepool-merge branch was first pushed to LoopKit
Test
Start with a local clone using the LoopWorkspace
tidepool-mergebranch:Confirmed that rPi DASH pod continues to operate as expected.
Code review
Confirm that the code difference between OmniBLE
devandtest_mergematches this PR exactly.Details
These are the CLI command used for local clone testing
Modify using merge
Confirm the changes with respect to dev match this PR, which was created against an earlier commit on the dev branch