Skip to content

Conversation

@dramageek
Copy link
Contributor

This re-organizes the xDrip CGM option to use xDrip+'s built-in webserver option rather than the on-rig xDripAPS.

New behavior: Checks for online status. If online, continue with regular ns-loop data. If offline, curl data from the local gateway- which should be a phone via either BT or WiFi Hotspot.

This is my first change to the code, so feel free to poke holes in it. I've tested on my rig that it works as intended, but I know I still need to see what it does in other failure modes- i.e. captive gateways.

@tim2000s
Copy link
Contributor

With the current xDrip model, xDrip is pushing data to the rig via a directly connected phone to a specific IP address. How do you ensure that when using the local webserver on a local wifi connected rig and phone, the data is coming from the correct xDrip instance (eg T1 meetups with multiple xdrip users)?

@dramageek
Copy link
Contributor Author

The data would still follow the same path: sensor>phone>rig. The difference is that rather than the phone pushing data to the rig, the rig asks the phone for it.
The xDrip+ webserver still uses your nightscout API_SECRET.

For a meetup situation, each rig would be connected to each person's phone and pull data from that. If everyone connected to the building's wifi, the rigs would see internet and pull from nightscout.

If Person A connected to Person B's phone hotspot, the system would still work as long as B's phone was online. If it went offline, A's rig would try to pull B's numbers, but would fail due to the API_SECRET not matching. Looping would stop for A, but it wouldn't result in wrong data.

I do need to test this out to see what the 'wrong API_SECRET' response looks like, and make sure it fails gracefully.

@alimhassam
Copy link
Contributor

I haven't looked in details, but does this remove support for local CGM uploaders such as xdrip-js? It currently relying on xdripAps.

@tim2000s
Copy link
Contributor

tim2000s commented Jul 22, 2018

@alimhassam YEs it does remove that as It removes the xDripAPS and SQL3 installations.

@dramageek
Copy link
Contributor Author

I didn't realize they reused the xdripAPS code. That probably makes my changes a non-starter if it breaks those.

@dramageek
Copy link
Contributor Author

To be clearer, there's nothing in this that conflicts with xDripAPS, I just removed it because I thought this made it unnecessary.
I'll dig in to this more to see what's going on with xDrip+ that it stopped working. Best I've seen, it stopped working when they changed something back in March.

@philipgo
Copy link
Contributor

Do I understand correctly that xDripAPS is not working properly right now in master/dev?

@scottleibrand
Copy link
Contributor

This PR has not been merged, so AFAIK xdripaps still works. In dev, the upload of BG data has been deprecated in favor of doing that from xDrip+.

@tim2000s
Copy link
Contributor

In dev it’s notnworking properly for some reason. I’ve opened an issue.

@dramageek
Copy link
Contributor Author

xDripAPS is not working for me on either of my Edison rigs either. That's why I started working on this.
I can't narrow down when it stopped working, since I so rarely go fully offline, so it's hard to track what might have caused the change. Went camping recently, and that spurred me to look in to how to fix it.

@alimhassam
Copy link
Contributor

There has been a behavior change in dev. openAps doesn't upload the CGM data from xdripaps to NS anymore.
The functionality was simplified in favor of having the collector do the uploading.

Does that seem like the problem you're having or is it something else? Which collector are you using? You can have multiple endpoint in xdrip+.

@dramageek
Copy link
Contributor Author

Mine is broken with current Master 0.6.2 too, so it's not that. I only targeted Dev since there's no point in trying to push to already-published master, especially with the other refactorings that were made (cron and the broken-install detection)

@tim2000s
Copy link
Contributor

Ah. I’d been running the 0.6.2 version of dev successfully before this update, so it’s this flip that’s broken it for me.

@philipgo
Copy link
Contributor

philipgo commented Jul 29, 2018

I today tested xDripAPS on 0.6.2 master and also found it to be not working.

Edit;
Could this be a Flask problem since the offline web page also seems to have problems? Calling http://APISecrret@rigIP:5000/api/v1/ results in an error. There are a few current CGM entries in /.xDripAPS_data/xDripAPS.db, so it seems to function from time to time (as does the offline web page)

@scottleibrand
Copy link
Contributor

@dramageek do you still want to try to move forward with this PR?

@scottleibrand
Copy link
Contributor

bump

@efidoman
Copy link
Contributor

If this PR does ever move forward, I ask that we consider to keep the existing xdripAPS functionality in place so that xdrip-js (Logger / Lookout) can continue to work.

@dramageek
Copy link
Contributor Author

Sorry, work is crazy at the moment, leaving me little in extra time.
I wholly agree with others that we can't break xdrip-js. A fix that just breaks something else isn't really a fix. In it's current form this patch would break those other sources.

I have been thinking about this, and my version of monitor-xdrip is functionally similar to ns-get.sh . Would I be better off adding this there instead of as a redefined/separate source? It could catch whatever error occurs when the rig is offline, and try this instead? xDripAPS could remain unchanged that way to preserve it's other functions. It sounds like it may still be working for some, so I don't want to impact them either.

Copy link
Contributor Author

@dramageek dramageek left a comment

Choose a reason for hiding this comment

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

updating to the current Dev

@applehat
Copy link
Contributor

Does the xDrip server expose a Nightscout Compatible API?

If so, we could just let OpenAPS have a concept of multiple Nightscout Upload paths (much like how xDrip will currently upload to multiple NS urls, and how its able to upload to Nightscout + xDripAPS)

@alimhassam
Copy link
Contributor

alimhassam commented Sep 17, 2018

@applehat
The point of xdripAps is to provide the glucose reading to openAps when offline.
I don't think its very useful to give it data that openAps already has. (What's the use case for that?)
It's the collectors's responsibility to push the data to xdripAps (and/or to NS when online). xdripAps then provides the data to openAps.

@dramageek
Copy link
Contributor Author

@applehat - I'd debated this too, but have no idea how to open that can of worms. I'm not sure what type of data the xDrip web service will host- it complies with requests for glucose data, but I haven't tried carbs or anything else.
The little info I found is from here: https://github.com/NightscoutFoundation/xDrip/blob/master/Documentation/technical/Local_Web_Services.md

@alimhassam - This doesn't send data to xDrip, it pulls data from it. Instead of the xDripAPS service receiving data from xDrip and then OpenAPS using it, this method has OpenAPS grab it directly from the xDrip app on the phone.

@scottleibrand
Copy link
Contributor

Can someone summarize the status of this PR?

@dramageek dramageek mentioned this pull request Nov 8, 2018
@scottleibrand
Copy link
Contributor

@renegadeandy would this address / obviate your concerns with xDripAPS?

@renegadeandy
Copy link
Contributor

Not entirely convinced it does. In the mean time, I have made a Pull request to allow oref0 to pull from my maintained xDripAPS.

@dramageek
Copy link
Contributor Author

Closing this request. in addition to @sulkaharo doing this much more elegantly in #1139 , @renegadeandy is fixing the original glitch in xDripAPS in #1210 that necessitated this request.

@dramageek dramageek closed this Feb 25, 2019
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.

8 participants