-
Notifications
You must be signed in to change notification settings - Fork 400
ability to add script plugins to oref0-pump-loop #1212
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
Conversation
|
Code looks good. If someone can post test results, I think we can merge this. |
|
needs a quick review again. I found an issue while testing where the plugins didn't run upon any loop failure. The intention is for plugins to run at the end of every pump loop regardless of pass or fail. Also, I updated the comments and logging a bit. |
|
Here's my latest test results that include the Logger PR to leverage this new plugin capability for running fakemeter: |
bin/oref0-pump-loop.sh
Outdated
| # pump-loop errored out for some reason | ||
| fail "$@" | ||
| fi | ||
| run_plugins |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving it here won't help make it run on failures, as the fail function calls exit 1. You'll instead need to add run_plugins after update_display there as well as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I was not aware. I have updated run_plugins to run in both places so it runs either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tweaked the placement to be more consistent between the two cases. LMK if you don't think that will do the right thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
When a loop fails, we should do the various reason-for-failure checks (and display their output) before moving on to run_plugins. Since we're doing run_plugins after update_display in the success case, we should do the same in the fail function.
scottleibrand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anything else before we merge?
|
It's ready. I've been watching the logs and testing - works great. |
This PR adds the ability to inject custom script(s) to run synchronously at the end of every oref0-pump-loop. Any script files with executable flag set will be run as follows:
If the script file is placed in the plugins/every folder then the script runs at the end of every loop.
If the script file is placed in the plugins/once folder then the script once at the end of the current loop.
Script files placed in the plugins/once folder will be removed once run. This allows for a use case such as the following:
Logger and Lookout both call a pump utility called fakemeter that helps out users in offline mode by sending the current CGM (Dexcom) glucose value as a meter glucose value. This shows the glucose on the pump every 5 minutes and allows you to quickly see the current glucose value on the pump. Since this script requires communication with the pump, it will collide with oref0 pump communications on occasion. This oref0-pump-loop plugin capability will allow Logger and Lookout to run fakemeter synchronously at the end of the pump loop and avoid any radio collisions.