Skip to content

Logic to capture reservoir changes with replaceComponent PumpEventType #89

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Logic to capture reservoir changes with replaceComponent PumpEventType #89

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions TidepoolServiceKit/Extensions/PersistedPumpEvent.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,11 @@ extension PersistedPumpEvent: IdentifiableDatum {
case .resume:
return dataForResume(for: userId, hostIdentifier: hostIdentifier, hostVersion: hostVersion)
case .rewind:
return dataForRewind(for: userId, hostIdentifier: hostIdentifier, hostVersion: hostVersion)
return dataForReservoirChange(for: userId, hostIdentifier: hostIdentifier, hostVersion: hostVersion)
case .suspend:
return dataForSuspend(for: userId, hostIdentifier: hostIdentifier, hostVersion: hostVersion)
case .replaceComponent(componentType: .reservoir):
return dataForReservoirChange(for: userId, hostIdentifier: hostIdentifier, hostVersion: hostVersion)
Comment on lines -57 to +61
Copy link
Contributor

@nhamming nhamming Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] semantically, rewind and reservoir change are not the same. Should the naming be updated to remove any confusion (maybe something like dataForReservoir or dataForInsulinReplacement... I'm not good at naming things)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually, we want the backend to absorb all of the replaceComponent pump event types (reservoir, pump, and infusionSet) added:
https://github.com/tidepool-org/LoopKit/pull/536/files

Currently we only have the TReservoirChangeDeviceEventDatum utilized by the .rewind case (not sure if this is accurate or a manipulation of the backend datum to accommodate a Medtronic rewind event) to capture a reservoir change event. We now have a new case replaceComponent(componentType: .reservoir) that needs to be accommodated but would essentially be the same function as rewind. I like your naming dataForReservoir as it tracks with the TReservoirChangeDeviceEventDatum type being used, should we go with this naming?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name is good as is, I believe; it should match the name of the model that it's creating on the back-end. We are mapping multiple pump event types to a single model on the backend, because Loop's model is a little more fine-grained.

default:
return []
}
Expand Down Expand Up @@ -137,7 +139,7 @@ extension PersistedPumpEvent: IdentifiableDatum {
return [datum]
}

private func dataForRewind(for userId: String, hostIdentifier: String, hostVersion: String) -> [TDatum] {
private func dataForReservoirChange(for userId: String, hostIdentifier: String, hostVersion: String) -> [TDatum] {
var data: [TDatum] = []
if dose?.type == .suspend {
data.append(contentsOf: dataForSuspend(for: userId, hostIdentifier: hostIdentifier, hostVersion: hostVersion))
Expand Down