Rework History Sheet

[BrianWieder]

• Reworks Glucose sheet based off Rework glucose dialog Artificial-Pancreas/iAPS#272
• Implements swipe to delete in both the treatment list, and the glucose history based off Implement swipe-to-delete for history treatments + Change alerts Artificial-Pancreas/iAPS#314
  • Only allow swipe to delete on Bolus, Carbs, and FPUs.
• Moves add buttons for insulin and glucose history to the top right of the sheet, and uses a drop icon for glucose instead of the unit
• Displays SMB as the type of treatment on the insulin history page instead of Bolus, with SMB next to the amount

[Sjoerd-Bo3]

I think that per guidelines, the “add” needs to be the same colour as the icon? And the fontsize looks a little off or something? Is that the standard?

Good work though Brian!

[Sjoerd-Bo3]

Or maybe we can make a plus within the droplet and + before the syringe icon (integrated into one icon)

[dnzxy]

To conform with guidelines, it‘d need to look like this; the addition of Add Glucose here is not guideline conform, but I personally think it’s an okay deviation because it adds clarity.

[Sjoerd-Bo3]

To conform with guidelines, it‘d need to look like this; the addition of Glucose here is not guideline conform, but I personally think it’s an okay deviation because it adds clarity.

So it also needs the plus? When we have already “add glucose”? Seems excessive, but if it says that. Lets indeed just go with the guidelines, makes it more coherent.

[dnzxy]

Technically the entire label is not conform and at the + is it, but I‘d rather go with something that comes close and looks, and adds clarity 😊

[Sjoerd-Bo3]

Technically the entire label is not conform and at the + is it, but I‘d rather go with something that comes close and looks, and adds clarity 😊

Also agree on that😂, indeed let’s get it like your proposal.

[bjornoleh]

Displays SMB as the type of treatment on the insulin history page instead of Bolus, with SMB next to the amount

Could we also get this differentiation displayed in NS? I think this was also introduced in iAPS around the same time as the other changes that are brought in here.

[bjornoleh]

Displays SMB as the type of treatment on the insulin history page instead of Bolus, with SMB next to the amount

Could we also get this differentiation displayed in NS? I think this was also introduced in iAPS around the same time as the other changes that are brought in here.

This was added to iAPS here:
Artificial-Pancreas/iAPS@24c5abb

And then apparently refactored for 3.0 here: https://github.com/Artificial-Pancreas/iAPS/blame/main/FreeAPS/Sources/APS/Storage/PumpHistoryStorage.swift#L214

[marionbarker]

Successful test.

The same patch files work for alpha as well as dev branch:

• I applied the patch from PR 140 to alpha, commit aa55271 (Merge pull request 120 from MikePlante1/disable-smb-schedule).
• I built successfully over an existing app on a test phone using NS as CGM and MDT 515 pump.
• I observed the history pages before and after (saved screen shots but will only post upon request.)

The previous requested changes have not been implemented, but the good news it same PR changes work for dev and alpha. Behavior is as previously reported.

[BrianWieder]

Updated the add buttons to match the proposal in #140 (comment) (though I personally think that "Log Insulin/Glucose" would be more accurate), and upload SMB treatment types to Nightscout. (Also rebased onto the updated dev branch)

[bjornoleh]

Thanks for adding the SMB/bolus distinction to NS entries!

though I personally think that "Log Insulin/Glucose" would be more accurate

I can agree that “Log” is better than “Add”, especially for bolus. The insulin is not added from or by the app, but simply logged after an external bolus. “Log” might be less ambiguous.

[dnzxy]

Great job @BrianWieder , thank you!

@bjornoleh relating to your comment: Glucose is added though, as it will be taken into consideration by the algorithm and also „overrides“ the current sensor value. Same for external insulin entries. You are adding to IOB by entering the entry; that thought (adding insulin not given by the pump but to be considered by the AID) is at the core of that logic by the way. Add is a pretty accurate description 😊

[dnzxy]

@BrianWieder would you also care to add the 2 tab vs. 3 tab option for history or is that too far outside the scope of this PR for your taste? Just asking because you have titled it "Rework History Sheet" 😊 Either is fine.

[BrianWieder]

@dnzxy I still think that log is a more straightforward term for insulin and glucose.

If I externally added insulin (for example via injection by pen), that insulin is already on board me, I am just logging it in Open-iAPS so the system can take it into account.

For glucose the same logic can apply, I am logging it in the app so the system can use it as a more recent datapoint over the CGM.

[BrianWieder]

@BrianWieder would you also care to add the 2 tab vs. 3 tab option for history or is that too far outside the scope of this PR for your taste? Just asking because you have titled it "Rework History Sheet" 😊 Either is fine.

You are referring to Artificial-Pancreas/iAPS#411?

If so, I can work on it, but it wouldn't be until early next week, as I am traveling this weekend.

[dnzxy]

Yeah I guess we can discuss this endlessly 😂 I consider "Add" the correct term because I am adding it to the data metrics for the AID to consider. Opposed to that, I wouldn’t accept "Dose" or "Bolus" or anything that would imply administering of insulin. We mean the same thing, we use different semantics with it 😊

For what it’s worth: these labels were discussed for a few days on the old discord and there were a bunch of people that went for "Add" while others didn’t need any label and others liked the simple unit + plus icon.

Comes down to personal preference I think 😊

Edit to add: Loop does not have any label with this and only uses the 100% iOS conform + icon. But in the context it uses this, you are within the insulin history, so there are not two actions in the same header nav item depending by tab like it is used for Open-iAPS.

[dnzxy]

You are referring to Artificial-Pancreas/iAPS#411?

If so, I can work on it, but it wouldn't be until early next week, as I am traveling this weekend.

Yes, essentially that 👍 Although you might be able to find a more elegant solution depending on how you go about it. Could totally also be done in a later PR, I think this one looks great and is feature-complete.

[bjornoleh]

I think it would be great to land this in its current form. I suppose it can be merged before we open up, as this is basically well tested code? I feel the 2/3 tab thing can wait a little longer :-)

[marionbarker]

Tested with the latest commits.

• Confirmed that I can add insulin and glucose.
• Confirmed that I can delete SMB, logged insulin and glucose.

First graphic is the treatments screen and add insulin screen.

Second graphic is the glucose screen and add glucose screen.

[marionbarker]

I'm seeing some odd behavior. Might be just me.
All my testing was done by applying a patch for this PR.
Tonight I did some testing where I commited this PR (along with others to a local branch).

Then every time I opened xcode to build, the PumpHistoryEvent.swift file would be modified.
Here's the diff I would see. I kept restoring the file, but thought I should report it.

% git diff
diff --git a/FreeAPS/Sources/Models/PumpHistoryEvent.swift b/FreeAPS/Sources/Models/PumpHistoryEvent.swift
index 0c606875..5aaed4dc 100644
--- a/FreeAPS/Sources/Models/PumpHistoryEvent.swift
+++ b/FreeAPS/Sources/Models/PumpHistoryEvent.swift
@@ -46,7 +46,7 @@ struct PumpHistoryEvent: JSON, Equatable {
 
 enum EventType: String, JSON {
     case bolus = "Bolus"
-    case SMB = "SMB"
+    case SMB
     case mealBolus = "Meal Bolus"
     case correctionBolus = "Correction Bolus"
     case snackBolus = "Snack Bolus"

[BrianWieder]

Weirdly I am seeing that as well. I thought that maybe it was just my laptop being weird, but now that I look at it again, I think it may be code formatting. My guess is that = "SMB" is redundant since the the enum case name being SMB implies the string value to be SMB. Will test and if that is correct will update the PR.

[BrianWieder]

Looks like removing the = "SMB" works as though it is there, so I removed it in order to prevent a future diff if XCode tries to format it. I also rebased again to the updated dev.

[BrianWieder]

@dnzxy After some more thought I updated the "Add Insulin" and "Add Glucose" buttons to "Log Insulin" and "Log Glucose". This matches the confirmation button on the next screen to log external insulin:

Also asked a non-technical/non-diabetic friend what their first instinct an "Add Insulin" button would do, and they thought it would take action and inject more insulin, and that log is more clear that it would not instruct the pump to bolus/inject more insulin.

What do you think?

[dnzxy]

I can still only point to the many discussions that happened months ago on the JBM iAPS server, those resulted in „Add“.

Ultimately, I don’t see this as a blocking issue, so I‘m okay with either 😊

[bjornoleh]

Tested the branch after merging dev into the branch (merges cleanly). Everything works as expected, including SMB distinction in both app and NS. Well done! I will approve this as is, just wanted to mention that we now have Log Glucose +, while the Title of the next screen is still Add Glucose. Maybe thats intentional, and I don't care too much either way :-)

[MikePlante1]

we now have Log Glucose +, while the Title of the next screen is still Add Glucose. Maybe thats intentional, and I don't care too much either way :-)

imo, these should match before merging.

I can still only point to the many discussions that happened months ago on the JBM iAPS server, those resulted in „Add“.

I looked for a bit but couldn’t find a suggestion to use “Log”. I do prefer “Log” myself, but “Add” is perfectly acceptable as well.

[BrianWieder]

just wanted to mention that we now have Log Glucose +, while the Title of the next screen is still Add Glucose

Good catch, updated all references to add glucose/insulin to log glucose/insulin in DataTableRootView as well.

[dnzxy]

I looked for a bit but couldn’t find a suggestion to use “Log”. I do prefer “Log” myself, but “Add” is perfectly acceptable as well.

That was my point, more or less. It was never once suggested by the great many discussion participants. Both are perfectly acceptable and Brian has gone with "Log", which is fine 😊

[marionbarker]

For reference, Loop uses “log”. Scroll down to the graphic in this section.

https://loopkit.github.io/loopdocs/loop-3/features/#non-pump-insulin

[MikePlante1]

Should the Log Insulin + view match the Log Glucose + view?

ie: Title lower and larger, and say Log External Insulin. And the confirm button changed to Save? Or the other view changed to Log glucose?

Current:

[BrianWieder]

Should the Log Insulin + view match the Log Glucose + view?

Good point, updated Log Insulin + view to match.

[MikePlante1]

Everything looks good to me now. I'm ready to merge this if you are, @BrianWieder and @dnzxy?

[BrianWieder]

SGTM, will let you merge whenever you are ready.