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

Allow history to show all readings even for 1 reading per minute #3202

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Nov 22, 2023

To recreate the problem reported here, #3190, I created a test version in which, I increased the period to 10 minutes (1 reading per 10 minutes rate).
Then, history I had generated with a 5 minute period (1 reading per 5 minutes), was shown only covering half a day everyday.

This PR increases the max rate to 1 reading per minute for history.

Fixes: #3190

@Navid200 Navid200 changed the title Allow history to show 1 reading per minute Allow history to show all readings even for 1 reading per minute Nov 22, 2023
@Navid200
Copy link
Collaborator Author

I cannot test this because I don't have a source that has a 1 reading per minute rate.
But, I verified that I can see the problem by going the other way and intentionally changed the code temporarily to reduce the allowed rate to less than 1 reading per 5 minutes and I could see the error as reported.

If you tell me how I can test it at 1 per minute, I will.
Otherwise, please merge and let it be tested as a Nightly.

Copy link
Collaborator

@jamorham jamorham left a comment

Choose a reason for hiding this comment

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

So it is possible to retrieve sample period via DexCollectionType.getSamplePeriod() but I'm thinking there could be problems if collection type is changed etc so for this table view which isn't used that frequently it is fine to request more rows than we might always need.

@jamorham jamorham merged commit bda62e3 into NightscoutFoundation:master Nov 29, 2023
1 check passed
@Navid200 Navid200 deleted the Navid_2023_11_22 branch November 30, 2023 02:01
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.

2 participants