search

nightscout / Trio

MIT License
64 stars 259 forks source link

Merge alpha into dev #117

Closed bjornoleh closed 3 months ago

bjornoleh commented 4 months ago

List of PRs listed in this as of 16:40 UTC 2024-04-16

avouspierre commented 4 months ago

Tests done with success :

MikePlante1 commented 4 months ago

I don't think Tidepool integration should be added to dev until its basal reporting is at least closer to what's reflected in AH/NS

I don't think oiref should be added to dev until the disable SMB scheduling feature is fixed, as alpha's implementation is currently more broken than dev&main. More details in https://github.com/nightscout/open-iaps-oref/issues/18 , https://github.com/nightscout/open-iaps-oref/pull/19 , https://github.com/nightscout/open-iaps-oref/pull/20

bjornoleh commented 4 months ago

I don't think Tidepool integration should be added to dev until its basal reporting is at least closer to what's reflected in AH/NS

Regarding Tidepool, it seems we have a proposed fix in #118 . I think we can keep Tidepool in dev, and hopefully get it to work properly soon. If not, I suggest we make it unavailable from the settings before release, then fix later.

marionbarker commented 4 months ago

Tidepool fix (PR #118) confirmed to work. Top half of graphic from this morning before the fix. Line 217 of FreeAPS/Sources/Services/Network/TidepoolManager.swift used to be .unitsPerHour

Bottom half of graphic after modifying that line to units. Note the scale for basal is now 2 U/hr as opposed to 1 U/hr this morning. The value for daily totals at the time graphic was acquired stated 17.35 U on MDT 515 pump and 17.3 on Tidepool graphic. (Note that Tidepool only reports one decimal place.)

tidepool-fix

bjornoleh commented 4 months ago

I don't think oiref should be added to dev until the disable SMB scheduling feature is fixed, as alpha's implementation is currently more broken than dev&main. More details in nightscout/open-iaps-oref#18 , nightscout/open-iaps-oref#19 , nightscout/open-iaps-oref#20

Perhaps we can keep it as it is, and make sure everything works before release? I trust this can be fixed soon too.

marionbarker commented 4 months ago

These graphics were captured before the Tidepool fix. Just posting graphics and insulin totals wrt Nightscout. This is a desktop test.

Configuration:

NS-alpha-test-dashboard

NS-alpha-test-day-to-day

MikePlante1 commented 4 months ago

I don't think oiref should be added to dev until the disable SMB scheduling feature is fixed, as alpha's implementation is currently more broken than dev&main. More details in nightscout/open-iaps-oref#18 , nightscout/open-iaps-oref#19 , nightscout/open-iaps-oref#20

Perhaps we can keep it as it is, and make sure everything works before release? I trust this can be fixed soon too.

I think this can and should be fixed before merging into dev. Was planning on doing it tomorrow, but I'll see if I can squeeze it in tonight.

MikePlante1 commented 4 months ago

I don't think oiref should be added to dev until the disable SMB scheduling feature is fixed, as alpha's implementation is currently more broken than dev&main. More details in nightscout/open-iaps-oref#18 , nightscout/open-iaps-oref#19 , nightscout/open-iaps-oref#20

Perhaps we can keep it as it is, and make sure everything works before release? I trust this can be fixed soon too.

I think this can and should be fixed before merging into dev. Was planning on doing it tomorrow, but I'll see if I can squeeze it in tonight.

Fix PR'd in https://github.com/nightscout/Open-iAPS/pull/120 and https://github.com/nightscout/open-iaps-oref/pull/20

marionbarker commented 3 months ago

2024-04-29 Successful Test:

bjornoleh commented 3 months ago

I don't think oiref should be added to dev until the disable SMB scheduling feature is fixed, as alpha's implementation is currently more broken than dev&main. More details in nightscout/open-iaps-oref#18 , nightscout/open-iaps-oref#19 , nightscout/open-iaps-oref#20

Perhaps we can keep it as it is, and make sure everything works before release? I trust this can be fixed soon too.

I think this can and should be fixed before merging into dev. Was planning on doing it tomorrow, but I'll see if I can squeeze it in tonight.

Fix PR'd in #120 and nightscout/open-iaps-oref#20

PR #120 is merged now. Does that mean alpha can be merged into dev now?

MikePlante1 commented 3 months ago

PR #120 is merged now. Does that mean alpha can be merged into dev now?

Yes, I think so.

bjornoleh commented 3 months ago

Ok, would be great to have this merged today. Could we get a couple of more reviews in?