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

Change deduping interval #7285

Merged
merged 1 commit into from
Feb 20, 2022
Merged

Change deduping interval #7285

merged 1 commit into from
Feb 20, 2022

Conversation

MilosKozak
Copy link
Contributor

Current behaviour of 1 min interval for deduplication is effectively blocking synchronization of 2 consequent events of the same type (like profile switch). I believe 2 sec should be enough for handling data with accuracy of seconds only

Copy link
Member

@bewest bewest left a comment

Choose a reason for hiding this comment

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

LGTM, I'm willing to try this.

@bewest
Copy link
Member

bewest commented Jan 7, 2022

For context on medtronic systems, Bolus records generally closely follow BolusWizard records. BolusWizard records have information concerning meal announcements and bolus calculator recommendations. However, only the Bolus record contains the amount of insulin actually requested as a dose at the completion of a bolus delivery, which occurs some time later. The duration of this lag is generally controlled by the size of the bolus and the max rate of bolus delivery per pump and four minutes was chosen as a reasonable default. Nightscout historically expects "meal events" to contain both insulin and carbs so that the bolus circle can also illustrate the carb to insulin ratio. In order to deliver these events to Nightscout, it's necessary to munge the raw device stream to merge these events together.
The device stream of events is a log of operations while Nightscout historically wants a log of therapeutic decisions.

Here's some further discussion:

It would be nice to gain some observability on the impact of these changes.

@MilosKozak
Copy link
Contributor Author

this constant is affecting only websocket comm. I believe currenly only AAPS is using this channel for data upload

@sulkaharo
Copy link
Member

FWIW I still think the websocket persistence code should be either fully removed or fully rewritten so the REST API and the websocket use the same semantics. We still have the live issue where AAPS users can have duplicate record issues due to NSClient using different data deduplication logic from the REST API. OTOH the 1 minute interval is also clearly wrong logic, so this won't hurt?

@MilosKozak
Copy link
Contributor Author

MilosKozak commented Jan 7, 2022

I agree. Next change will be to API v3. I rewrote NSClient to allow easy transition

@juzi
Copy link

juzi commented Mar 15, 2022

Sorry for the naive question (I don't know much about your code):
I've heard a lot about problems with synchronization and duplicate entries in the past, and I've always wondered why you don't introduce a UUID for each entry that is sent to Nightscout? Like that you could always distinguish two events with the same values. But maybe that is not relevant here.

@sulkaharo sulkaharo deleted the MilosKozak-patch-2 branch December 30, 2022 20:40
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.

None yet

4 participants