Loop status upload#110
Conversation
| failureReason = nil | ||
| } | ||
|
|
||
| let loopName = NSBundle.mainBundle().bundleDisplayName |
There was a problem hiding this comment.
Can we move lines like this out of the LoopDataManager and into the receiver's method?
There was a problem hiding this comment.
I think ideally all the NightscoutUploadKit models would be created somewhere else, like DiagnosticLogger, or an extension of NightscoutUploader?
|
🎉👍👏😂😍 So glad you did this, @ps2. This will make integrating with Urchin so much easier - I've been running a much simpler/hackier version of these changes to show the prediction graph and eventual BG: |
Some suggested changes based on my comments
|
|
||
| if insulinOnBoard == nil { | ||
| dispatch_group_enter(updateGroup) | ||
| deviceDataManager.doseStore.insulinOnBoardAtDate(NSDate()) { (value, error) in |
There was a problem hiding this comment.
Is this the right timestamp to use?
There was a problem hiding this comment.
from a dosing perspective, yeah, you'd want it to be as close-as-possible to startDate on line 331, which it will be here, since these data retrievals should be pretty fast. If it needs to be exact, you could set startDate on line 126 and pass it in as an argument to self.updatePredictedGlucoseAndRecommendedBasal
|
|
||
| let loopStatus = LoopStatus(name: loopName, version: loopVersion, timestamp: statusTime, iob: iob, suggested: loopSuggested, enacted: loopEnacted, failureReason: lastLoopError) | ||
|
|
||
| deviceDataManager.remoteDataManager.uploadDeviceStatus(nil, loopStatus: loopStatus) |
There was a problem hiding this comment.
Maybe this implementation can move into here, now, to keep all the NS models together?
Also, would be good to punt at the top of this method if there's no nightscout uploader
…ting suggested attributes when last basal is successful
|
@mddub I've been using urchin, and love it. Forecasted bg would be amazing! I'm just about done with this, I think. |
|
Was trying to get COB and IOB NS attributes populated to wrap this up, but it looks like they might require more work, so I'll tackle that as a separate PR. I'll cut a RL release, and then point this to it, and the PR should be ready for review again. |
| - parameter resultsHandler: A closure called once the values have been retrieved. The closure takes the following arguments: | ||
| - predictedGlucose: The calculated timeline of predicted glucose values | ||
| - insulinEffect: The predicted effect of insulin | ||
| - carbEffect: The predicted effect of carbohydrates |
There was a problem hiding this comment.
still need to add these?
There was a problem hiding this comment.
I'm going to soon; I just need to figure out how to turn them into BG lines. carbEffect -> predicted BG if only carbs were considered, and insulinEffect -> predicted BG if only insulin were considered.
But I can remove them for this PR, if you'd like.
There was a problem hiding this comment.
Not sure I follow with the "only [x] were considered"?
I'm good with merging this PR if you are, which would mean reconciling this docstring and reverting the import line at the top. It sounds like there's still some munging of the carb/insulin effect lines for Nightscout, though ideally you can sort that processing out in the cloud. Just let me know what you'd like to do.
Uploads Loop status to Nightscout, with BG forecasts.