Skip to content

Conversation

bastiaanv
Copy link
Contributor

@bastiaanv bastiaanv commented Apr 21, 2025

After quite some development time and several fixes, I believe DanaKit is ready to merge to Loop!

ping @marionbarker

@marionbarker
Copy link
Contributor

I will test this again @bastiaanv
My recommendation is that we wait until after Loop 3.6.0 is released and then discuss adding this to dev for evaluation.

@marionbarker
Copy link
Contributor

Successful test on 26 April 2025:

  • I started with LoopWorkspace main branch, which is currently at the same level as the dev branch
  • I confirmed I was using DanaKit SHA: f870821
  • I tested using the DanaKit simulator

Comment:

  • This same repository is used by testers of the Trio app (they point to loopandlearn/DanaKit dev branch directly)
  • A "copied" version of this repository is incorporated into the iAPS code base and provided to users of the iAPS app
  • @bastiaanv has been accepting input from all sources to improve his repository

@marionbarker marionbarker requested a review from ps2 April 27, 2025 17:09
Copy link
Contributor

@marionbarker marionbarker left a comment

Choose a reason for hiding this comment

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

Approve from code review and test.

@marionbarker marionbarker changed the base branch from dev to feat/dana July 23, 2025 17:16
@marionbarker
Copy link
Contributor

Plan is to merge into feat/dana (at same level as dev) for final testing before doing a final merge to dev next.

@marionbarker marionbarker merged commit 97d02f6 into LoopKit:feat/dana Jul 23, 2025
@marionbarker
Copy link
Contributor

@bastiaanv I am testing in feat/dana branch. I was not expecting to run into problems.

I am getting a threading violation:

  1. Tried to connect to dana-simulator

  2. I could not get past this to even delete the Dana pump so I deleted the Loop app from the test phone. I did a fresh build and then tried to add Dana again, taking it more slowly this time.

    • first attempt was Dana-i (2 copies of the ID showed up under the scan, tapped and nothing happened, tapped back)
    • second attempt was DanaRS-v1 (got message that it is not supported)
    • third attempt was DanaRS-v3 - only one copy of ID (from simulator) showed up, tapped and it connected
      • this message is seen in the Xcode log: DanaKitPumpManager.swift - notifyStateDidChange()#1638: State update could not be reported -> Missing delegate
      • tapped finish
      • got hang in same section of code as before:
+[UIView setAnimationsEnabled:] being called from a background thread. Performing any operation from a background thread on UIView or a subclass is not supported and may result in unexpected and insidious behavior. [redacted]

Unsupported enumeration of UIWindowScene windows on non-main thread.

failure in assertBarrierOnQueue of <FBSMainRunLoopSerialQueue:0x107035aa0> (FBSSerialQueue.m:264) : threading violation: expected the main thread
  1. After that hang, I rebuilt the code, it continued and shows the Dana-i (BLE5) on the pump manager screen.

    • I was able to bolus and run a few pump interface commands and it was working in closed loop
  2. I then attempted to delete the Dana pump from Loop and got the hang in the same place as before.

  3. This time, after the rebuild, the delete pump had gone through and I had the Add Pump icon on HUD.

Loop-hang-danakit-pr247

I was able to repeat this several times with the same results. I'm not sure if being shown the Dana-i (BLE5) is attached screen following addition of DanaRS-v3 is expected when using a simulator.

@marionbarker
Copy link
Contributor

Summary

I believe the problem is in DanaKit in commit dcc4495

Trouble Shooting:

Last time I tested integration of DanaKit to LoopWorkspace was with DanaKit commit f870821 on April 21, 2025

Problem Summary

The current failure is:

  • When adding a Dana pump to Loop

    • the app crashes when you tap Finish after adding the pump
    • the app can be restarted following the crash and the pump appears to work as expected
  • When deleting a Dana pump from Loop

    • the app crashes when you tap Delete Pump after requesting delete pump
    • the app can be restarted following the crash and the pump was successfully deleted and a different pump can be added

The crash for both adding and deleting the pump is on line 2057 of LoopWorkspace/Loop/Loop/View Controllers/StatusTableViewController.swift

Identify "problem" commit

review commit list

Look at recent commits for DanaKit (full list at bottom of this comment):

I was suspicious that this commit may have caused an issue with the LoopWorkspace implementation.

test with 41b592e

I modified my local clone of LoopWorkspace: feat/dana branch to point to the commit in DanaKit before the one I suspected, i.e., commit 41b592e.

With this commit, the Dana pump is able to be added and deleted from LoopWorkspace without crashing the app and appears to work nominally (using an rPi dana simulator).

test with dcc4495

With this commit, dcc4495, the problem of crashing during addition or removal of the Dana pump returns.

Trio results

Because Trio uses the same repository as LoopWorkspace, I repeated this test with Trio 0.5.1.10. The notes for this are found in Trio PR 720: chore: update DanaKit.

Those tests indicate this commit, dcc4495, also causes an issue for Trio.

complete DanaKit commit list since last successful LoopWorkspace

This is the complete commit list since the successful test in April 2025.

59f4853 (HEAD -> dev, origin/l10n_dev, origin/dev, origin/HEAD) New translations localizable.strings (Polish) (#13)
1ea5e38 chore: linter
dcc4495 Extra pump events & pump delete (#11)
41b592e chore: fix warning
ca240f9 fix: add extra logs for detecting missing PumpDelegate
70610c4 New Crowdin updates (#9)
96c6e3e i18n: update translation
5f14486 New Crowdin updates (#8)
0d54871 style: add DanaRS v1 warning
ee9ebdd chore: linter
fbf1fad style: seperate stop temp basal button from suspend/resume button
49896ef fix: Prevent double Resume & Suspend event recording
3c0336b chore: linting
d73b369 fix: rework async DanaKit
89062b0 fix
8bf3b3a fix: rename log filenames
f870821 fix: update bolus state even with forced disconnect

@marionbarker
Copy link
Contributor

I tested again with the fix to DanaKit dev branch from 9 August 2025.

This was successful.

I'll create a new PR using the updated version of LoopKit/LoopWorkspace feat/dana

@marionbarker
Copy link
Contributor

marionbarker commented Aug 9, 2025

oops - I should have completed the test.

The build of Loop using updated dev branch for DanaKit succeeded when adding the Dana pump, but failed when disconnecting the Dana pump.

  • [y] Able to add Dana pump
  • [y] Able to sync to pump
  • [y] Able to achieve a green loop with Loop
  • [failed] Able to remove Dana pump and add a different pump (simulator pump)
  • [] Able to remove different pump and add Dana pump successfully

Crashes at same place as was reported before.
A restart (rebuild) of the Loop app shows the pump was removed.

@marionbarker
Copy link
Contributor

Following merge of PR #308, the LoopKit feat/dana branch works for iOS 18 phones and DanaKit pumps.

  • [y] Able to add Dana pump
  • [y] Able to sync to pump
  • [y] Able to achieve a green loop with Loop
  • [y] Able to remove Dana pump and add a different pump (simulator pump)
  • [y] Able to remove different pump and add Dana pump successfully

However, with the minimum deployment for DanaKitPlug (currently set to 18.1), Loop builds on iOS 15 and 16 phones, but the Dana plugin does not work.

Investigating with @bastiaanv whether the minimum deployment for DanaKitPlugin can be changed to 15.6 without affecting the ability of this repository to also support Trio.

In tests with the simulator, this modification still worked and enabled Loop to talk to the Dana simulator for iOS 15.8.4 and iOS 16.7.11 devices.

vanzaam pushed a commit to vanzaam/LoopWorkspace that referenced this pull request Aug 21, 2025
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