ivalkou / freeaps

FreeAPS X - an artificial pancreas system for iOS based on OpenAPS
MIT License
162 stars 141 forks source link

Fixed display of nested temporary targets #173

Closed DobbyWanKenoby closed 2 years ago

DobbyWanKenoby commented 2 years ago

The project has bug with display temporary targets (TT), which overlapping each other. If one TT start on time N, finish on time M, and other TT place in range (N,M), where it start > N, and finish < M, then second part of first TT doesn't display.

Fix improve bug. You can use this temptargets.json for tests (pls change dates)

[
  {
    "_id" : "6286F30D-0E83-4688-9E18-62DB1DD0EE13",
    "name" : "Temp target",
    "reason" : "Temp target",
    "created_at" : "2022-01-12T08:05:00.000Z",
    "targetTop" : 117.117117117117095508481454427404360236,
    "duration" : 30,
    "enteredBy" : "freeaps-x",
    "targetBottom" : 117.117117117117095508481454427404360236
  },
  {
    "_id" : "5F198783-3158-4C53-B609-E527DA7B6859",
    "name" : "๐Ÿ˜Œ Normal",
    "reason" : "๐Ÿ˜Œ Normal",
    "created_at" : "2022-01-12T06:25:45.276Z",
    "targetTop" : 99.099099099099080814868922977034458662,
    "duration" : 120,
    "enteredBy" : "freeaps-x",
    "targetBottom" : 90.099099099099080814868922977034458662
  },
  {
    "_id" : "A652AD85-F433-4F31-BEE9-255CCB75286A",
    "name" : "Temp target",
    "reason" : "Temp target",
    "created_at" : "2022-01-12T05:30:00.000Z",
    "targetTop" : 144.144144144144117548900251602959212599,
    "duration" : 320,
    "enteredBy" : "freeaps-x",
    "targetBottom" : 108.108108108108088161675188702219409449
  },
  {
    "_id" : "B8E90922-069D-4DF3-BF52-A8E5DD8B5C5C",
    "name" : "Temp target",
    "reason" : "Temp target",
    "created_at" : "2022-01-12T05:02:00.000Z",
    "targetTop" : 90.090090090090073468062657251849507874,
    "duration" : 15,
    "enteredBy" : "freeaps-x",
    "targetBottom" : 72.072072072072058774450125801479606299
  },
  {
    "_id" : "5F198783-3158-4C53-B609-E527DA7B6859",
    "name" : "๐Ÿ˜Œ Normal",
    "reason" : "๐Ÿ˜Œ Normal",
    "created_at" : "2022-01-12T03:42:08.661Z",
    "targetTop" : 99.099099099099080814868922977034458662,
    "duration" : 120,
    "enteredBy" : "freeaps-x",
    "targetBottom" : 99.099099099099080814868922977034458662
  },
  {
    "_id" : "CF99D482-06D2-4EE3-9F50-37E2A5A9A611",
    "name" : "๐Ÿ“Eating soon",
    "reason" : "๐Ÿ“Eating soon",
    "created_at" : "2022-01-12T03:07:37.904Z",
    "targetTop" : 81.081081081081066121256391526664557087,
    "duration" : 60,
    "enteredBy" : "freeaps-x",
    "targetBottom" : 81.081081081081066121256391526664557087
  }
]
mountrcg commented 2 years ago

I had commented on the commit earlier. In NS and any other system a new TT always cancels the previous TT. I don't think the the first TT(N,M) should be allowed to finish it's original time until M, if that other TT(>N,<M) finishes before M. Instead there should be no TT after second TT(>N,<M) finishes.

ivalkou commented 2 years ago

@mountrcg original OpenAPS logic is the same as in the PR https://github.com/openaps/oref0/blob/master/lib/profile/targets.js

bjornoleh commented 2 years ago

@mountrcg original OpenAPS logic is the same as in the PR https://github.com/openaps/oref0/blob/master/lib/profile/targets.js

Hi @ivalkou , I didnโ€™t quite understand the logic in the oref0 code you linked to, so I donโ€™t know if you already agree with the below or not:

The way FAX works now is problematic because what you see in Nightscout (previous long lasting TT is terminated when short TT expire) is different from the actual situation in FAX (previous TT resume after a TT with shorter duration expires).

Based on NS experience from other APS (Loop, and probably also AAPS?) I would expect the same result in FAX as in NS. At any rate, the situation in FAX must be correctly displayed in NS to avoid misunderstandings.

ivalkou commented 2 years ago

@bjornoleh I think you are right. 012af037ecac8418966b0201cf353198787fe142 @DobbyWanKenoby PTAL, if it's ok close the PR

DobbyWanKenoby commented 2 years ago

@bjornoleh I think you are right.

012af037ecac8418966b0201cf353198787fe142

@DobbyWanKenoby PTAL, if it's ok close the PR

I think its ok