Skip to content

Conversation

@efidoman
Copy link
Contributor

installing flask-cors module and enabling in app.py allows a web-based script (such as IOS shortcut to running javascript) to access the rig's local web server for retrieving JSON information such as BG records, IOB, COB, Temp Basal/Duration, Battery Percentage, etc.

@efidoman
Copy link
Contributor Author

efidoman commented Jun 2, 2019

I added two more json endpoints for the web service.

cgm - the xdripjs cgm pill json record

hostname - the hostname of the rig. This is important for ensuring the calling application / script is accessing the correct rig and can display the rig hostname to the user. Since many use multiple rigs per person and multiple family members with rigs, it is important to know which rig the data is coming from.

www/app.py Outdated

@app.route("/cgm")
def cgm():
json_url = os.path.join("/root/myopenaps/monitor/xdripjs/cgm-pill.json")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should use myopenaps_dir, not be hardcoded to the default path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is incorrect throughout this file. It appears that @zjedr removed the only uses of myopenaps_dir in 6483692 and replaced them with hard-coded paths.

So if you'd like to defer fixing that to a later PR, I'd also be OK with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestions. I updated all hard-coded paths to use myopenaps_dir. Please take another look while I'm testing tonight.

Copy link
Contributor

@scottleibrand scottleibrand left a comment

Choose a reason for hiding this comment

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

Other than the myopenaps_dir thing this LGTM. Has it received sufficient testing to merge to dev?

@efidoman
Copy link
Contributor Author

efidoman commented Jun 4, 2019

Tested out good for me.

@efidoman
Copy link
Contributor Author

efidoman commented Jun 4, 2019

Yes, it's all working for me in my tests from my browsers on my laptop and from my phone.

@scottleibrand scottleibrand merged commit 8299010 into openaps:dev Jun 4, 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.

2 participants