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

fix missing timestamp on ongoing BG notification #3579

Conversation

richard-dennehy
Copy link
Contributor

Fixes the issue described in #3578

@jamorham
Copy link
Collaborator

jamorham commented Aug 2, 2024

Thanks for this. I believe this could work but also there was at one point a problem where every update to the persistent notification would cause a notification on Android Wear. I am not sure whether enabling this could cause that to reoccur.

@richard-dennehy
Copy link
Contributor Author

I don't have an Android Wear device to test this on, so I can't confirm either way, but it looks to me like the existing code here is intended to work around the potential issue you're describing

@Navid200
Copy link
Collaborator

Navid200 commented Aug 4, 2024

@PhilTU By any chance, will you be able to use your WearOS device, and a test phone, to do a test if I provide a test xDrip version to you?
I'm sorry for putting you on the spot. Please don't hesitate to decline.

@PhilTU
Copy link

PhilTU commented Aug 5, 2024

@PhilTU By any chance, will you be able to use your WearOS device, and a test phone, to do a test if I provide a test xDrip version to you?

Could I use the built in data generator rather than a sensor? If so, yes.

@Navid200
Copy link
Collaborator

Navid200 commented Aug 5, 2024

Could I use the built in data generator rather than a sensor? If so, yes.

Yes, of course: https://navid200.github.io/xDrip/docs/FakeDataSource.html

@Navid200
Copy link
Collaborator

Navid200 commented Aug 5, 2024

@PhilTU This is the test xDrip:
https://drive.google.com/file/d/1B2EuOGpxyeb80yU31vyLmZY7KJNOp0EQ/view?usp=sharing
Please let me know when you have downloaded it.

With respect to the tests, please kindly confirm the following:
1- On the phone, pulling down from the top of the screen to expose the ongoing notification should show the time in addition to the value.
2- On the watch, there should not be a new notification once every 5 minutes.

Thanks


Edit:
The shared file is the latest Nightly (2024.08.02) plus this PR.
It has a different signature with respect to the official xDrip.
It is only meant for testing and should not be used for regular use.

@PhilTU
Copy link

PhilTU commented Aug 5, 2024

Download and installed. Noted re version / signature. Awaiting arrival of mock data...

@PhilTU
Copy link

PhilTU commented Aug 5, 2024

_20240805_211224

Confirm that notifications now include relative time stamp. WearOS tomorrow....

@PhilTU
Copy link

PhilTU commented Aug 6, 2024

WEAROS configured and showing delta time display as expected. No notifications received as yet (of any sort) certainly not every 5 minutes as per item 2.

DSC_2375

@PhilTU
Copy link

PhilTU commented Aug 6, 2024

I do have frequent event log entries on the phone (one or two per minute)

"AlertPlayer

13:15

setting next alert to 1 minute from now (no problem right now, but needs a fix someplace else)"

This started after snoozing a low alert...

Screenshot_20240806-132730

@Navid200
Copy link
Collaborator

Navid200 commented Aug 6, 2024

@PhilTU Thanks so much for the tests.

I have seen that log (... but needs a fix somewhere else). But, I have never got to work on it yet. I will figure it out some time.

Thanks so so much for running the tests.

@jamorham jamorham merged commit 5597bd7 into NightscoutFoundation:master Aug 7, 2024
1 check passed
@richard-dennehy
Copy link
Contributor Author

Thanks, all 👍

@richard-dennehy richard-dennehy deleted the ongoing-bg-notification-fix branch August 7, 2024 16:20
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.

4 participants