Unacknowledged command handling#16
Conversation
| } | ||
|
|
||
| struct MessageSendSuccess: MessageResult { | ||
|
|
There was a problem hiding this comment.
Good to see the MessageResult protocol, the MessageSendFailure, and the MessageSendSuccess structs go away. I had prototyped a simpler way to do this using no return type for sendMessage() which throws on errors, but this won't be needed with this approach using a SendMessageResult return.
| } | ||
|
|
||
| if let isLowTemp = rawValue["isLowTemp"] as? Bool { | ||
| self.isHighTemp = isLowTemp |
There was a problem hiding this comment.
If this odd looking code is correct, a comment would be helpful.
| "scheduledCertainty": scheduledCertainty.rawValue, | ||
| "automatic": automatic | ||
| "automatic": automatic, | ||
| "isLowTemp": isHighTemp, |
There was a problem hiding this comment.
Ditto. Not sure if is a mistake or being done on purpose.
| throw IllegalResponseException(responseType, response) | ||
| } | ||
| */ | ||
|
|
There was a problem hiding this comment.
I believe that there was some special stuff added added to Dash messaging for dealing with alerts at a higher level that this commented out code that is being deleted is referencing. Has this new Dash alert system been investigated at all?
There was a problem hiding this comment.
I'm not sure what you're referring to, or what this code was doing. If you have any clues, let me know.
There was a problem hiding this comment.
Looking at this again, this commented out code is taken from the Kotlin code here: https://github.com/nightscout/AndroidAPS/blob/master/omnipod-dash/src/main/java/info/nightscout/androidaps/plugins/pump/omnipod/dash/driver/comm/session/Session.kt. This used to be the place in AAPS where they were checking for an unexpected fault (alarm) response or a nak response from the pod. This checking has since apparently moved elsewhere for AAPS. For Loop this handling is handled at the next level up in PodCommSessions.send() as it was for Eros. So there is no problem removing this commented out code which isn't helpful or useful (and I agree that it would be good to do).
The special stuff for dealing with alerts that I had originally thought might be related was something completely different that I looked like was for dealing with the 8 pod alerts & the reminder beeps at a different level from what I remembered from some time back now. When I did some quick searches in the AAPS code, I couldn't find anything that looked like what I remember. This might have been something I saw in the decompiled PDM source which I can no longer find online. In any case, what I am remembering has nothing to do with this particular code being deleted.
|
|
||
| import Foundation | ||
| import LoopKit | ||
| import OmniKit |
There was a problem hiding this comment.
Can this import be avoided? I am hoping that eventually all the code in OmniBLE/OmnipodCommon can be made into a separate Framework that can be shared between OmniKit and OmniBLE. With some more work, it might be possible to share even more code between OmniKit and OmniBLE.
2 participants
This PR implements the OmniBLE portions of the unacknowledged command handling system in Loop dev.
Upon any failure where Loop has both sent an insulin delivery affecting command, and not heard back whether the pod received it, Loop will be in a mode of uncertain delivery. Loop will not dose further in this mode until communications are restored, and Loop has determined if the pod heard the previous message.
It does this by recording the message sequence number of the last sent command, and checking the last command sequence field that is returned in status responses from the pod when comms are restored. If they match, then Loop knows the pod received the command, and will update its state to match. If the sequence does not match, then Loop will consider the command to not have been received by the pod, and Loop state will remain as if the command were not issued.
This all happens automatically; there are some visual screens to let the user know what's going on in this mode, but it's possible that the problem will arise and be resolved without any user action. This is what should happen in most cases, and is a big improvement over previous handling, where we just marked these commands as uncertain.
If communications cannot be restored, the user can use the onscreen modal to deactivate/discard the pod. In this case Loop will resolve the unacknowledged commands in a way that assumes the greatest amount of insulin delivery, to err in the direction of hyperglycemia instead of hypoglycemia. For example, an uncertain bolus will be assumed to have been successful, while a suspend will be assumed to have failed.