-
Notifications
You must be signed in to change notification settings - Fork 29
Use correct units when parsing BolusWizard & CallBGForPH records #29
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
Conversation
This also fixes #26 |
Can you add a note here with what testing (beyond what is in #26) someone could do to validate the before and after behavior of this change? Thanks. |
For me, the most noticable effect was that meals/treatments weren't showing on the BG graph in NS. A more practical way to validate the issue is to set the pump to the "incorrect" unit based on the opposite of what is found in "decocare/models/__init__.py". You then bolus using the bolus wizard and enter the BG (ex: 6.2 mmol/L). The treatment should show up in the "Treatments" report and will look like this (incorrect BG in bold):
Repeating the same process with my changes should yield the following:
Thanks! |
@SebastienLussier as written in ITB I'll test the PR in about 1 week once return back from my vacation, thanks for your investigation so far! |
To make this easier to test, I just pushed a SebastienLussier-fixes branch (https://github.com/openaps/decocare/tree/SebastienLussier-fixes), so you can just clone decocare, |
@scottleibrand thanks for pushing to a separate branch |
@SebastienLussier @scottleibrand the fix is working and solves the issue, thanks very much!!! |
Also checked this on an European Veo 754 with mg/dl - putting in the BG together with carbs (BolusExpert). |
@SebastienLussier: It works for me for Event Type @Missdazzle : a European Veo with mg/dl? You sure you don't mean mmol/l? |
@PieterGit Oh yes, I´m sure! :) |
So should I merge this to dev for further testing there? Or is there anything we need to fix first? |
For me it's okay, what's about the 6.4U fix? As I understood it is not merged to dev yet... |
The 6.4U/h fix is in decocare 0.0.31 (and in both the dev and master branches), which is available via |
@PieterGit 👍 or 👎 for merge to dev? |
@scottleibrand : please merge to dev. The fix fixes at least the |
Sorry to pop up so late about this, but using the same rig with openaps 0.6.0 dev4, the bolus wizard events are detected and reported correctly using a 754WW pump, but display to problem described above when I use a 715WW or 722WW pump, all in mmol/l. Do I understand correctly that it should have been fixed long ago ? My right was built last may and many times updated. Shouldn't the decocare part be up to date as well ? |
@lsandini Current oref0 0.6.0 release installs decocare-0.0.31-py2.7.egg and then a branch checkout of I noticed this patch is on the dev branch, see also https://github.com/openaps/decocare/network Can you test on a test rig if using the That is, it would of course be better to create, test and release a fresh decocare release with all relevant fixes included. @ecc1 and @scottleibrand and @bewest : thoughts? |
It took me some time, sorry. I had to retrieve my 722WW first. Unfortunately the problem persists with 0.6.0 master. I didn't try the 715WW yet, but it is probably the same. |
As discussed in issue #8, the processing of these record was previously hardcoded to a specific type of unit depending on the model. For the context, we own both a 722 & 522 and we are using mmol/L but these two models were hardcoded to mg/dl.
Tested on my 722, on which I issued consecutive boluses using the wizard, switching the unit in between.
"openaps use pump iter_pump" correctly displayed the different records in the unit in which they were entered.