-
Notifications
You must be signed in to change notification settings - Fork 400
Split the pre-bolus ESC key presses in two commands. #1100
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
The 4 ESC seems to be done too quickly now that we moved to the go library. It seems like it isn't as good at avoiding A52 errors as what was done on master. I think on master the pump is wakened up again between each key presses, or the python layer adds some latency which isn't there on the new version. The compromise here is to send the first ESC, wait for 0.5s before sending the remaining ESC. From testing on a test pump, this seems to reduce the chance of getting an A52 error.
|
Is this related to the fact that ESCs sent while holding down a button (scrolling to dial in a bolus or carb entry) are ignored? If so, is 1+3 sufficient? Or do we need 2+2 or 3+3 or something? |
|
yes, the ESC sent while holding the down button are ignored. you're right, might be better to send 1+4 instead. |
|
Can we tell from the return code or something if the pump actually accepted the button press? |
|
not that I'm aware of, unfortunately. The first ESC sent while the button down is pressed does seem to do something as it pauses the scrolling. But since the down or up button is pressed, it doesn't move us back to the previous page of the wizard. |
|
Would sending four ESCs as individual commands break out of the scroll menu even if the button is still held down? Or do you have to react and release it before they’ll take? |
|
You have to react and release before the ESC can actually move the menu to the previous page. It seems to be the same as trying to press ESC manually on the pump (which makes sense). I also tried manually, and while keeping the down arrow pressed to scroll, no matter how fast or how many time I press ESC, it doesn't exit the wizard. |
|
So we have to have a long enough wait to expect the user to be able to notice the scrolling has stopped and release the button, but not so long that they’ll start scrolling again? Human factors are fun. Can I put in another vote for “let’s stop using the bolus wizard”? |
bin/oref0-pump-loop.sh
Outdated
| echo -n "Sending ESC ESC ESC ESC to exit any open menus before SMBing " | ||
| try_return mdt -f internal button esc esc esc esc 2>&3 \ | ||
| try_return mdt -f internal button esc 2>&3 \ | ||
| && sleep 0.5s \ |
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.
Is this syntax correct? Or do we drop the s?
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 think it's okay, but I introduced an unwanted space in my last change.
bin/oref0-pump-loop.sh
Outdated
| # press ESC four times on the pump to exit Bolus Wizard before SMBing, to help prevent A52 errors | ||
| echo -n "Sending ESC ESC ESC ESC to exit any open menus before SMBing " | ||
| try_return mdt -f internal button esc esc esc esc 2>&3 \ | ||
| try_return mdt -f internal button esc 2>&3 \ |
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.
Let’s do two here. Having an even total number of ESCs puts the pump back on the home screen, vs the info screen you get with one ESC.
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.
done.
|
👍 I use it occasionally 5-10 times a week, but I only got A52 once earlier this year. Just wanted to follow up and try to make it a bit better. If the additional delay here is not worth it, I'm okay also with dropping this pr completely. |
|
I think a half second delay is worth adding. |
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.
Looks like sleep does the same thing with or without an s after the number of seconds.
The 4 ESC seems to be done too quickly now that we moved to the go library.
It seems like it isn't as good at avoiding A52 errors as what
was done on master. I think on master the pump is wakened up again between
each key presses, or the python layer adds some latency which isn't
there on the new version.
The compromise here is to send the first ESC, wait for 0.5s before sending the
remaining ESC. From testing on a test pump, this seems to reduce the
chance of getting an A52 error.