-
Notifications
You must be signed in to change notification settings - Fork 514
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
Display Pump Status Highlight Message in home view #218
Conversation
Display the eventual Pump Statut Highlight Message in home view if state is warning or critical. fix partially issue nightscout#207
I suppose Statut is a typo and should be replaced with Status in English? |
I agree with @bjornoleh. The variable and function names should use Status: Start by testing what is there (or not there) in dev, commit 5e51344. Test without the patch with pods using an rPi DASH simulator.
Test after applying the patch with pods using an rPi DASH simulator.
These next things are new and a success:
Yet to do: Medtronic pod testing. So far so good. |
@marionbarker - I tried to analyse some test results with my rPi simulator :
Seems a correct behavior.
Seems a correct behavior. When reservoir = 0 ➡️ message "No insulin" and error alert.
Seems correct because a alert (Exceeded maximum Pod of Life...) is provided by pump when pod age > max ➡️ message "Pod expired". |
Change statut to status variable and function
Updated with the correction of statut ➡️ status |
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.
Just a minor change in a comment.
My prior test was not valid. I repeated the test for what happens near the time the pod reaches expiration.
Stages:
|
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.
I tested for Pods and approve this PR based on those tests.
For the second review, it should probably be tested with Medtronic.
GitHub/Xcode problems slowed me down a bunch, so I'll have to continue this tomorrow. From my initial test: |
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.
Using the pumpManager's pumpStatusHighlightMessage for the HUD is
corrects a long standing shortcoming in iAPS. The code LGTM, but I haven't tried it out myself. I can't particularly say that I particularly love the new look though, hopefully it'll grow on me... ;-)
Okay, I finished some more tests with my Medtronic 722. It shows The typical Medtronic status icons (battery and reservoir) are displayed at all other times, including when the pump claims 0U but actually still has insulin left, when the pump is out of range of the OrangeLink, and when the OrangeLink is out of range of the phone. |
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.
Tested with a Medtronic 722 and everything looked good.
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.
Looks good from testing on a phone.
Tested "No pod" with Dash, and Simulator suspend and occlusion.
Did not review code.
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 Tested with Omnipod Dash
Merging with two approvals (several when including those before the last commit with only a trivial change). |
Display the eventual Pump Statut Highlight Message in home view if state is warning or critical.
fix partially issue #207 with displaying "No pod" in the homeview.
Example in French :