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

Remote Commands #7791

Draft
wants to merge 18 commits into
base: dev
Choose a base branch
from

Conversation

gestrich
Copy link
Contributor

@gestrich gestrich commented Dec 29, 2022

Background

The Loop community uses iOS push notifications to deliver commands to Loop such as boluses, carb entries and treatment overrides. These are valuable for caregivers that manage a Looper's treatment remotely (i.e. while in school) Nightscout is used to deliver these commands; however, there are a few shortcomings:

  • There is no command status available. A caregiver may issue a treatment and can't be sure whether the command was issued or not.
  • Push notifications on iOS may be delivered late causing the command to be rejected.
  • A caregiver's decision to send a command depends on having up-to-date information on a Looper's T1D treatment state -- i.e. IOB, COB. If a network delay exists, commands may be issued based on old information. Multiple caregivers may issue redundant commands

This PR enhances the existing Loop command delivery by adding command tracking. Additional planning information exist in the following Google doc.

Features

  • Use a mongo collection to store commands.
  • Add a REST API for adding commands, updating commands, deleting commands, etc.
  • Loop will query these APIs for pending commands, update their status to in-progress, and mark as Success/Fail.
  • The only client at the moment is a Caregiver iOS app but see TBD for later plans to surface this info in NS.

TBD

  • Update Existing Loop Careportal forms to use this new command API: Carbs, Bolus, Remote override.
  • Provide a new Pill to show command status.
  • Visualization in timeline.

@sulkaharo
Copy link
Member

So... Please write up a description of what this change is intended to do? FWIW if I'm right at guessing what this is intended for, the architectural way we're discussed Nightscout would handle this is providing an API that allows users to store "treatment requests" that cannot be deleted so the system retains a track record of data. Also note for a new feature to be added to Nightscout, it should be implemented as a full class feature, including the ability add the data using the web view and visualising the data in the timeline.

@gestrich
Copy link
Contributor Author

gestrich commented Jan 1, 2023

@sulkaharo I created this as a Draft just to begin sharing some early information with the community. This feature has been in discussions on the Loop side in Zulip (see the Caregiver stream) but I realize we need to get alignment with the Nightscout community on how this happens. I don't know how this fits in with other Nightscout plans but welcome feedback on how this can advance.

@sulkaharo
Copy link
Member

So - ideally any features added into Nightscout are implemented in manner that makes them cross-compatible across multiple systems with similar features. So in case of being able to remotely request treatments, it'd be great if folks such as the AndroidAPS and Loop teams worked together to figure out a spec on how this could work, including doing the risk analysis over how a feature like this can be implemented without compromising safety. One of the issues I'm discussing with a variety of folks right now is how to proceed with Nightscout development, where a key problem I'd like to solve is that the codebase includes features that have been implemented for a specific external application (including Loop and AndroidAPS) while at the same time these features don't have good automated test coverage and nobody is maintaining the code. What this has lead to is, as of right now, the dev branch of Nightscout has multiple dependencies with critical security issues and those cannot be upgraded without a large refactor of the entire codebase, which is practically impossible without cleaning unmaintained features away as I can't test a lot of the aforementioned code and the Nightscout maintenance has for years pretty much been a single man job, with help from Ben and some other contributors. So I guess my question here is - if you take a long view of how this feature can exist in a sustainable way, how'd you implement it?

@gestrich
Copy link
Contributor Author

gestrich commented Jan 1, 2023

@sulkaharo Thanks for the background. I suppose a good place to start is for me to chat with the AAPS developers to understand how this could fit into their feature set. Who or where would be the right place to initiate that discussion?

@sulkaharo
Copy link
Member

To add more context - the current Loop push notification implementation uses the apn package, which hasn't been updated in 5 years and now depends on npm modules that have critical security issues. If you have the time to update the current code to using components that are still maintained, that'd be awesome.

@gestrich
Copy link
Contributor Author

gestrich commented Jan 3, 2023

I can look into upgrading the library as part of this work. Node-apn from parse-community seems like the successor to use.

In terms of the coordination you suggested with AAPS developers, I'll plan to post a proposal in the #nightscout-developers channel in Discord. But let me know if there is a more preferred method to get this proposal to the AAPS group for discussion.

@gestrich
Copy link
Contributor Author

I can look into upgrading the library as part of this work. Node-apn from parse-community seems like the successor to use.

In terms of the coordination you suggested with AAPS developers, I'll plan to post a proposal in the #nightscout-developers channel in Discord. But let me know if there is a more preferred method to get this proposal to the AAPS group for discussion.

@sulkaharo Here's a draft PR for the APN update. If you are able to give any feedback as I do some of my own additional testing, let me know. #8047

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