Skip to content

Miscellaneous Omnipod code improvements & cleanup #135

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

Merged
merged 4 commits into from
Dec 10, 2024

Conversation

itsmojo
Copy link

@itsmojo itsmojo commented Nov 18, 2024

  • Remove unneeded & redundant didSend() & didReceive() message logging
  • Improved and additional PodState debugDescription display for insulin values
  • Improved pod suspend testing when updating delivery status
  • Update suspended, bolusing, tempBasalRunning, extendedBolusRunning definitions
    for better efficency and clarity using @dnzxy suggestion on OmniBLE PR Fix handling of comms error on resume & all pod DeliveryStatus cases #123
  • Use supportedTempBasalRates var for better OmniKit consistency

+ Remove unneeded & redundant didSend() & didReceive() message logging
+ Improved and additional PodState debugDescription display for insulin values
+ Improved pod suspend testing when updating delivery status
+ Update suspended, bolusing, tempBasalRunning, extendedBolusRunning definitions
  for better efficency and clarity using @dnzxy suggestion on OmniBLE PR LoopKit#123
+ Use supportedTempBasalRates var for better OmniKit consistency
@marionbarker
Copy link
Collaborator

Status

All tests successful. LGTM.

Code Review

Code reviewed completed. Only comment was sent via PM to change units to U/hr.
That was completed.

Test

Most of the changes were cosmetic (consistent comments for OmniBLE and OmniKit, log info instead of log error, preferred formatting of the suspended, bolusing, tempBasalRunning, and extendedBolusRunning public var in Pod.swift). No specific testing is required for these.

Test Issue Report Changes

To compare to Loop 3.4.4 with no modifications - use my personal report for comparison.
Apply this PR to Loop 3.4.4 and pair pod using an rPi simulator.

  1. Change to existing line (note my version used a real pod and gives an extra 0.35 U upon insertion - only review the format, not the amount):
    • before: * setupUnitsDelivered: Optional(3.2)
    • after: * setupUnitsDelivered: 3.10 U
  2. New lines added
    • before: nothing between activeAlertsSlots and messageTransportState
    • after: first lines below are inserted
    • use front end to modify reservoir level to 45 U; manually bolus 1 U so delivered will be different
    • after: second lines below are inserted:
* delivered: 1.50 U
* reservoirLevel: 50+ U
* delivered: 2.50 U
* reservoirLevel: 44.00 U

Test supportedTempBasalRates

Verify the manual temp basal rates are enacted as expected.
This change for OmniBLE only uses separate supportedTempBasalRates and supportedBasalRates required by OmniKit. For OmniBLE, those rates are the same, but having specific variable names is a step towards combining some portions of the OmniXXX code to simplify future work.

Set manual temp basal for several rates. Confirm the Hex command sent to rPi matches expected value.

  • 0.00 U/hr, success
  • 0.50 U/hr, success
  • 3.00 U/hr, success
  • modify max basal rate so can test 30.00 U/hr, success

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.

2 participants