Dev 1.1.1 x22 battery status #259
Conversation
- Added Battery Chemistry Selection in SettingsTableViewController - Add User Var to store the Battery Chemistry Setting - Added defaults and Enum for Battery Chemistry Type
- Added Battery Calculation - Hide Setting for Battery Type on MySentry Pumps
- Updated Switch Descriptions and Alkaline syntax
ps2
left a comment
There was a problem hiding this comment.
Thanks for the update Jeremy. Looks really good. You can just update the branch, and not submit a new PR.
| if (self.batteryChemistry == .lithium){ | ||
| minVoltage = 1.32 | ||
| maxVoltage = 1.58 | ||
| batteryNotification = 0.12 |
There was a problem hiding this comment.
The point at which we display 0% battery should be equal the point at which a notification should be made, no?
There was a problem hiding this comment.
The intent of this Notification was to "warn" a user of an impending battery failure or intermittent communications failures due to low battery. This is a 8 to 12 hour warning prior to battery = 0.
| /// | ||
| /// - parameter currVoltage: Current Voltage Reading from Pump | ||
| /// | ||
| public var x22BatteryPercentRemaining : Double = -1 |
There was a problem hiding this comment.
Since we already have a battery % for x23 devices, let's track battery % as a single ivar.
Updating that ivar can trigger both battery change event (AnalyticsManager.sharedManager.pumpBatteryWasReplaced()) and notification (NotificationManager.sendPumpBatteryLowNotification()), and then the StatusTableViewController can check one place for battery percent, instead of checking both dataManager.latestPumpStatusFromMySentry.batteryRemainingPercent and dataManager.x22BatteryPercentRemaining
There was a problem hiding this comment.
Don't use magic number sentinels; use an optional.
Since percentage is derived from chemistry, and chemistry can be changed at any time, this shouldn't be stored at all.
There was a problem hiding this comment.
As I understand it you are saying I should write the Battery % Remaining to dataManager.latestPumpStatusFromMySentry.batteryRemainingPercent. I totally agree reuse of the already existing structure would be great, however I am unsure how to write to the batteryRemainingPercent var.
latestPumpStatusFromMySentry?.batteryRemainingPercent = ((currVoltage - minVoltage)/(maxVoltage - minVoltage))
I receive an error
Cannot assign to property: 'batteryRemainingPercent' is a 'let' constant
| @@ -0,0 +1,86 @@ | |||
| // | |||
| // BatteryTypeSelectionTableViewController.swift | |||
| // Loop | |||
There was a problem hiding this comment.
Was it not possible to re-use RadioSelectionTableViewController?
There was a problem hiding this comment.
I am sure it may be. I am just not swift enough to fully comprehend how. So I leveraged the existing pattern Nate had provided.
|
Closed for #275 |
* LOOP-1938: Acknowledge all given an identifier I thought it would be sufficient to only acknowledge the latest Alert, but LOOP-1938 points out that if an alert repeats without (prior issued Alerts) getting acknowledged, those prior Alerts get "stuck" and will replay on restart until they all are acknowledged. Becky convinced me that, conceptually, if you get multiple issues of the _same_ alert, recording an acknowledgement date on all of them is appropriate. * checkpoint * Unit tests & refactoring
New PR for #141 Updated to now allow for discrete selection of Alkaline and LIthium battery chemistry. View Controller and Enum added to allow for named selection following pattern of preferredInsulinDataSource.