Skip to content

Conversation

@jpcunningh
Copy link
Contributor

If pump settings read is interrupted from an event such as a kill or rig reboot, the loop will fail until profile or settings needs to be refreshed.

This validates the pump settings files contain valid content as part of the decision to call get_settings.

@scottleibrand
Copy link
Contributor

Why did you invert the success criteria? Doesn’t that change it from an AND to an OR, such that only one validation must be successful?

@jpcunningh
Copy link
Contributor Author

Command return values are inverted from normal boolean logic.

0 is true, and anything not zero is false. The first check for SUCCESS in each line allows the function to skip the remaining checks once an error occurs. The second check for SUCCESS in each line limits the echo statement to only the error instead of the error and every line after.

Here is a test output from copy and pasting the function into the shell:

root@MajorCAPP:~# function valid_pump_settings() {
>   SUCCESS=0
>
>   [[ $SUCCESS -eq 0 ]] && valid_insulin_sensitivities >&3 || { [[ $SUCCESS -eq 1 ]] || echo "Invalid insulin_sensitivites.json"; SUCCESS=1; }
>   [[ $SUCCESS -eq 0 ]] && valid_carb_ratios >&3 || { [[ $SUCCESS -eq 1 ]] || echo "Invalid carb_ratios.json"; SUCCESS=1; }
>
>   if ! grep -q 12 settings/model.json; then
>     [[ $SUCCESS -eq 0 ]] && valid_bg_targets >&3 || { [[ $SUCCESS -eq 1 ]] || echo "Invalid bg_targets.json"; SUCCESS=1; }
>     [[ $SUCCESS -eq 0 ]] && valid_basal_profile >&3 || { [[ $SUCCESS -eq 1 ]] || echo "Invalid basal_profile.json"; SUCCESS=1; }
>     [[ $SUCCESS -eq 0 ]] && valid_settings >&3 || { [[ $SUCCESS -eq 1 ]] || echo "Invalid settings.json"; SUCCESS=1; }
>   fi
>
>   return $SUCCESS
> }
root@MajorCAPP:~# function valid_insulin_sensitivities() {
>   set -o pipefail
>   local FILE="${1:-settings/insulin_sensitivities_raw.json}"
>   [ -s $FILE ] && cat $FILE | jq .units | grep -e "mg/dL" -e "mmol"
> }
root@rigName:~# exec 3>&1
root@rigName:~# valid_pump_settings
Invalid insulin_sensitivites.json
grep: settings/model.json: No such file or directory
root@rigName:~# cd myopenaps/
root@rigName:~/myopenaps# valid_pump_settings
"mg/dL"
-bash: valid_carb_ratios: command not found
Invalid carb_ratios.json
root@rigName:~/myopenaps# echo $?
1

@jpcunningh
Copy link
Contributor Author

@scottleibrand, did that answer your question above?

@scottleibrand
Copy link
Contributor

In function get_settings you do [[ $SUCCESS -eq 1 ]] && retry_return check_model 2>&3 >&4 || SUCCESS=0 etc. for each command. Here, it seems you're using $SUCCESS in an opposite way, setting SUCCESS=0 initially and setting SUCCESS=1 if a command fails. The latter feels counterintuitively backwards to me, and inconsistent with the way you did it in get_settings.

@jpcunningh
Copy link
Contributor Author

@scottleibrand, in get_settings, we weren't trying to use the value to set the return value of the function.

I can reverse it to be more intuitive in the body (shell return values always seem counter-intuitive to me being C was my first language 😄), then test the value and return 0 or 1 depending on the value of SUCCESS.

@scottleibrand
Copy link
Contributor

Oh, ok. I missed that we were directly using it as the return code. I agree that the new reversed logic is much easier to understand: thanks.

@scottleibrand scottleibrand merged commit 288343c into openaps:dev Sep 24, 2018
@jpcunningh jpcunningh deleted the validate_pump_settings branch September 25, 2018 01:40
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