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

Add specific regional versions of Microtech and Ottai in companion #3756

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Angus-repo
Copy link

  1. Add specific regional versions of Microtech and Ottai in companion mode.
  2. Modify companion mode so that when an xDrip user selects mg/dL as the display unit, if the source reading is in mmol/L, it is multiplied by 18 to convert to mg/dL.

Explanation: In some regions, CGMs are more affordable. Addressing the differences in blood glucose units enables users without insurance to access and use these more affordable CGM options.

… mode.

2. Modify companion mode so that when an xDrip user selects mg/dL as the display unit, if the source reading is in mmol/L, it is multiplied by 18 to convert to mg/dL.
@Navid200
Copy link
Collaborator

Navid200 commented Nov 3, 2024

Thanks for this.
For the real review that counts, please be patient.

I don't think you need to convert units here. xDrip already offers the user to choose the unit they desire. This has nothing to do with the units used in the companion app.
If I were you, I would only add the first 4 lines this PR currently adds minus all the comments.
I would remove everything that is there for the conversion of units.

If you can show a scenario where xDrip does not let you choose the unit of your choice, I would love to see a video clip.

All that is just my suggestion and is not the review that counts.

@Angus-repo
Copy link
Author

Thank you for your reply.

Initially, I only added the first four lines, but I noticed that com.ottai.tag was not displaying correctly. Since com.ottai.tag is intended for specific regions, its unit cannot be changed and defaults to mmol/L. With mmol/L, decimal points are used, which leads to a java.lang.NumberFormatException in the original code when it reaches the line:

mgdl = Integer.parseInt(String.valueOf(ftext))

Given the cost advantage of com.ottai.tag, I’d like to convert the CGM readings in mmol/L to be used as mg/dL. Please review this approach, thank you.

Screenshot_20241103_161426
Screenshot_20241103_161428

@Navid200
Copy link
Collaborator

Navid200 commented Nov 4, 2024

There is no reason to mention the cost advantage. It's really irrelevant to what I suggested.

Can you show a notification of the companion app instead of the app itself?

@Angus-repo
Copy link
Author

Here is the program notification

Screenshot_20241104_085427

@Navid200
Copy link
Collaborator

Navid200 commented Nov 4, 2024

When you compiled your original attempt with only the first 4 lines, did it work if you chose mmol/L in xDrip and only failed when you chose mg/dL in xDrip?
Or did it fail either way?

I know that you want to use mg/dL. I'm not suggesting otherwise. But, I am asking as a test, would it work if you chose mmol/L?

@Angus-repo
Copy link
Author

Yes, if mmol/L is selected as the unit in xDrip, it can read the values from com.ottai.tag.

@Navid200
Copy link
Collaborator

Navid200 commented Nov 4, 2024

OK,
Thanks for patiently answering my questions and sorry about my impatience.
Let's wait for JamOrHam. He will have all the answers.

Thanks again for your submission.

@Navid200
Copy link
Collaborator

Navid200 commented Nov 4, 2024

Wait a minute! This is actually a limitation of the companion app mode.
I'm sorry about my confusion. It may be there for a very important reason.
We will get back to you.

@Angus-repo
Copy link
Author

Alright, I'll wait for your response.

@jamorham
Copy link
Collaborator

jamorham commented Nov 8, 2024

So the only problem with this PR is the mixing of units. One of the main reasons to only parse mmol/l units from a companion app for a mmol/l xDrip is to avoid any chance of parsing data which is not glucose related to minimize any chance of getting the wrong values. I would have to think on it further but I think to allow this we might have to introduce a new configuration option so that the user has to opt in to using different units like this.

@Angus-repo
Copy link
Author

Thank you for your response. Looking forward to the final decision.

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