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

Handling Temp Basal events in NightscoutTreatments. #3597

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jan-rycko
Copy link

@jan-rycko jan-rycko commented Jul 27, 2024

This is a proposal of handling "Temp Basal" eventType entries from Nightscout.

I'm not Java-native, so NightscoutTreatment class might me the wrong place for it. Yet it's working fine as this class processes treatments anyway.

WIP - as we don't have NS profile yet. I plan to add support for it once I have the gist of how BasalProfile is working. For now there's a 0.15U base hardcoded.

I tested it on Nightscout follower flow. I have yet to test it when xDrip pushes data to Nightscout and loads treatments from it while AAPS loads profile.

Screenshot_20240728_014342_xDrip+

I believe this PR will fix:

WIP - as we don't have NS profile yet.
@jan-rycko jan-rycko changed the title [WIP] Handling Temp Basal events in NightscoutTreatments. Handling Temp Basal events in NightscoutTreatments. Jul 27, 2024
@wirbel-at-vdr-portal
Copy link

Could pls explain any of your abbreviations?
Those are difficult to understand and might be misunderstood.

Thank you.

  1. WIP - (work in progress, walking in a park, .. ?)
  2. NS profile (netscape communicator profile?, nightscout profile?)
  3. AAPS loads profile (American Association of Pharmaceutical Scientists: ... ?)
  4. "Temp Basal" eventType

@Navid200
Copy link
Collaborator

@wirbel-at-vdr-portal

If you believe that xDrip will be changed in a negative way if this submission is approved, please let's talk about it here.

Otherwise, if you have questions about what different acronyms mean, please open a discussion here: https://github.com/NightscoutFoundation/xDrip/discussions

The person who needs to understand those acronyms in order to review this submission, understands every one of them perfectly.

Thanks

@wirbel-at-vdr-portal
Copy link

Hi Navid200,

i think any change request to a software should be 100% clearly understandable without secondary discussions.
If someone would request such way in my repos, i would plain reject.

@jan-rycko
Copy link
Author

@wirbel-at-vdr-portal this PR (pull request) is marked as draft. It serves a purpose (possibly just in my head, each person can have their own standards) to state my intent of working on the issues mentioned above related to passing temporary basal from Nightscout to xDrip. When I finish I'll provide more detailed description of the resolution as well as possible issues.

Feel free to reject any pull requests. I'm also comfortable with this PR being rejected as this is not my codebase and I have little-to-no knowledge of standards that @Navid200 keeps. But I'm also open to comments and suggestions that might help me get it working and in a good enough shape to merge. In other case I'll just use fork for my own purposes when I have it fully functional.

WIP – work in progress, NS – nightscout, AAPS – Android APS, "Temp Basal" eventType – one of event types passed from nightscout /api/v1/treatments endpoint.

@Navid200 thanks for answering. I'll be updating this branch possibly in the near future.

@Navid200
Copy link
Collaborator

Feel free to reject any pull requests.

No, no one who has posted here so far can reject this pull request.
The only one who can reject it has not reviewed it yet.

Thanks for your submission.
I cannot promise that it will be approved because I am not the one who can. But, based on my experience over the past 4 years, all submissions are reviewed, and he is reasonable.

Not working yet – showing it in the view.
@wirbel-at-vdr-portal
Copy link

Hi jan-rycko,

Sorry for dropping a comment.
Comments are not welcome in this repo, not going to bother anymore.

@jan-rycko
Copy link
Author

jan-rycko commented Aug 5, 2024

Ok, so few updates from me.
I cleaned up the solution.

Main part is moved to separate class NightscoutBasalRate to handle temporary basal rate as well as profile to fill the gaps in graph. There should also not be any Refusing to create record older than current... errors caused by handling treatments that were already used before.

There are two new settings:

image

First one turns on loading of profile and uses treatments to plot TBR. Second one is to load data from Nightscout more frequently – it's up for discussion here if it's too much. I find it very helpful in some situations.

When the first setting is turned on, we also sync xDrip "active" Profile with Nightscout one:

image

I wrongly assumed that Basal Rate graph behaves similarly to Nightscout one and that it shows Basal Profile with dashed line. It's not – it always show straight line, which means 100% of profile. I suppose we can think of changing that in the future, but it's a case for another PR.

As such, I'll mark this PR as ready and wait for comments from maintainers of this repo.

In the near future I'll try to apply same functionality to NightscoutUploader, but let's try to review this one first.

@jan-rycko jan-rycko marked this pull request as ready for review August 5, 2024 18:48
@asavageiv
Copy link

Could the setting be "Nightscout connect frequency" with a default of "every 5 minutes" That way users aren't locked in to the developer choices.

@jan-rycko
Copy link
Author

Could the setting be "Nightscout connect frequency" with a default of "every 5 minutes" That way users aren't locked in to the developer choices.

Probably yes, but it should still be min 1 minutes. Anything shorter will be rate limited.

Anyway, still up for debate if such setting is actually ok. Possibly it will influence battery.

@jan-rycko
Copy link
Author

Since there is no much movement here, I'll add something that I wanted to try. I added graphical indicator of basal based on profile. Dashed line represents profile, loaded from Nightscout and basal graph is relative to it (or actually – properly scaled absolute value).

Screenshot_20240817_020200_xDrip+

Also – current time is represented, without lacking value at the end – graph is shown up to current moment.

Screenshot_20240817_021205_xDrip+

Code concerning plotting this data needs to be properly secured for edge cases (at least leaving former behavior as an option or testing is basing this chart on default profile is good enough).

Anyway, I started to work on NightscoutUploader – for it to also load profile along treatments and calculating APStatus, thus allowing to show Basal Rate on chart. NightscoutUploader code is in not-so-good shape, but let's hope for the best.

@rstutsman
Copy link

Any chance this will be merged? This is been on my wishlist for years now.

@jan-rycko
Copy link
Author

I'm waiting for the feedback / review from xDrip+ authors on this. As some time passed, I paused development on this feature, but I have several improvements on my mind and also I'd like to handle downloading profile/basal on Nightscout Uploader as well (it is nearly ready, but much harder to test thoroughly).

Anyway I'd like to merge this as well, soon. Then maybe merge Nightscout Uploader part in next MR if everything goes well. For now I use this build as my daily driver and it works pretty well.

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.

5 participants