Update Dexcom xDrip sensor start time from transmitter

[Navid200]

If you establish connectivity to a G6 session in progress, xDrip automatically starts an xDrip sensor session and gives it a start time of 3 hours prior to now.
You can confirm this by looking at the classic status page, where the xDrip sensor start time is shown.
If you then look at the G5/G6/G7 status page, you will see the start time reported by the transmitter as "Sensor status".
Those two are in agreement when xDrip is used to start the sensor.

As of the December 5, 2023 release, xDrip broadcasts the xDrip sensor start time to AAPS.
Therefore, the broadcast time is not perfectly accurate for those who use their t:slim pump or Dexcom receiver for starting their sensors.

This PR updates the xDrip sensor start time to match the time reported by the transmitter for G6 Firefly and G7.
As a result the issue reported below is also fixed by this PR.

Fixes: #3250

[Navid200]

This has been tested with G6 and G7 as far as the value shown on the classic status page.

I have not tested AAPS as I don't use AAPS. But, it should work fine.

[jgslade]

I have tested this PR with AAPS and AAPS did receive the correct start time for the G7 Sensor from xDrip.

[Navid200]

@jamorham
When you get a chance, would you please review this?

[DiBochev]

not sure if this could be null...

[Navid200]

@jamorham Would you please review this?
It has been tested by an AAPS user as well.

[jamorham]

I don't think megastatus is the right place to update this as that is only triggered when the UI is displayed. I think changes are needed like:

• Don't change start time the difference is already within say 10 minutes of the existing value.
• Don't change the sensor start time if the start time would be more than say a month in the past
• I would functionalize the check and move the call to it to just after the call to DexTimeKeeper.updateAge() in ob1 state machine EGlucoseRxMessage2 and also in ClassifierAction in the G7 section.

[Navid200]

There is an existing function that is called after every reading, checkAndActivateSensor().
It's function is very close to what we need.
So, I just added a section to it to do what we need.

It only triggers if there is already an active sensor and only if the device is preemptive restart incapable.
Then, it only acts if the delta time is greater than 10 minutes as you advised.

The only thing I have not done that you asked for is the requirement for the start time to not be older than 30 days.
My thinking is that this function is called after we have already accepted a reading from the transmitter logged with Got usable glucose data from transmitter!!. If the transmitter is malfunctioning, we should not accept a reading from it. If its reading is good enough, why should we doubt its session start time?
That was why I did not implement your other suggestion.

Please tell me if this is not good enough.

I have verified that the start time on the classic status page is updated for G6 and for G7.

Thanks

[Navid200]

You asked me to do three things. I believe I have done the first and the last.
With respect to the second, I don't understand the logic. Please help me understand.

If we update the start time only if it is less than a month in the past, it means that we don't believe the sensor could have started more than 30 days ago and s till be active. If that is the reason, I don't understand why we are showing it on the G5/G6 status page. If we are showing it there, doesn't that mean that you believe it is valid? Then, why would you question it for putting it on the classic status page?
I mean if you doubt the validity, should we not ask the same question when we update the other status page as well?

You may be thinking, if you just do what I tell you, this will be merged.
I am thinking I want this to be merged. But, firstly, we have G6 transmitters with sensors that can run for 8 weeks. So, 30 days is not enough for all cases. But, more importantly I want to understand the logic of your request. I don't just want this to be merged. I want to also understand why you ask me to do something so that the next time you won't have to ask me again.

[nickb24]

I ran the code and I noticed a pretty big bug with it. When the start time is updated, the next reading of the G6 sensor then requests a full backfill of glucose data. My use case is that I use an xDrip variant to start and pre-soak a G6 before switching to that sensor on the my main xDrip app. xDrip feeds my glucose data to AAPS and therefore this sort of extra backfilling is not acceptable in my opinion.

If the sensor start date is going to be changed on an active sensor, then the code for the backfill calculation needs to be modified to not backfill beyond when the last glucose reading from the old sensor was received, or beyond the last sensor stop time. I think the easiest would be to not backfill beyond the last sensor stop.

[Navid200]

@nickb24 Thanks

[Navid200]

@nickb24 In 5 days, I will test this when I start my next G7. It should let me recreate what you are showing.
Then, I will find a way to limit backfill so that this does not happen.

[nickb24]

I think it would do the same on the G7. Can't test it because I don't have a G7. But the code here is shows why it backfills to the start time of the sensor:

xDrip/app/src/main/java/com/eveningoutpost/dexdrip/g5model/Ob1G5StateMachine.java

Line 1099 in 2db9aa6

final long startTime = Math.max(earliest_timestamp - (Constants.MINUTE_IN_MS * 5), sensor.started_at);

Not sure why we would even backfill beyond the last timestamp of glucose data. Not sure of the history of this part of the code.

[nickb24]

@Navid200 I had some free time today to take a closer look and figured out where the issue lies.

At this line,

xDrip/app/src/main/java/com/eveningoutpost/dexdrip/g5model/Ob1G5StateMachine.java

Line 1069 in 1212e75

final List<BgReading> lastReadings = BgReading.latest_by_size(check_readings);

The backfill calculator is getting the list of last readings from the past in order from newest to oldest. However that function gets a list of glucose readings, but ONLY from the active sensor, as seen in that function:

xDrip/app/src/main/java/com/eveningoutpost/dexdrip/models/BgReading.java

Line 793 in f1dd5e9

.where("Sensor = ? ", sensor.getId())

I'm a little surprised by all this, because I thought people do switch from an active G6 sensor started somewhere else into xDrip. Unless no one has really cared about this in the past, or there is some other logic that is different in this use case. The only way to skip this logic is for ask_for_backfill = false;. But I don't see how that will ever be false in this case.

[Navid200]

@nickb24 People start G6 with a receiver or with their t:slim pump or even with a previous xDrip installation.
There is nothing wrong with connecting to such a session with xDrip.

And I have not been able to recreate the problem you have reported with this PR. Is it possible that you had anything else changes in your compile?
I installed this PR after an uninstall. I set up fake data source and let it run for a few hours. I then switched data source to a G6 that has a session in progress. It only backfilled starting from when I switched to G6. So, it never brought in the existing previous readings to overlap the fake data source readings.

[nickb24]

@Navid200 I'm sorry I just reviewed this pull request history and I see that there was only 1 user who tried this. That's my fault. I thought more people tested it before me.

People start G6 with a receiver or with their t:slim pump or even with a previous xDrip installation.
There is nothing wrong with connecting to such a session with xDrip.

Yes that works, but that's because the sensor start time is not set to the time the sensor was actually started. It is set to the current time when xDrip first connects to a session in progress. So the backfill code won't backfill passed the sensor start time.

And I have not been able to recreate the problem you have reported with this PR. Is it possible that you had anything else changes in your compile?

My code has nothing else that would conflict. The only other thing I did is that when I switch from xDrip variant, to my main xDrip, I make sure to unbond the transmitter first. So then on my main xDrip it does a full bond/pairing before connecting to the in progress session. Maybe that's the issue, not sure.

[Navid200]

@nickb24 Was engineering mode enabled when you ran your test?

[nickb24]

@nickb24 Was engineering mode enabled when you ran your test?

No engineering mode is off.

I did think about this more and I maybe have a way to replicate this. So I when I switch to an active sensor I would notice sometimes that it would never pick up on the active session even after 4-5 reads. I would still have to click on start sensor and then it would pick up the session.

So this is the sequence I did to cause my error. G6 session running already. On my main xDrip, this transmitter is not bonded. So, first reading happens and it bonds and I accept pairing request, then that's it. Next read comes in and detects an active session but still doesn't pick up glucose readings yet (logs show that since no sensor is active then no readings). So after this second reading I go and start the sensor from the menu, enter the sensor code. Next reading comes in and gets glucose and that it's it. Then on the next reading, it gets glucose, (presumably updates the sensor start time to the real one), and then it backfills the last 3 hours of that sensor and overlaps the readings.

[Navid200]

@nickb24 Can you check your Gitter messages please?

[Navid200]

I was testing a different PR (#3319) and got an overlap:

The hardware data source was on xDrip sync follow. I switched it to G6 to test the PR, with that PR installed, and the overlap ocured.

I am not sure what causes this. But, I don't believe it is related to the change I am proposing here in this current PR for updating the start time from the transmitter for G7 and Firefly.

[Navid200]

Again, I have a case of an overlap not with this PR.

This time, I have switched xDrip from being a G7 collector to a Nightscout follower.
I don't believe the overlap is caused by this PR.

[Der-Schubi]

Tested with a four day old G7. Without this commit the classic status page has shown 14 days sensor time. With this commit it updated start time on classic status page and sent it correctly to AAPS.

[jamorham]

I'm thinking simplistically we can't update the start time of the current sensor before the end time of the previous sensor. If we want to allow overlapping sensors then the logic for detecting duplicates needs to ignore which sensor a reading comes from but that likely involves quite a few more changes and I'm worried about unintended consequences there.

Would incorporating the check for the previous sensors end time as a gate for how far we can move the sensor start time backwards at least solve many instances of the problem this PR is designed to address?

[Navid200]

Sorry. This sounds like a misunderstanding. There is no overlapping.
There is only one active sensor at a time.

You can do the exact same thing with a G6. xDrip automatically stops the previous sensor before switching to the new one.
G7 behavior is different in xDrip and I suspect (only suspect) it has to do with the fact that we cannot associate the Bluetooth name because Dexcom changed the naming convention with G7.

[Navid200]

What I mean is that as far as xDrip is concerned, there is only one active sensor at a time.
There is a time we have two active sensors running. But, xDrip only knows the first. We use a different xDrip on a different phone to verify that the second one is running.
After the first sensor stops, we change the transmitter ID in xDrip to the pairing code of the new sensor.

[Der-Schubi]

Could this be a problem with backfilling? Wenn we set (and therefore start) a new G7 while the old one has a few hours left? Will xDrip backfill the data back til sensor start after connecting to it? Would we end up with double data?

…

On 4 March 2024 19:34:26 CET, Navid ***@***.***> wrote: Sorry. This sounds like a misunderstanding. There is no overlapping. There is only one active sensor at a time. You can do the exact same thing with a G6. xDrip automatically stops the previous sensor before switching to the new one. G7 behavior is different in xDrip and I suspect (only suspect) it has to do with the fact that we cannot associate the Bluetooth name because Dexcom changed the naming convention with G7. -- Reply to this email directly or view it on GitHub: #3263 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[Navid200]

No, there is no problem with double data. You can test this with a G6. xDrip only backfills the missing data.

[Der-Schubi]

Then we should be fine @jamorham

…

On 4 March 2024 19:42:28 CET, Navid ***@***.***> wrote: No, there is no problem with double data. You can test this with a G6. xDrip only backfills the missing data. -- Reply to this email directly or view it on GitHub: #3263 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[Navid200]

@jamorham The problem this PR is trying to resolve is a problem we have had since G6. May be even G5, but I am not sure.
The start time shown on the classic status page does not match the start time shown on the main status page.
I never wanted to do anything about it because we could use some G6 devices in non-native mode. In that case, it didn't matter what the transmitter thought the start time was.

But, we cannot do that with G7.
However, the problem has become much bigger with G7 for some reason I am not sure about resulting in the sensor start time never updating in xDrip for G7.

This may not solve the root cause of the second problem.
But, it will solve the problem of sending the correct start time to AAPS.

There is a lot about G7 I am still trying to figure out.
For example, I know that backfill only works if the start time is more than 24 hours in the past. If it is less, xDrip never backfills from G7. I intend to fix all of these.
I suggest we go one step at a time.

This PR fixes what is shown on the classic status page and sent to AAPS. And it does not break anything else.
I will continue to work on remaining issues.

[jamorham]

What I'm saying is if we adjust the start time of a sensor and this brings it back before the end of the previous sensor (which as far as I can see is not being checked here) then code which checks for glucose values that only reads data matching the current sensor can miss there being multiple readings for the same time period.

What I am suggesting is that we don't rewrite the sensor start time earlier than the previous sensor stop time to avoid that potential problem.

[Navid200]

But, I want the sensor start time to be rewritten so that the start time reported to AAPS would be correct.

I didn't think there was a chance xDrip would show two readings for one time. However, we have seen it happen occasionally.
Isn't it a better solution to ensure xDrip would never do that?
I mean can we not have a check in place to never accept a reading for a time that already has another reading with a time less than 1 minute difference?

[Der-Schubi]

Isn't it a better solution to ensure xDrip would never do that? I mean can we not have a check in place to never accept a reading for a time that already has another reading with a time less than 1 minute difference?

This would have to be in less than five minutes, not one.

[Der-Schubi]

What I am suggesting is that we don't rewrite the sensor start time earlier than the previous sensor stop time to avoid that potential problem.

In case of G7 this would be bad, as no one would know when the sensor will stop exactly.
(Cause the G7 starts itself when it gets setted)

[Navid200]

This would have to be in less than five minutes, not one.

We have devices that create a reading once every 5 minutes and we also have devices that create a reading once a minute.
We can have the new check I am suggesting take into account the repeat cycle of the previous device as well as the new device and decide what delta is expected between two adjacent readings, and if the new reading does not satisfy it, it should be rejected.

[jgslade]

From my experience there's no overlapping data from having overlapping sensors. I overlap my sensors by about 6 hours each time and have not experienced overlapping data.

Here's an example from my last sensor change:

Here is the last reading before I switched, this sensor was read on the 2s and is.

This is the first reading of the new sensor it's being read on the 1s and 6s.

I inserted the new sensor at 6:33 am here's a screenshot showing the entire overlapping time and there's no double readings.

So the double readings Nick encountered were not from this patch, or even necessarily from overlapping sensors.

[Navid200]

I just rebased this to make sure it does not have a conflict with the magic here: 24922b4

After that, I established connectivity to a newly started G7:

And this shows the status page:

And this shows the correct start time on the classic status page.

I then switched to my old G7, which is still active. There is no overlap:

These are the status pages after the switch showing the correct start time on the classic page.

[Navid200]

Please don't consider this PR to be something that solves all problems.
Please consider it as a step forward. It updates the start time of an active session to match the reported start time from the transmitter.

The start time of the xDrip session had a significant importance when we used G5 because we could choose to use it in non-native mode.
I don't think G5 sensors are being made any longer.
The remaining products, G6 and G7 and their variants, can only be used in native mode. There is no reason to have an xDrip session start time that is different from the start time reported by the transmitter.

[jamorham]

We can't set a transmitter's start time before the end time of a previous transmitter. Yes it would be better if we could do this but the de-duplication code is itself duplicated over different collectors and I believe is limited in its search to the current sensor session. Changing all of that could introduce other unintended consequences as well as duplicate data on the graph and there may be other places where it is assumed that the sensor sessions cannot overlap. It feels too risky to change.

Instead I would suggest solving this issue by reporting what we think is the transmitter's version of the sensor start time to AAPS instead of xDrip's sensor start time because these are decoupled in the implementation at the moment because it is based on a model of how G4 worked.

I think this can be done by looking at the DexCollectionType and then providing the data from the session time keeper to AAPS instead of trying to alter xDrip's Sensor start time.

[Navid200]

@jamorham

Yes it would be better if we could do this but the de-duplication code is itself duplicated over different collectors and I believe is limited in its search to the current sensor session.

Why are you concerned about different collectors.
This change is applied only to devices using the G7 or Firefly G6 collectors.

Would you please look at the following image?

It shows that my G7 sensor started 26 days ago.
But, we know that is incorrect. This PR fixes that.

It will also allow us to close this issue as well: #1632 as this is the only remaining discrepancy between the two status pages for a Firefly.

It is possible I am misunderstanding you. What can go wrong for someone who is using a G7 or a G6 Firefly?
I showed in my previous post how I switched back and forth between two active G7 devices and there was no duplication on the graph.

[Navid200]

@jamorham If the change this PR makes can possibly be a problem, we can remove the sensor start time from being shown on the classic status page for G7 and Firefly G6.
Would that be acceptable then?

[Navid200]

For those of you interested in this fix, I have opened a new PR to address the latest raised concerns: #3414

If you can test it, please report in the PR. Please do not test anything that could compromise your diabetes control.
Please do not uninstall xDrip before backing up your database and settings.

[Navid200]

The correct start time is now broadcasted for G7 after this: #3414

As far as the start time being shown incorrectly on the classic status page, we need to deal with that separately. What really needs to be fixed is getting xDrip to auto start. Sometimes, it doesn't. That will be very nice to fix.
The start time can be removed from the classic status page if needed.
Anyway, all of that needs to be done in other PRs. So, there is no need for this one as it stands.