Improve SMSCommunicator safety

app/src/main/java/info/nightscout/androidaps/plugins/general/smsCommunicator/AuthRequest.kt

@@ -1,13 +1,16 @@
 package info.nightscout.androidaps.plugins.general.smsCommunicator
 
+import android.os.SystemClock
 import dagger.android.HasAndroidInjector
 import info.nightscout.androidaps.Constants
 import info.nightscout.androidaps.R
+import info.nightscout.androidaps.interfaces.CommandQueueProvider
 import info.nightscout.androidaps.logging.AAPSLogger
 import info.nightscout.androidaps.logging.LTag
-import info.nightscout.androidaps.utils.DateUtil
 import info.nightscout.androidaps.plugins.general.smsCommunicator.otp.OneTimePassword
 import info.nightscout.androidaps.plugins.general.smsCommunicator.otp.OneTimePasswordValidationResult
+import info.nightscout.androidaps.utils.DateUtil
+import info.nightscout.androidaps.utils.T
 import info.nightscout.androidaps.utils.resources.ResourceHelper
 import javax.inject.Inject
 
@@ -23,6 +26,7 @@ class AuthRequest internal constructor(
     @Inject lateinit var resourceHelper: ResourceHelper
     @Inject lateinit var otp: OneTimePassword
     @Inject lateinit var dateUtil: DateUtil
+    @Inject lateinit var commandQueue: CommandQueueProvider
 
     private var date = 0L
     private var processed = false
@@ -49,6 +53,19 @@ class AuthRequest internal constructor(
         }
         if (dateUtil.now() - date < Constants.SMS_CONFIRM_TIMEOUT) {
             processed = true
+            if (action.pumpCommand) {
+                val start = dateUtil.now()
+                //wait for empty queue
+                while (start + T.mins(3).msecs() > dateUtil.now()) {
+                    if (commandQueue.size() == 0) break
+                    SystemClock.sleep(100)
+                }
+                if (commandQueue.size() != 0) {
+                    aapsLogger.debug(LTag.SMS, "Command timed out: " + requester.text)
+                    smsCommunicatorPlugin.sendSMS(Sms(requester.phoneNumber, resourceHelper.gs(R.string.sms_timeout_while_wating)))
+                    return
+                }
+            }
             aapsLogger.debug(LTag.SMS, "Processing confirmed SMS: " + requester.text)
             action.run()
             return

app/src/main/java/info/nightscout/androidaps/plugins/general/smsCommunicator/SmsAction.kt

@@ -1,37 +1,37 @@
 package info.nightscout.androidaps.plugins.general.smsCommunicator
 
-abstract class SmsAction : Runnable {
+abstract class SmsAction(val pumpCommand: Boolean) : Runnable {
+
     var aDouble: Double? = null
     var anInteger: Int? = null
     var secondInteger: Int? = null
     var secondLong: Long? = null
     var aString: String? = null
 
-    internal constructor()
+    internal constructor(pumpCommand: Boolean, aDouble: Double) : this(pumpCommand) {
-    internal constructor(aDouble: Double) {
         this.aDouble = aDouble
     }
 
-    internal constructor(aDouble: Double, secondInteger: Int) {
+    internal constructor(pumpCommand: Boolean, aDouble: Double, secondInteger: Int) : this(pumpCommand) {
         this.aDouble = aDouble
         this.secondInteger = secondInteger
     }
 
-    internal constructor(aString: String, secondInteger: Int) {
+    internal constructor(pumpCommand: Boolean, aString: String, secondInteger: Int) : this(pumpCommand) {
         this.aString = aString
         this.secondInteger = secondInteger
     }
 
-    internal constructor(anInteger: Int) {
+    internal constructor(pumpCommand: Boolean, anInteger: Int) : this(pumpCommand) {
         this.anInteger = anInteger
     }
 
-    internal constructor(anInteger: Int, secondInteger: Int) {
+    internal constructor(pumpCommand: Boolean, anInteger: Int, secondInteger: Int) : this(pumpCommand) {
         this.anInteger = anInteger
         this.secondInteger = secondInteger
     }
 
-    internal constructor(anInteger: Int, secondLong: Long) {
+    internal constructor(pumpCommand: Boolean, anInteger: Int, secondLong: Long) : this(pumpCommand) {
         this.anInteger = anInteger
         this.secondLong = secondLong
     }

app/src/main/java/info/nightscout/androidaps/plugins/general/smsCommunicator/SmsCommunicatorPlugin.kt

@@ -94,8 +94,8 @@ class SmsCommunicatorPlugin @Inject constructor(
 
     private val disposable = CompositeDisposable()
     var allowedNumbers: MutableList<String> = ArrayList()
-    var messageToConfirm: AuthRequest? = null
+    @Volatile var messageToConfirm: AuthRequest? = null
-    var lastRemoteBolusTime: Long = 0
+    @Volatile var lastRemoteBolusTime: Long = 0
     var messages = ArrayList<Sms>()
 
     val commands = mapOf(
@@ -278,6 +278,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                     else sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.wrongformat)))
                 "BOLUS"    ->
                     if (!remoteCommandsAllowed) sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.smscommunicator_remotecommandnotallowed)))
+                    else if (commandQueue.bolusInQueue()) sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.smscommunicator_another_bolus_in_queue)))
                     else if (divided.size == 2 && dateUtil.now() - lastRemoteBolusTime < minDistance) sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.smscommunicator_remotebolusnotallowed)))
                     else if (divided.size == 2 && pump.isSuspended()) sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.pumpsuspended)))
                     else if (divided.size == 2 || divided.size == 3) processBOLUS(divided, receivedSms)
@@ -303,9 +304,13 @@ class SmsCommunicatorPlugin @Inject constructor(
                     else sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.wrongformat)))
                 else       ->
                     if (messageToConfirm?.requester?.phoneNumber == receivedSms.phoneNumber) {
-                        messageToConfirm?.action(divided[0])
+                        val execute = messageToConfirm
                         messageToConfirm = null
-                    } else sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.smscommunicator_unknowncommand)))
+                        execute?.action(divided[0])
+                    } else {
+                        messageToConfirm = null
+                        sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.smscommunicator_unknowncommand)))
+                    }
             }
         }
         rxBus.send(EventSmsCommunicatorUpdateGui())
@@ -344,7 +349,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                     val passCode = generatePassCode()
                     val reply = String.format(resourceHelper.gs(R.string.smscommunicator_loopdisablereplywithcode), passCode)
                     receivedSms.processed = true
-                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = false) {
                         override fun run() {
                             uel.log(Action.LOOP_DISABLED, Sources.SMS)
                             loop.enabled = false
@@ -368,10 +373,10 @@ class SmsCommunicatorPlugin @Inject constructor(
                     val passCode = generatePassCode()
                     val reply = String.format(resourceHelper.gs(R.string.smscommunicator_loopenablereplywithcode), passCode)
                     receivedSms.processed = true
-                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = false) {
                         override fun run() {
                             uel.log(Action.LOOP_ENABLED, Sources.SMS)
-                            loop.enabled= true
+                            loop.enabled = true
                             sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.smscommunicator_loophasbeenenabled)))
                             rxBus.send(EventRefreshOverview("SMS_LOOP_START"))
                         }
@@ -395,7 +400,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                 val passCode = generatePassCode()
                 val reply = String.format(resourceHelper.gs(R.string.smscommunicator_loopresumereplywithcode), passCode)
                 receivedSms.processed = true
-                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true) {
                     override fun run() {
                         uel.log(Action.RESUME, Sources.SMS)
                         disposable += repository.runTransactionForResult(CancelCurrentOfflineEventIfAnyTransaction(dateUtil.now()))
@@ -432,7 +437,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                     val passCode = generatePassCode()
                     val reply = String.format(resourceHelper.gs(R.string.smscommunicator_suspendreplywithcode), duration, passCode)
                     receivedSms.processed = true
-                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(duration) {
+                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, duration) {
                         override fun run() {
                             uel.log(Action.SUSPEND, Sources.SMS)
                             commandQueue.cancelTempBasal(true, object : Callback() {
@@ -478,7 +483,7 @@ class SmsCommunicatorPlugin @Inject constructor(
     @kotlin.ExperimentalStdlibApi
     private fun processHELP(divided: Array<String>, receivedSms: Sms) {
         when {
-            divided.size == 1                                                               -> {
+            divided.size == 1                                                             -> {
                 sendSMS(Sms(receivedSms.phoneNumber, commands.keys.toString().replace("[", "").replace("]", "")))
                 receivedSms.processed = true
             }
@@ -490,7 +495,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                 }
             }
 
-            else                                                                            -> sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.wrongformat)))
+            else                                                                          -> sendSMS(Sms(receivedSms.phoneNumber, resourceHelper.gs(R.string.wrongformat)))
         }
     }
 
@@ -513,7 +518,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_pumpconnectwithcode), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true) {
                 override fun run() {
                     uel.log(Action.RECONNECT, Sources.SMS)
                     commandQueue.cancelTempBasal(true, object : Callback() {
@@ -546,7 +551,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                 val passCode = generatePassCode()
                 val reply = String.format(resourceHelper.gs(R.string.smscommunicator_pumpdisconnectwithcode), duration, passCode)
                 receivedSms.processed = true
-                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true) {
                     override fun run() {
                         uel.log(Action.DISCONNECT, Sources.SMS)
                         val profile = profileFunction.getProfile() ?: return
@@ -601,7 +606,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                     val reply = String.format(resourceHelper.gs(R.string.smscommunicator_profilereplywithcode), list[pIndex - 1], percentage, passCode)
                     receivedSms.processed = true
                     val finalPercentage = percentage
-                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(list[pIndex - 1] as String, finalPercentage) {
+                    messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, list[pIndex - 1] as String, finalPercentage) {
                         override fun run() {
                             profileFunction.createProfileSwitch(store, list[pIndex - 1] as String, 0, finalPercentage, 0, dateUtil.now())
                             val replyText = resourceHelper.gs(R.string.profileswitchcreated)
@@ -622,7 +627,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_basalstopreplywithcode), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true) {
                 override fun run() {
                     commandQueue.cancelTempBasal(true, object : Callback() {
                         override fun run() {
@@ -657,7 +662,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                 val passCode = generatePassCode()
                 val reply = String.format(resourceHelper.gs(R.string.smscommunicator_basalpctreplywithcode), tempBasalPct, duration, passCode)
                 receivedSms.processed = true
-                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(tempBasalPct, duration) {
+                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, tempBasalPct, duration) {
                     override fun run() {
                         commandQueue.tempBasalPercent(anInteger(), secondInteger(), true, profile, PumpSync.TemporaryBasalType.NORMAL, object : Callback() {
                             override fun run() {
@@ -701,7 +706,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                 val passCode = generatePassCode()
                 val reply = String.format(resourceHelper.gs(R.string.smscommunicator_basalreplywithcode), tempBasal, duration, passCode)
                 receivedSms.processed = true
-                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(tempBasal, duration) {
+                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, tempBasal, duration) {
                     override fun run() {
                         commandQueue.tempBasalAbsolute(aDouble(), secondInteger(), true, profile, PumpSync.TemporaryBasalType.NORMAL, object : Callback() {
                             override fun run() {
@@ -739,7 +744,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_extendedstopreplywithcode), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true) {
                 override fun run() {
                     commandQueue.cancelExtended(object : Callback() {
                         override fun run() {
@@ -769,7 +774,7 @@ class SmsCommunicatorPlugin @Inject constructor(
                 val passCode = generatePassCode()
                 val reply = String.format(resourceHelper.gs(R.string.smscommunicator_extendedreplywithcode), extended, duration, passCode)
                 receivedSms.processed = true
-                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(extended, duration) {
+                messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, extended, duration) {
                     override fun run() {
                         commandQueue.extendedBolus(aDouble(), secondInteger(), object : Callback() {
                             override fun run() {
@@ -815,7 +820,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             else
                 String.format(resourceHelper.gs(R.string.smscommunicator_bolusreplywithcode), bolus, passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(bolus) {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, bolus) {
                 override fun run() {
                     val detailedBolusInfo = DetailedBolusInfo()
                     detailedBolusInfo.insulin = aDouble()
@@ -917,7 +922,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_carbsreplywithcode), grams, dateUtil.timeString(time), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(grams, time) {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = true, grams, time) {
                 override fun run() {
                     val detailedBolusInfo = DetailedBolusInfo()
                     detailedBolusInfo.carbs = anInteger().toDouble()
@@ -954,7 +959,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_temptargetwithcode), divided[1].uppercase(Locale.getDefault()), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = false) {
                 override fun run() {
                     val units = profileFunction.getUnits()
                     var keyDuration = 0
@@ -1016,7 +1021,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_temptargetcancel), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = false) {
                 override fun run() {
                     disposable += repository.runTransactionForResult(CancelCurrentTemporaryTargetIfAnyTransaction(dateUtil.now()))
                         .subscribe({ result ->
@@ -1041,7 +1046,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_stopsmswithcode), passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction() {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = false) {
                 override fun run() {
                     sp.putBoolean(R.string.key_smscommunicator_remotecommandsallowed, false)
                     val replyText = String.format(resourceHelper.gs(R.string.smscommunicator_stoppedsms))
@@ -1059,7 +1064,7 @@ class SmsCommunicatorPlugin @Inject constructor(
             val passCode = generatePassCode()
             val reply = String.format(resourceHelper.gs(R.string.smscommunicator_calibrationreplywithcode), cal, passCode)
             receivedSms.processed = true
-            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(cal) {
+            messageToConfirm = AuthRequest(injector, receivedSms, reply, passCode, object : SmsAction(pumpCommand = false, cal) {
                 override fun run() {
                     val result = xdripCalibrations.sendIntent(aDouble!!)
                     val replyText =

app/src/main/res/values/strings.xml

@@ -1153,5 +1153,7 @@
     <string name="key_ns_receive_cgm" translatable="false">ns_receive_cgm</string>
     <string name="ns_receive_cgm">Receive/backfill CGM data</string>
     <string name="ns_receive_cgm_summary">Accept CGM data from NS</string>
+    <string name="sms_timeout_while_wating">Timeout while waiting for finish of previous pump communication</string>
+    <string name="smscommunicator_another_bolus_in_queue">There is another bolus in queue. Try again later.</string>
 
 </resources>

app/src/test/java/info/nightscout/androidaps/plugins/general/smsCommunicator/AuthRequestTest.kt

@@ -56,7 +56,7 @@ class AuthRequestTest : TestBase() {
 
     @Test fun doTests() {
         val requester = Sms("aNumber", "aText")
-        val action: SmsAction = object : SmsAction() {
+        val action: SmsAction = object : SmsAction(false) {
             override fun run() {
                 actionCalled = true
             }

app/src/test/java/info/nightscout/androidaps/plugins/general/smsCommunicator/SmsActionTest.kt

@@ -10,14 +10,14 @@ class SmsActionTest {
 
     var result = ""
     @Test fun doTests() {
-        var smsAction: SmsAction = object : SmsAction() {
+        var smsAction: SmsAction = object : SmsAction(false) {
             override fun run() {
                 result = "A"
             }
         }
         smsAction.run()
         Assert.assertEquals(result, "A")
-        smsAction = object : SmsAction(1.0) {
+        smsAction = object : SmsAction(false, 1.0) {
             override fun run() {
                 result = "B"
             }
@@ -25,7 +25,7 @@ class SmsActionTest {
         smsAction.run()
         Assert.assertEquals(result, "B")
         Assert.assertEquals(smsAction.aDouble(), 1.0, 0.000001)
-        smsAction = object : SmsAction(1.0, 2) {
+        smsAction = object : SmsAction(false, 1.0, 2) {
             override fun run() {
                 result = "C"
             }
@@ -34,7 +34,7 @@ class SmsActionTest {
         Assert.assertEquals(result, "C")
         Assert.assertEquals(smsAction.aDouble(), 1.0, 0.000001)
         Assert.assertEquals(smsAction.secondInteger().toLong(), 2)
-        smsAction = object : SmsAction("aString", 3) {
+        smsAction = object : SmsAction(false, "aString", 3) {
             override fun run() {
                 result = "D"
             }
@@ -43,7 +43,7 @@ class SmsActionTest {
         Assert.assertEquals(result, "D")
         Assert.assertEquals(smsAction.aString(), "aString")
         Assert.assertEquals(smsAction.secondInteger().toLong(), 3)
-        smsAction = object : SmsAction(4) {
+        smsAction = object : SmsAction(false, 4) {
             override fun run() {
                 result = "E"
             }
@@ -51,7 +51,7 @@ class SmsActionTest {
         smsAction.run()
         Assert.assertEquals(result, "E")
         Assert.assertEquals(smsAction.anInteger().toLong(), 4)
-        smsAction = object : SmsAction(5, 6) {
+        smsAction = object : SmsAction(false, 5, 6) {
             override fun run() {
                 result = "F"
             }

app/src/test/java/info/nightscout/androidaps/plugins/general/smsCommunicator/SmsCommunicatorPluginTest.kt

@@ -86,6 +86,7 @@ class SmsCommunicatorPluginTest : TestBaseWithProfile() {
                 it.resourceHelper = resourceHelper
                 it.otp = otp
                 it.dateUtil = dateUtil
+                it.commandQueue = commandQueue
             }
         }
     }
@@ -262,7 +263,7 @@ class SmsCommunicatorPluginTest : TestBaseWithProfile() {
         Assert.assertTrue(smsCommunicatorPlugin.isCommand("BOLUS", ""))
         smsCommunicatorPlugin.messageToConfirm = null
         Assert.assertFalse(smsCommunicatorPlugin.isCommand("BLB", ""))
-        smsCommunicatorPlugin.messageToConfirm = AuthRequest(injector, Sms("1234", "ddd"), "RequestText", "ccode", object : SmsAction() {
+        smsCommunicatorPlugin.messageToConfirm = AuthRequest(injector, Sms("1234", "ddd"), "RequestText", "ccode", object : SmsAction(false) {
             override fun run() {}
         })
         Assert.assertTrue(smsCommunicatorPlugin.isCommand("BLB", "1234"))