Skip to content
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.

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

Various MessageBlocks fixes & updates #21

Merged
merged 2 commits into from
Jan 26, 2024
Merged

Various MessageBlocks fixes & updates #21

merged 2 commits into from
Jan 26, 2024

Conversation

itsmojo
Copy link
Contributor

@itsmojo itsmojo commented Dec 25, 2023

  • Correct PodInfoActivationTime & PodInfoTriggeredAlerts defs
  • Rename PodInfoConfiguredAlerts to PodInfoTriggeredAlerts
  • Update PodInfoTriggeredAlerts to be only time based triggers
  • Update PodInfoPulseLog & PodInfoPulseLogPlus output
  • Have ConfigureAlertsCommand enforce the max alert duration value
  • Update TimeIntervalStr to only include minutes when non-zero
  • Update all PodInfoTests to compile, cleanly run & make sense
  • Sort files OmniKit/OmnipodCommon/MessageBlocks in Xcode

+ Correct PodInfoActivationTime & PodInfoTriggeredAlerts defs
+ Rename PodInfoConfiguredAlerts to PodInfoTriggeredAlerts
+ Update PodInfoTriggeredAlerts to be only time based triggers
+ Update PodInfoPulseLog & PodInfoPulseLogPlus output
+ Have ConfigureAlertsCommand enforce the max alert duration value
+ Update TimeIntervalStr to only include minutes when non-zero
+ Update all PodInfoTests to compile, cleanly run & make sense
+ Sort files OmniKit/OmnipodCommon/MessageBlocks in Xcode
@marionbarker
Copy link
Collaborator

marionbarker commented Jan 10, 2024

  1. Code Inspection - OK
  2. Test with Eros Pod - OK (this is with all open PR applied, 17, 20 and 21)
  3. Run Test cases - All pass (but first need to apply the iOS 17 updates from Loop PR 2080 and LoopKit PR 517)

Details:

Code Inspection:

Compare this PR to OmniBLE PR 109.

  • The diffs are identical (except for one minor public change - see note at end of comment) and the obvious Bluetooth changes not required for OmniKit

Test with Eros:

  • with the exception of pair/insert/deactivate, test every Omnipod command on Eros pod
  • set reservoir level reminder to 50 U and then reduce level below that - ensure alert appears as expected
    • got pod beeping at 50 U followed by modal alert on phone
  • plan to allow pod to reach at least 48 hours age so can see the expiration reminder alert, but will not hold up publishing this comment for that alert test

Regarding the change to public for twoDecimals inside DetailedStatus.swift:

  • for twoDecimals to be available for possible use in any pump manager specific UI code, the OmniKit twoDecimals needs to be declared public while the OmniBLE does not.

@ps2 ps2 merged commit f2fc341 into LoopKit:main Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants