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

loopalyzer enhanced #4235

Closed
wants to merge 29 commits into from

Conversation

PieterGit
Copy link
Contributor

@PieterGit PieterGit commented Jan 26, 2019

Extends #4215 of @lixgbg and fixes some problems of that PR.

Differences/fixes:

  • Fixed profile loading. Loads my profiles and does not get stuck in Rendering...
  • Shows Profilenames
  • changed Show profiles table to unchecked by default
  • Fixed problem that it doesn't show the latest profile name if the last two profiles had the same basal, csf, isf

Fixes (not related to the original PR):

  • drop Unable to find element for #chartContainer warning, because it's bugging the console log with the reports view and is a false positive.
  • npm update, upgrade moment, mongodb, swagger-ui-dist, uglify-js, supertest, webpack, webpack-cli

Todo:

  • set laDebug = false just before merge

@PieterGit PieterGit mentioned this pull request Jan 26, 2019
@PieterGit PieterGit requested a review from sulkaharo January 26, 2019 17:16
@PieterGit
Copy link
Contributor Author

PieterGit commented Jan 26, 2019

@jpcunningh or @sulkaharo Can you also review these changes. I think this is ready to merge to dev. I would like to get your opinion on this PR and if it's ok to merge it to dev (and thus release it with Nightscout 0.11)

I need to re-apply the merge. I shouldn't do development with Git/Heroku but thought I could fix it in a couple of commits. I was wrong 😒

…then the previous profile was shown. see if this fixes the problem
@nightscout nightscout deleted a comment Jan 26, 2019
@PieterGit
Copy link
Contributor Author

@lixgbg Can you explain a bit more of what timeshifting does? It's not clear in the UI that it's only shown if you select multiple days. If you have only a single day it won't show the checkbox Timeshift on meals larger than X g carbs consumed between Y and Z.

I think this is ready for merging to dev.

@PieterGit PieterGit added this to the 0.11.0 milestone Jan 26, 2019
@lixgbg
Copy link
Contributor

lixgbg commented Jan 27, 2019

@PieterGit Sorry for not being responsive, I've been traveling and in job workshops for two weeks. I have been working on improving the tolerance to bad profiles, such as yours, and also to improve the deduplication. So it seems we've been working in parallel.

I have not had a chance to look at your/this PR and verify that it works as I intended. I'm not sure about the fix to the deduplication you made. The purpose is to show a profile only when it has distinguishing changes to basal, cr, isf, but not startDate (Loop e.g. often creates copies of your profile with just a new startDate). Did your update fix any bugs there?

I'll test your version with my profiles and see if the deduplication works as expected, asap.

Regarding Timeshift -> If you have breakfast at say 7am one day and at 8am another day. Then when showing Loopalyzer over these two days you can enable Timeshift from e.g. 06:00. It will then find the first carbs logged after 06:00 on each day and when they were logged. Then it will calculate the "average time" you had these carbs, so 07:30am in this example. It will then timeshift, move, everything (carbs, bolus, basal, BG) etc the first day by +30 mins and the second day by -30 mins, so it looks like you had the breakfast at the exact same time both days. This removes the smearing of BG response when you average multiple days. It then displays this period after the "new breakfast time" with gray background so you can see more clearly. Not that ALL values for the days are timeshifted, so also values for e.g. evening snack are shifted +30min/-30min in this example.

@PieterGit
Copy link
Contributor Author

PieterGit commented Jan 27, 2019

@lixgbg I made it possible to show the name of the profile. That is what the users needs for looking up the profiles (not the startDate of the profile). As far as I can tell the deduplication still works as expected. Please check my changes, and see if you need to add changes to your PR (I don't mind which PR we use to merge).

We should do more explanation of the timeshift function, and it should be visible regardless of whether the user has selected one day or more. It's not a good UI habit to hide/show UI elements. Perhaps it's better to use a greyed out, to show that the UI elemement is not enabled for a single day.

I don't believe my profiles are bad, but I think the profile UI is confusing and it's possible to create multiple profiles for the same period (in several profile records). We should add a check function for that in /admin for Nightscout 0.12. In my case I think those profiles were unchanged, so only duplicated. We should make sure that the loopalyzer works for that with Nightscout 0.11.

@lixgbg
Copy link
Contributor

lixgbg commented Jan 28, 2019

Hi, thanks for your work. However I don't agree on the profile name. I'm a Loop user and Loop sets profile name = 'Default' to ALL profiles it creates from within the Loop app. So in my NS all profiles have the same name, hence showing it doesn't add much for Loop users. But I've kept it as your version, so displaying both name and date.

Fixed problem that it doesn't show the latest profile name if the last two profiles had the same basal, csf, isf
This is exactly what the deduplication is supposed to do. If you have multiple profiles over the days you are showing you only want to display the oldest of the ones that have identical basal, carbratio, isf, but when a profile that has a different setting appears it should be displayed.

So if (only isf changing, for brevity):
startDate = monday, isf = 1.0
startDate = tuesday, isf = 1.0
startDate = wednesday, isf = 2.0
startDate = thursday, isf = 2.0
startDate = friday, isf = 2.0
startDate = saturday, isf = 1.0
startDate = sunday, isf = 1.0

then you want to show Monday, Wednesday and Saturday. These are when there is a change to the profile which distinguishes it from the previous one. So deduplication should remove the others.

I have pulled your version and working on debugging the deduplication. Will fix and push when I have a working version.

/Henrik

@PieterGit
Copy link
Contributor Author

@lixgbg pulled the changes on this PR to #4215
Closing this PR and we will work together on finishing #4215

@PieterGit PieterGit closed this Feb 2, 2019
@sulkaharo sulkaharo mentioned this pull request Feb 7, 2019
sulkaharo added a commit that referenced this pull request Feb 7, 2019
Draft release notes for upcoming 0.11 release (currently release candidate phase).

# Changes

Over 360 commits, 89 files changed, +8,428 / −6,569 lines of changes (full list of changes here: 
https://github.com/nightscout/cgm-remote-monitor/pull/4022/commits )

## New features
- Fully secure by default out of the box.  Unsecure access via http is not allowed anymore by default. This might force you to re-authenticate with your `API_SECRET` or token if you were using unsecure access. (@PieterGit )
- No outdated packages with vulnerabilities are being used anymore (@PieterGit ) 
- Add Week to Week report (@jpcunningh, #4123 ) 
- Add Loopalyzer report to analyse looping. Visualize your loop (@lixgbg, #3629 #4235 )
- Add predictions support to Day to Day report (@lixgbg, #3179 )
- Add cgm sensor stop to Careportal (@jpcunningh, #4060)

## Removed features
- remove `mqtt` module, because it had a security issue and was not used
- remove `sgvdata`  module, because it had a security issue, added a lot of complexity and wasn't needed (@PieterGit ). Replacement implementation for CSV and TSV export (@sulkaharo ).

## Improvements
- Fix MongoDB database insert handling. Log error on inserts and don't crash in case the MongoDB disk is full or MongoDB quota is reached (@sulkaharo and @jpcunningh)
- Upgrade packages to recent version, fixing all known security issues with dependencies (@PieterGit)
- Redirect redirect HTTP to HTTPS and implement HSTS (@jweismann, @PieterGit, #4044 and #4010	and #4253 )
- Technical improvement: Migrate from `uglify-js` to `terser-webpack-plugin` (@PieterGit)
- Streamlined Heroku deployment template with more descriptive text and more appropriate defaults for new users (@unsoluble, #4116 )

## Bug fixes
- Fix CGM voltageb battery warning level to match xDrip+ (@jpcunningh, #3954 )
- Fix daylight saving and reloading bug in profile editor, (@DigitalDan1, @Kywalh #4029 and #4074 )
- Reduce the amount of Profile Switch treatments being loaded to fix UI slowdown and Nightscout home screen losing AAPS data from >3 hours ago, (@sulkaharo, @vickster1, #4055 )
- Upgrade to [share2nightscout 0.2.0](https://github.com/nightscout/share2nightscout-bridge/releases/tag/0.2.0). Prevent Nightscout server crashes in case Dexcom server does not respond (@PieterGit, @veryfancy)
- Fix UI so pills are updated immediately after new data is loaded (@sulkaharo)
- Fixes to If-Modified-Since HTTP header handling for BG data (@sulkaharo)

## Documentation and language updates
- Language updates for Danish, Dutch, German, Hebrew, Norwegian, Russian 
- New languages: Japanese, Turkish
- Update Alexa documentation. Note that some Alexa improvements are postponed to Nightscout 0.12 because the Alexa plugin needs refactoring, see #4168 (comment)
- Update IFTTT maker-setup.md docs (@Dave9111, @unsoluble, #4206 )
- Updated various docs, including [CONTRIBUTING](https://github.com/nightscout/cgm-remote-monitor/blob/dev/CONTRIBUTING.md) documentation

# Upgrade notes
- We only allow Nightscout to start with a secure Node JS. 
  - Latest Node 8 LTS (8.15.0 or later) and Latest Node 10 LTS (10.15.1 or later) are recommended and supported. 
  - Latest Node version on Azure (currently 10.14.0) is tolerated, but not recommended
  - Other versions will not start 
- The [rawbg](https://github.com/nightscout/cgm-remote-monitor#rawbg-raw-bg) settings are converted to a single setting tri-state variable.
- We improved security and added several new environment variables such as [INSECURE_USE_HTTP and SECURE_HSTS_HEADER](https://github.com/nightscout/cgm-remote-monitor/#predefined-values-for-your-server-settings-optional)
  - Your site redirects to https by default. If you don't want that or use a Nginx or Apache proxy, set `INSECURE_USE_HTTP` to `true`.
  - We enabled [HTTP Strict Transport Security (HSTS)](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) headers by default, settings `SECURE_HSTS_HEADER` and `SECURE_HSTS_HEADER_*`

## Upgrade notes for Azure users

We recommend Azure users consider migrating their hosting to Heroku, as we've observed Heroku users have significantly less issues with having their sites work reliably. If you want to continue using Azure, change the following configuration variables in Azure before updating to the latest Nightscout version:
```
WEBSITE_NODE_DEFAULT_VERSION=10.14.1
SCM_COMMAND_IDLE_TIMEOUT=300
```

# Install instructions

Install instructions can be found: https://github.com/nightscout/cgm-remote-monitor/blob/master/README.md#install

# Contributors to this release

The release coordination for this release was done by @PieterGit 
We would like to thank the following people for their contribution (in alphabetical order):
@anderser, @apanasef, @balshor, @bewest, @blocklist_twitter, @CaroGo, @cascer1, @cluckj, @danamlewis, @Dave9111, @diabetlum, @herzogmedia, @janrpn, @jasoncalabrese, @jpcunningh, @jweismann, @kenstack, @Kywalh, @lixgbg, @LuminaryXion, @MilosKozak, @mitrei, @PaperT1D, @PieterGit, @unsoluble, @rarneson, @renegadeandy, @scottleibrand, @sulkaharo, @T-o-b-i-a-s, @tynbendad, @unsoluble, @veryfancy, @viq, @wootmasterslick

(if I forgot somebody, please respond)

# TODO

TODO: Translations, Languages with less than 80% will be removed in a future Nightscout version. Currently the following languages are at risk: 
中文(繁體) (zh_tw), Hrvatski (hr), Ελληνικά (el), 한국어 (ko)
See https://gitter.im/nightscout/public?at=5bef2f34de42d46bba766f66

TODO: Fix Codacy errors, https://app.codacy.com/app/Nightscout/cgm-remote-monitor/issues?bid=2452379&filters=W3siaWQiOiJDYXRlZ29yeSIsInZhbHVlcyI6WyJFcnJvciBQcm9uZSJdfV0=

TODO: test dev after all new features are merged for at least two weeks
@PieterGit PieterGit deleted the 201901_loopalyzer-enhanced branch September 3, 2020 18:12
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.

2 participants