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

Ascending volume to medium instead of maximum #3297

Merged
merged 4 commits into from
May 15, 2024

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Jan 19, 2024

Edit (April 11, 2024):
This is now about more than just adding 3 subpages. It now also adds the new setting to allow ascending volume profile to ascend to medium instead of high.


This PR creates 3 subpages on the Glucose Alert Settings page.
There are already many settings on the page. I would like to add one more. There will be too many settings.
Grouping the settings such that settings that are strongly tied together on the same subpage would allow us to reduce the number of settings while allowing the user to easily find the settings they need.

Before After
Screenshot_20240119-152722 Screenshot_20240119-152408









@Navid200
Copy link
Collaborator Author

In this image, I have circled the settings that are being grouped together.
Screenshot_20240119-152722

The third one has only one setting in it. But, that's the one that will have a second setting after/if this PR is merged.

@Navid200
Copy link
Collaborator Author

If we create too many sub pages (too much hierarchy), it could make things worse as far as helping user find what they are looking for. So, we should not overdo it.

Here, I am only placing settings on a sub page that should not be needed by anyone on a daily basis. These are settings that most users spend some time tweaking until they are happy and hopefully after that, they never touch them again.

Secondly, the settings that are placed on the same subpage are either dependent on each other, or are strongly related to each other.

Der-Schubi added a commit to Der-Schubi/xDrip that referenced this pull request Feb 8, 2024
Merge remote-tracking branch 'Navid200/Navid_2024_01_19' into schubi
@Navid200
Copy link
Collaborator Author

Navid200 commented Feb 15, 2024

And these are the three sub pages.
Screenshot_20240215-000604

Screenshot_20240215-000631

Screenshot_20240411-000720

@Navid200 Navid200 marked this pull request as draft April 11, 2024 02:42
@Navid200 Navid200 changed the title Alert Settings cleanup Ascending volume to medium instead of maximum Apr 11, 2024
@Navid200
Copy link
Collaborator Author

I have added the option to ascend to medium instead of maximum if the ascending volume profile is selected.
By default, this new setting is disabled resulting in the ascending volume profile acting as it did before.

Enabling the new setting does not change the slope. So, the ascending speed is not change. Only the value is clamped at the medium level if the new setting is enabled.

@Navid200 Navid200 marked this pull request as ready for review April 11, 2024 04:14
@Navid200
Copy link
Collaborator Author

I have tested this by monitoring the detail logs and verifying that the volume goes through the expected steps in both cases when the setting is enabled or not.

@Navid200
Copy link
Collaborator Author

The request for this has been made by someone who may occasionally take a while to walk to the phone to snooze the alert.
In the meantime, the ascending volume has reached the max volume annoying everyone else.
They suggested that it may be better if it ascended to medium instead.

When I planned this, I could have taken two different approaches.
I could have changed the max to medium, which would have lowered the slope. I intentionally avoided that because I did not want to slow down the ascension. So, I decided to just clamp it.

@Navid200
Copy link
Collaborator Author

Navid200 commented Apr 13, 2024

This is what we have now in the guide: https://navid200.github.io/xDrip/docs/Ascending-volume-profile.html

If this is approved, this is the image I am currently working on just to give you a better idea how the new setting behaves.
Screenshot 2024-04-12 211807

The draft marker of course will be removed when it is done. And I am still working on it. I am not sure about the guide details yet.

@Navid200 Navid200 requested a review from jamorham May 10, 2024 19:42
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.

Seems reasonable

@jamorham jamorham merged commit c967ee7 into NightscoutFoundation:master May 15, 2024
1 check passed
@Navid200 Navid200 deleted the Navid_2024_01_19 branch May 16, 2024 14:02
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