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

Add toggle to allow downloads from Nightscout (And improve NS Config UI) #221

Merged
merged 5 commits into from
May 23, 2024

Conversation

dsnallfot
Copy link
Contributor

@dsnallfot dsnallfot commented May 20, 2024

Linked to issue #198

Adds a Download toggle (Default setting false) that needs to be flipped on to allow any Nightscout download of Treatments (Carbs, Temp Targets) and Announcements (Remote commands for pump, basal, overrides and bolus).

When the toggle is disabled the functions fetchCarbs, fetchTempTargets and fetchAnnouncements is blocked from running (the same way that the toggle for Upload blocks upload and deletion of Treatments, preferences and settings.

Header renamed from "Allow Uploads" to "Allow up- and downloads"

IMG_3761

This PR do not change the current Allow remote control of Trio toggles since there could be room for discussion if disabling bolus commands maybe should have its own toggle (ie that you want to block Remote bolus announcements but still want to allow fetching carbs and temp targets).

@dsnallfot dsnallfot marked this pull request as ready for review May 20, 2024 15:28
@dsnallfot
Copy link
Contributor Author

dsnallfot commented May 20, 2024

@Sjoerd-Bo3 @aug0211 @MikePlante1
I cannot add reviewers for the PR, so I just tag you here.

This is just a PR for the download treatments toggle. No refactoring of the "allow remote bolus" toggles that was also discussed in discord. That could be of course be added, but I realized that its probably good to discuss if we want to keep the possibility to allow carbs and temp targets downloads, but block remote commands for bolusing for instance. Then we need to keep the allow remote control-toggle (but maybe rename it somehow)

@aug0211
Copy link
Contributor

aug0211 commented May 21, 2024

@dsnallfot what happens if we enable downloads but disable the other remote treatments toggle? I haven’t fully grokked how these work together yet.

@dsnallfot
Copy link
Contributor Author

@aug0211 if the download toggle is off, no treatments or announcements gets fetched.

If the download toggle is on, treatments get fetched. But announcements get fetched only if the remote control toggle also is on.

To make this better we could disable and hide the "remote control" toggle when download is off? What do you think?

@dsnallfot dsnallfot marked this pull request as draft May 21, 2024 05:48
@dsnallfot
Copy link
Contributor Author

Converted to draft until decided if I should also hide the remote control toggle/section when download is off.

- Imroved structure and UX workflow when configuring Nightscout settings
- Added Connect, Upload, Fetch views with relevant settings per view
- Import settings and backfill glucose buttons kept in rootview since they are more frequently used
- Added footers for many sections to explain what the toggles actually do
- Loacalizaation needed for new textstrings. Just updated that with small changes in already existing strings
@dsnallfot dsnallfot changed the title Add toggle to allow downloads from Nightscout Add toggle to allow downloads from Nightscout (And improve NS Config UI) May 21, 2024
@dsnallfot dsnallfot marked this pull request as ready for review May 21, 2024 17:55
@dsnallfot
Copy link
Contributor Author

dsnallfot commented May 21, 2024

Demo after latest commit 6e187c1

Simulator.Screen.Recording.-.iPhone.13.mini+.-.2024-05-21.at.20.05.36.mp4

@bjornoleh bjornoleh linked an issue May 22, 2024 that may be closed by this pull request
Copy link
Contributor

@dnzxy dnzxy left a comment

Choose a reason for hiding this comment

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

Looks good to me overall. Great work. Maybe add the new isDownloadEnabled settings to the defaults file as well? Small comment regarding multiline text.

@dsnallfot
Copy link
Contributor Author

dsnallfot commented May 22, 2024

Looks good to me overall. Great work. Maybe add the new isDownloadEnabled settings to the defaults file as well? Small comment regarding multiline text.

Ok. I thought I had added it everywhere? (Used the existing isUploadEnabled as guidance. Can you please point me to where I find the defaults file?

@dnzxy
Copy link
Contributor

dnzxy commented May 22, 2024

I meant this file https://github.com/dsnallfot/Trio/blob/dev-test-stuff/FreeAPS/Resources/json/defaults/freeaps/freeaps_settings.json but just realized I missed that you had added it already. Please disregard that comment 😅

- Break up long strings in several text elements (enables re-use of some already existing translations)
- Change button text "Connect" to "Connect to Nightscout" in NightscoutConnectView
@dsnallfot dsnallfot requested a review from dnzxy May 22, 2024 21:12
Copy link
Contributor

@aug0211 aug0211 left a comment

Choose a reason for hiding this comment

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

Approved!

I was a bit thrown off by how many localization updates were in here, but they are

  1. trivial in nature (capitalization)
  2. consistent/intentional
  3. related to the NS options focused on across this PR

Let's go! 🚢

@aug0211 aug0211 merged commit fc1fd4b into nightscout:dev May 23, 2024
1 check passed
@MikePlante1 MikePlante1 mentioned this pull request May 27, 2024
@dsnallfot dsnallfot deleted the dev-test-stuff branch June 16, 2024 12:35
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Aug 1, 2024
Change the "rotation hint" for watch bolus confirmation
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.

Toggle option to Disable Fetching of Nightscout Carbs in Trio
3 participants