search

NightscoutFoundation / xDrip

Nightscout version of xDrip+
https://jamorham.github.io/#xdrip-plus
GNU General Public License v3.0
1.41k stars 1.15k forks source link

Basal Rates sent to Tidepool wrong / too high #2130

Closed janvitos closed 2 years ago

janvitos commented 2 years ago

Subject of the issue

xDrip is now sending the Basal Rates to Tidepool, but the values sent are often wrong and way too high.

Your environment

Expected behavior

Basal Rates in Tidepool should always reflect the proper Basal Rates from xDrip and AndroidAPS.

Actual behavior

The current Basal Rate is 0.40 U/h, and because of the AndroidAPS closed loop and Temp Basals, it fluctuates between 0.00 U/h and 1.20 U/h. But in Tidepool, when the Basal Rate changes to 0.20 U/h, 0.50 U/h or 0.90 U/h for example, the Basal Rates shown in Tidepool are 20 U/h, 50 U/h and 90 U/h. This also results in a total daily Basal Insulin of 100 U+, when it should be closer to 10 U.

It seems like the Basal Rates are only good in Tidepool when they're at their base of 0.40 U/h. Otherwise, when the Temp Basal kicks in and Basal Rates fluctuate, xDrip (or Tidepool) seems to multiply the Basal by 10.

Screenshots

https://imgur.com/a/juRFmij

janvitos commented 2 years ago

Here's another screenshot I took after a full day of Basal Rates sent to Tidepool: https://imgur.com/a/uqXRdnH

As you can see, Total Basal Insulin stands at 182.9 U, when in reality, it should be closer to 8.0 U as confirmed by the Nightscout Daily Report.

Navid200 commented 2 years ago

Please open a discussion instead of opening an issue.
https://github.com/NightscoutFoundation/xDrip/discussions

This is requested from everyone. If a developer asked you to open this, please clarify and we can reopen.

janvitos commented 2 years ago

Please open a discussion instead of opening an issue.

Alright. Thanks.

tolot27 commented 2 years ago

This is clearly a bug.

psonnera commented 2 years ago

Reopening as inconsistent basal rates appear in Tidepool.

janvitos commented 2 years ago

I am available for testing if you need so.

jamorham commented 2 years ago

Anyone with this bug can you open xDrip's basal profile editor and check that the rates there match what is expected? @janvitos

fglta commented 2 years ago

I can confirm that the rates in xDrip's basal profile editor are normal and as expected.

I'm trying to piece together what I see, and so far I'm noticing this behavior with TBRs. Specifically, I see a TBR of "1.10U/h" be sent to Tidepool as a "10U/h", and a "1.60U/h" TBR to be sent to Tidepool as "60U/h". I saw the regexp pattern-match in getAbsoluteBR() but I can't figure out yet where that fails. It would help me if I understood what the input to this function is (statusLine). Is it possible to view that somewhere?

One of the (unvalidated) theories that I have is that this may be locale-dependent. This behavior is observed on a phone with a locale set to one where the decimal point is "," (comma) and not "." (period). I am wondering if the TBR is formatted as "0,40U/h" in the statusLine string, which would explain why the .*(^|[^0-9.])([0-9.]+U/h regexp parses it as 40U/h. If I can find the statusLine string somewhere I can set a test TBR and try to validate this theory.

jamorham commented 2 years ago

Yes this is quite possible. I wasn't able to test the absolute basal rates generated by AAPS 3.x and just used examples provided to me , Please provide some more examples with different numerical formatting. See what is in use at the moment in the tests: https://github.com/NightscoutFoundation/xDrip/blob/master/app/src/test/java/com/eveningoutpost/dexdrip/wearintegration/ExternalStatusServiceTest.java#L28

janvitos commented 2 years ago

Anyone with this bug can you open xDrip's basal profile editor and check that the rates there match what is expected?

The basal rates are fine and as expected.

One of the (unvalidated) theories that I have is that this may be locale-dependent

Indeed, the phone with xDrip that is sending the basal rates to Tidepool is in French, using a coma (,) as decimal point. So this theory makes total sense.

jamorham commented 2 years ago

If I can get a screenshot or similar showing what the external status looks like I can add that to the unit tests and make sure it passes. Do we know if there would be any other locale issues like AAPS using something other than U/h to represent basal?

janvitos commented 2 years ago

I'm testing the Regex as we speak and I can confirm the problem most likely lies there.

First, there's an issue in the Regex:

.*(^|[^0-9.])([0-9.]+U/h)

The forward slash in U/h should be escaped:

.*(^|[^0-9.])([0-9.]+U\/h)

Second, the result when using a dot as a decimal point is fine and the entire group is selected. But when using a coma, only the part after the dot is selected (second group). For instance, if the TBR is 0,40U/h, then only 40U/h is returned. This is exactly what is happening in Tidepool.

Here's the fixed Regex for dot (.) AND coma (,) as decimal point:

.*(^|[^0-9.|,])([0-9.|,]+U\/h)

Using this Regex, 0,40U/h returns the entire string, and not just 40U/h.

jamorham commented 2 years ago

Ok, good can you supply a test string for integration to the unit tests? I have already added tolerantParseDouble in to getAbsoluteBRDouble()

janvitos commented 2 years ago

Ok, good can you supply a test string for integration to the unit tests?

Not quite sure what you are asking here. Where would I find the test string?

jamorham commented 2 years ago

I'm asking what a status line for french locale would look like in xDrip when provided by AAPS, see the link above to the unit tests with the existing AAPS 3.x absolute basal string which is being used for tests.

jamorham commented 2 years ago

I'm pretty sure this is in fact the correct java regex pattern .*(^|[^0-9.,])([0-9.,]+U/h) - the | or operator is not needed inside a character range and escaping \ is invalid according to the ide. I think if other locales are using different unit description eg, not U/h then there is going to need to be a more complicated fix but I think this should work definitely for french.

janvitos commented 2 years ago

Thanks for the clarification @jamorham.

What I'd like to know, is where can I find this status line? xDrip debug? AAPS debug? AAPS code? I'm really not sure where to look.

I will gladly provide this status line if I know where to look.

Thanks!

janvitos commented 2 years ago

I'm pretty sure this is in fact the correct java regex pattern .*(^|[^0-9.,])([0-9.,]+U/h) - the | or operator is not needed inside a character range

I will test this now.

Edit: Is valid and tests out fine.

escaping \ is invalid according to the ide

Probably using the Python (or similar) regex flavor then.

jamorham commented 2 years ago

I think to see the status line you'd need a working AAPS installation. I'm not sure exactly where it is generated within AAPS code.

janvitos commented 2 years ago

I think to see the status line you'd need a working AAPS installation

I do have a working AAPS installation.

Let me do a bit of digging.

jamorham commented 2 years ago

in xDrip settings search for Extra Status Line and enable it and tick on the External Status checkbox. It should then display top left of xDrip home screen.

janvitos commented 2 years ago

in xDrip settings search for Extra Status Line and enable it and tick on the External Status checkbox

Will do!

janvitos commented 2 years ago

Here's a status line example for French xDrip / AAPS installations:

0,00U/h 1,04U(1,11|-0,07) -0,35 4(20)g

Right now, the TBR is at 0,00, so maybe I can wait until there is something like 0,40 (or any value that normally causes problems in Tidepool).

But my child is really insulin sensitive today so it might not happen...

jamorham commented 2 years ago

ok I will incorporate this to the unit tests and if confident roll a nightly with these changes. Thanks for all your help.

janvitos commented 2 years ago

ok I will incorporate this to the unit tests and if confident roll a nightly with these changes

Sounds great!

I will gladly test the nightly and give my feedback.

Thanks for your guidance.

jamorham commented 2 years ago

https://github.com/NightscoutFoundation/xDrip/commit/e44f4e1c0029f0d58b6faf70ac578458563bb1b2 the change is here.

jamorham commented 2 years ago

Nightly is now available.

janvitos commented 2 years ago

Nightly is now available.

Awesome! Installing now.

Feedback coming soon.

janvitos commented 2 years ago

So I can confirm that the new nightly build fixes the issue!

Here is a screenshot from Tidepool:

https://imgur.com/a/TRul7kz

As you can see, since updating to the most recent xDrip nightly build, the basal rates are now properly passed on from AAPS / xDrip to Tidepool. I can also confirm that the values and timestamps in Tidepool match the ones from AAPS.

Thanks to @fglta and @jamorham for your great work.