diff --git a/plugins/sync/src/main/java/info/nightscout/plugins/sync/nsclient/services/NSClientService.kt b/plugins/sync/src/main/java/info/nightscout/plugins/sync/nsclient/services/NSClientService.kt index 6a88662054..aa2a2a4558 100644 --- a/plugins/sync/src/main/java/info/nightscout/plugins/sync/nsclient/services/NSClientService.kt +++ b/plugins/sync/src/main/java/info/nightscout/plugins/sync/nsclient/services/NSClientService.kt @@ -509,10 +509,44 @@ class NSClientService : DaggerService(), NsClient.NSClientService { val gson = GsonBuilder().also { it.registerTypeAdapter(JSONObject::class.java, deserializer) }.create() - val devicestatuses = gson.fromJson(data.getString("devicestatus"), Array::class.java) - if (devicestatuses.isNotEmpty()) { - rxBus.send(EventNSClientNewLog("DATA", "received " + devicestatuses.size + " device statuses")) - nsDeviceStatusHandler.handleNewData(devicestatuses) + + try { + // "Live-patch" the JSON data if the battery value is not an integer. + // This has caused crashes in the past due to parsing errors. See: + // https://github.com/nightscout/AndroidAPS/issues/2223 + // The reason is that the "battery" data type has been changed: + // https://github.com/NightscoutFoundation/xDrip/pull/1709 + // + // Since we cannot reliably derive an integer percentage out + // of an arbitrary string, we are forced to replace that string + // with a hardcoded percentage. That way, at least, the + // subsequent GSON parsing won't crash. + val devicestatusJsonArray = data.getJSONArray("devicestatus") + for (arrayIndex in 0 until devicestatusJsonArray.length()) { + val devicestatusObject = devicestatusJsonArray.getJSONObject(arrayIndex) + if (devicestatusObject.has("uploader")) { + val uploaderObject = devicestatusObject.getJSONObject("uploader") + if (uploaderObject.has("battery")) { + val batteryValue = uploaderObject["battery"] + if (batteryValue !is Int) { + aapsLogger.warn( + LTag.NSCLIENT, + "JSON devicestatus object #$arrayIndex (out of ${devicestatusJsonArray.length()}) " + + "has invalid value \"$batteryValue\" (expected integer); replacing with hardcoded integer 100" + ) + uploaderObject.put("battery", 100) + } + } + } + } + + val devicestatuses = gson.fromJson(data.getString("devicestatus"), Array::class.java) + if (devicestatuses.isNotEmpty()) { + rxBus.send(EventNSClientNewLog("DATA", "received " + devicestatuses.size + " device statuses")) + nsDeviceStatusHandler.handleNewData(devicestatuses) + } + } catch (e: JSONException) { + aapsLogger.error(LTag.NSCLIENT, "Skipping invalid Nightscout devicestatus data; exception: $e") } } if (data.has("food")) { @@ -666,4 +700,4 @@ class NSClientService : DaggerService(), NsClient.NSClientService { aapsLogger.debug(LTag.NSCLIENT, alarm.toString()) } } -} \ No newline at end of file +} diff --git a/pump/combov2/comboctl/src/commonMain/kotlin/info/nightscout/comboctl/main/Pump.kt b/pump/combov2/comboctl/src/commonMain/kotlin/info/nightscout/comboctl/main/Pump.kt index a4e403cd42..0507cfe41b 100644 --- a/pump/combov2/comboctl/src/commonMain/kotlin/info/nightscout/comboctl/main/Pump.kt +++ b/pump/combov2/comboctl/src/commonMain/kotlin/info/nightscout/comboctl/main/Pump.kt @@ -2587,10 +2587,12 @@ class Pump( val currentSystemTimeZone = TimeZone.currentSystemDefault() val currentSystemUtcOffset = currentSystemTimeZone.offsetAt(currentSystemDateTime) val dateTimeDelta = (currentSystemDateTime - currentPumpDateTime) + var needsPumpDateTimeAdjustment = false logger(LogLevel.DEBUG) { "History delta size: ${historyDelta.size}" } logger(LogLevel.DEBUG) { "Pump local datetime: $currentPumpLocalDateTime with UTC offset: $currentPumpDateTime" } logger(LogLevel.DEBUG) { "Current system datetime: $currentSystemDateTime" } + logger(LogLevel.DEBUG) { "Current system timezone: $currentSystemTimeZone" } logger(LogLevel.DEBUG) { "Datetime delta: $dateTimeDelta" } // The following checks update the UTC offset in the pump state and @@ -2605,10 +2607,6 @@ class Pump( // TBRs are not affected by this, because the TBR info we store in the // pump state is already stored as an Instant, so it stores the timezone // offset along with the actual timestamp. - // For the same reason, we *first* update the pump's datetime (if there - // is a deviation from the system datetime) and *then* update the UTC - // offset. The pump is still running with the localtime that is tied - // to the old UTC offset. // Check if the system's current datetime and the pump's are at least // 2 minutes apart. If so, update the pump's current datetime. @@ -2624,6 +2622,23 @@ class Pump( "system / pump datetime (UTC): $currentSystemDateTime / $currentPumpDateTime; " + "datetime delta: $dateTimeDelta" } + needsPumpDateTimeAdjustment = true + } + + // Check if the pump's current UTC offset matches that of the system. + + if (currentSystemUtcOffset != currentPumpUtcOffset!!) { + logger(LogLevel.INFO) { + "System UTC offset differs from pump's; system timezone: $currentSystemTimeZone; " + + "system UTC offset: $currentSystemUtcOffset; pump state UTC offset: ${currentPumpUtcOffset!!}; " + + "updating pump state and datetime" + } + pumpStateStore.setCurrentUtcOffset(bluetoothDevice.address, currentSystemUtcOffset) + currentPumpUtcOffset = currentSystemUtcOffset + needsPumpDateTimeAdjustment = true + } + + if (needsPumpDateTimeAdjustment) { // Shift the pump's new datetime into the future, using a simple // heuristic that estimates how long it will take updatePumpDateTime() // to complete the adjustment. If the difference between the pump's @@ -2635,30 +2650,20 @@ class Pump( // too far in the past. By estimating the updatePumpDateTime() // duration and taking it into account, we minimize the chances // of the pump's new datetime being too old already. + // NOTE: It is important that currentPumpDateTime has been produced + // with the _unadjusted_ UTC offset (in case the code above noticed + // a difference between system and pump UTC offsets and adjusted the + // latter to match the former). Otherwise, the estimates will be off. val newPumpDateTimeShift = estimateDateTimeSetDurationFrom(currentPumpDateTime, currentSystemDateTime, currentSystemTimeZone) - updatePumpDateTime( - (currentSystemDateTime + newPumpDateTimeShift).toLocalDateTime(currentSystemTimeZone) - ) + updatePumpDateTime((currentSystemDateTime + newPumpDateTimeShift).toLocalDateTime(currentSystemTimeZone)) } else { logger(LogLevel.INFO) { "Current system datetime is close enough to pump's current datetime, " + - "no pump datetime adjustment needed; " + + "and timezones did not change; no pump datetime adjustment needed; " + "system / pump datetime (UTC): $currentSystemDateTime / $currentPumpDateTime; " + "datetime delta: $dateTimeDelta" } } - - // Check if the pump's current UTC offset matches that of the system. - - if (currentSystemUtcOffset != currentPumpUtcOffset!!) { - logger(LogLevel.INFO) { - "System UTC offset differs from pump's; system timezone: $currentSystemTimeZone; " + - "system UTC offset: $currentSystemUtcOffset; pump state UTC offset: ${currentPumpUtcOffset!!}; " + - "updating pump state" - } - pumpStateStore.setCurrentUtcOffset(bluetoothDevice.address, currentSystemUtcOffset) - currentPumpUtcOffset = currentSystemUtcOffset - } } private suspend fun fetchHistoryDelta(): List { @@ -3024,7 +3029,7 @@ class Pump( calcLongRTButtonPressObservationPeriod(dayDistance) logger(LogLevel.DEBUG) { - "Current local pump / system datetime: $currentLocalPumpDateTime / $currentLocalSystemDateTime " + + "Current local pump / local system datetime: $currentLocalPumpDateTime / $currentLocalSystemDateTime " + "; estimated duration: $estimatedDuration" } diff --git a/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/ComboV2Plugin.kt b/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/ComboV2Plugin.kt index d540801bc1..5c0326d00c 100644 --- a/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/ComboV2Plugin.kt +++ b/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/ComboV2Plugin.kt @@ -46,6 +46,7 @@ import info.nightscout.interfaces.utils.DecimalFormatter import info.nightscout.interfaces.utils.TimeChangeType import info.nightscout.rx.bus.RxBus import info.nightscout.rx.events.EventDismissNotification +import info.nightscout.rx.events.EventInitializationChanged import info.nightscout.rx.events.EventOverviewBolusProgress import info.nightscout.rx.events.EventOverviewBolusProgress.Treatment import info.nightscout.rx.events.EventPumpStatusChanged @@ -138,6 +139,7 @@ class ComboV2Plugin @Inject constructor ( // These are initialized in onStart() and torn down in onStop(). private var bluetoothInterface: AndroidBluetoothInterface? = null private var pumpManager: ComboCtlPumpManager? = null + private var initializationChangedEventSent = false // These are initialized in connect() and torn down in disconnect(). private var pump: ComboCtlPump? = null @@ -155,6 +157,12 @@ class ComboV2Plugin @Inject constructor ( // state (in other words, while isBusy() was returning true). private var disconnectRequestPending = false + // Set to true in when unpair() starts and back to false in the + // pumpManager onPumpUnpaired callback. This fixes a race condition + // that can happen if the user unpairs the pump while AndroidAPS + // is calling connect(). + private var unpairing = false + // The current driver state. We use a StateFlow here to // allow other components to react to state changes. private val _driverStateFlow = MutableStateFlow(DriverState.NotInitialized) @@ -239,6 +247,7 @@ class ComboV2Plugin @Inject constructor ( pumpManager = ComboCtlPumpManager(bluetoothInterface!!, pumpStateStore) pumpManager!!.setup { _pairedStateUIFlow.value = false + unpairing = false } // UI flows that must have defined values right @@ -252,6 +261,8 @@ class ComboV2Plugin @Inject constructor ( _pairedStateUIFlow.value = paired setDriverState(DriverState.Disconnected) + + // NOTE: EventInitializationChanged is sent in getPumpStatus() . } override fun onStop() { @@ -268,8 +279,16 @@ class ComboV2Plugin @Inject constructor ( bluetoothInterface?.teardown() bluetoothInterface = null + // Set this flag to false here in case an ongoing pairing attempt + // is somehow aborted inside the interface without the onPumpUnpaired + // callback being invoked. + unpairing = false + setDriverState(DriverState.NotInitialized) + rxBus.send(EventInitializationChanged()) + initializationChangedEventSent = false + super.onStop() } @@ -295,11 +314,13 @@ class ComboV2Plugin @Inject constructor ( // Setup coroutine to enable/disable the pair and unpair // preferences depending on the pairing state. preferenceFragment.run { - // TODO: Verify that the lifecycle and coroutinescope are correct here. - // We want to avoid duplicate coroutine launches and premature coroutine terminations. - // The viewLifecycle does not work here since this is called before onCreateView() is, - // and it is questionable whether the viewLifecycle is even the one to use - verify - // that lifecycle instead of viewLifecycle is the correct choice. + // We use the fragment's lifecyle instead of the fragment view's, since the latter + // is initialized in onCreateView(), and we reach this point here _before_ that + // method is called. In other words, the fragment view does not exist at this point. + // repeatOnLifecycle() is a utility function that runs its block when the lifecycle + // starts. If the fragment is destroyed, the code inside - that is, the flow - is + // cancelled. That way, the UI flow is automatically reconstructed when Android + // recreates the fragment. lifecycle.coroutineScope.launch { lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { val pairPref: Preference? = findPreference(rh.gs(R.string.key_combov2_pair_with_pump)) @@ -343,9 +364,10 @@ class ComboV2Plugin @Inject constructor ( // NOTE: Even though the Combo is technically already connected by the // time the DriverState.CheckingPump state is reached, do not return // true then. That's because the pump still tries to issue commands - // during that state even though isBusy() returns true. Worse, it - // might try to call connect()! - // TODO: Check why this happens. + // during that state. isBusy() informs about the pump being busy during + // that state, but that function is not always called before commands + // are dispatched, so we announce to the queue thread that we aren't + // connected yet. DriverState.Ready, DriverState.Suspended, is DriverState.ExecutingCommand -> true @@ -366,6 +388,11 @@ class ComboV2Plugin @Inject constructor ( override fun connect(reason: String) { aapsLogger.debug(LTag.PUMP, "Connecting to Combo; reason: $reason") + if (unpairing) { + aapsLogger.debug(LTag.PUMP, "Aborting connect attempt since we are currently unpairing") + return + } + when (driverStateFlow.value) { DriverState.Connecting, DriverState.CheckingPump, @@ -618,6 +645,13 @@ class ComboV2Plugin @Inject constructor ( executeCommand { pump?.updateStatus() } + + // We send this event here, and not in onStart(), to include + // the initial pump status update before emitting the event. + if (!initializationChangedEventSent) { + rxBus.send(EventInitializationChanged()) + initializationChangedEventSent = true + } } catch (e: CancellationException) { throw e } catch (_: Exception) { @@ -858,10 +892,6 @@ class ComboV2Plugin @Inject constructor ( } reportFinishedBolus(rh.gs(R.string.bolus_delivered, detailedBolusInfo.insulin), pumpEnactResult, succeeded = true) - - // TODO: Check that an alert sound and error dialog - // are produced if an exception was thrown that - // counts as an error } catch (e: CancellationException) { // Cancellation is not an error, but it also means // that the profile update was not enacted. @@ -958,8 +988,19 @@ class ComboV2Plugin @Inject constructor ( val cctlTbrType = when (tbrType) { PumpSync.TemporaryBasalType.NORMAL -> ComboCtlTbr.Type.NORMAL PumpSync.TemporaryBasalType.EMULATED_PUMP_SUSPEND -> ComboCtlTbr.Type.EMULATED_COMBO_STOP - PumpSync.TemporaryBasalType.PUMP_SUSPEND -> ComboCtlTbr.Type.COMBO_STOPPED // TODO: Can this happen? It is currently not allowed by ComboCtlPump.setTbr() PumpSync.TemporaryBasalType.SUPERBOLUS -> ComboCtlTbr.Type.SUPERBOLUS + PumpSync.TemporaryBasalType.PUMP_SUSPEND -> { + aapsLogger.error( + LTag.PUMP, + "PUMP_SUSPEND TBR type produced by AAPS for the TBR initiation even though this is supposed to only be produced by pump drivers" + ) + pumpEnactResult.apply { + success = false + enacted = false + comment = rh.gs(R.string.error) + } + return pumpEnactResult + } } setTbrInternal(limitedPercentage, durationInMinutes, cctlTbrType, force100Percent = false, pumpEnactResult) @@ -978,8 +1019,19 @@ class ComboV2Plugin @Inject constructor ( val cctlTbrType = when (tbrType) { PumpSync.TemporaryBasalType.NORMAL -> ComboCtlTbr.Type.NORMAL PumpSync.TemporaryBasalType.EMULATED_PUMP_SUSPEND -> ComboCtlTbr.Type.EMULATED_COMBO_STOP - PumpSync.TemporaryBasalType.PUMP_SUSPEND -> ComboCtlTbr.Type.COMBO_STOPPED // TODO: Can this happen? It is currently not allowed by ComboCtlPump.setTbr() PumpSync.TemporaryBasalType.SUPERBOLUS -> ComboCtlTbr.Type.SUPERBOLUS + PumpSync.TemporaryBasalType.PUMP_SUSPEND -> { + aapsLogger.error( + LTag.PUMP, + "PUMP_SUSPEND TBR type produced by AAPS for the TBR initiation even though this is supposed to only be produced by pump drivers" + ) + pumpEnactResult.apply { + success = false + enacted = false + comment = rh.gs(R.string.error) + } + return pumpEnactResult + } } setTbrInternal(limitedPercentage, durationInMinutes, cctlTbrType, force100Percent = false, pumpEnactResult) @@ -988,11 +1040,6 @@ class ComboV2Plugin @Inject constructor ( } override fun cancelTempBasal(enforceNew: Boolean): PumpEnactResult { - // TODO: Check if some of the additional checks in ComboPlugin.cancelTempBasal can be carried over here. - // Note that ComboCtlPump.setTbr itself checks the TBR that is actually active after setting the TBR - // is done, and throws exceptions when there's a mismatch. It considers mismatches as an error, unlike - // the ComboPlugin.cancelTempBasal code, which just sets enact to false when there's a mismatch. - val pumpEnactResult = PumpEnactResult(injector) pumpEnactResult.isPercent = true pumpEnactResult.isTempCancel = enforceNew @@ -1124,13 +1171,7 @@ class ComboV2Plugin @Inject constructor ( "Cannot include base basal rate in JSON status " + "since no basal profile is currently active" ) - try { - // TODO: What about the profileName argument? - // Is it obsolete? - put("ActiveProfile", profileFunction.getProfileName()) - } catch (e: Exception) { - aapsLogger.error("Unhandled exception", e) - } + put("ActiveProfile", profileName) when (val alert = lastComboAlert) { is AlertScreenContent.Warning -> put("WarningCode", alert.code) @@ -1283,11 +1324,17 @@ class ComboV2Plugin @Inject constructor ( override fun canHandleDST() = true override fun timezoneOrDSTChanged(timeChangeType: TimeChangeType) { - // Currently just logging this; the ComboCtl.Pump code will set the new datetime - // (as localtime) as part of the on-connect checks automatically. - // TODO: It may be useful to do this here, since setting the datetime takes - // a while with the Combo. It has to be done via the RT mode, which is slow. aapsLogger.info(LTag.PUMP, "Time, Date and/or TimeZone changed. Time change type = $timeChangeType") + + val reason = when (timeChangeType) { + TimeChangeType.TimezoneChanged -> rh.gs(R.string.combov2_timezone_changed) + TimeChangeType.TimeChanged -> rh.gs(R.string.combov2_datetime_changed) + TimeChangeType.DSTStarted -> rh.gs(R.string.combov2_dst_started) + TimeChangeType.DSTEnded -> rh.gs(R.string.combov2_dst_ended) + } + // Updating pump status implicitly also updates the pump's local datetime, + // which is what we want after the system datetime/timezone/DST changed. + commandQueue.readStatus(reason, null) } /*** Loop constraints ***/ @@ -1405,8 +1452,13 @@ class ComboV2Plugin @Inject constructor ( } fun unpair() { + if (unpairing) + return + val bluetoothAddress = getBluetoothAddress() ?: return + unpairing = true + disconnectInternal(forceDisconnect = true) runBlocking { @@ -1436,6 +1488,9 @@ class ComboV2Plugin @Inject constructor ( _baseBasalRateUIFlow.value = null _serialNumberUIFlow.value = "" _bluetoothAddressUIFlow.value = "" + + // The unpairing variable is set to false in + // the PumpManager onPumpUnpaired callback. } @@ -1914,12 +1969,18 @@ class ComboV2Plugin @Inject constructor ( aapsLogger.info(LTag.PUMP, "Setting Combo driver state: old: $oldState new: $newState") - // TODO: Is it OK to send CONNECTED twice? It can happen when changing from Ready to Suspended. when (newState) { DriverState.Disconnected -> rxBus.send(EventPumpStatusChanged(EventPumpStatusChanged.Status.DISCONNECTED)) DriverState.Connecting -> rxBus.send(EventPumpStatusChanged(EventPumpStatusChanged.Status.CONNECTING)) - DriverState.Ready, - DriverState.Suspended -> rxBus.send(EventPumpStatusChanged(EventPumpStatusChanged.Status.CONNECTED)) + // Filter Ready<->Suspended state changes to avoid sending CONNECTED unnecessarily often. + DriverState.Ready -> { + if (oldState != DriverState.Suspended) + rxBus.send(EventPumpStatusChanged(EventPumpStatusChanged.Status.CONNECTED)) + } + DriverState.Suspended -> { + if (oldState != DriverState.Ready) + rxBus.send(EventPumpStatusChanged(EventPumpStatusChanged.Status.CONNECTED)) + } else -> Unit } } diff --git a/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/activities/ComboV2PairingActivity.kt b/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/activities/ComboV2PairingActivity.kt index 844253153a..61c72a3c95 100644 --- a/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/activities/ComboV2PairingActivity.kt +++ b/pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/activities/ComboV2PairingActivity.kt @@ -73,9 +73,12 @@ class ComboV2PairingActivity : DaggerAppCompatActivity() { // // xxx xxx xxxx binding.combov2PinEntryEdit.addTextChangedListener(object : TextWatcher { + var previousText = "" + override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) { - // Nothing needs to be done here; overridden method only exists - // to properly and fully implement the TextWatcher interface. + // Store the text as it is before the change. We need this + // to later determine if digits got added or removed. + previousText = binding.combov2PinEntryEdit.text.toString() } override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) { @@ -109,11 +112,44 @@ class ComboV2PairingActivity : DaggerAppCompatActivity() { // listener is called, listener changes text). binding.combov2PinEntryEdit.removeTextChangedListener(this) - // Shift the cursor position to skip the whitespaces. - val cursorPosition = when (val it = binding.combov2PinEntryEdit.selectionStart) { - 4 -> 5 - 8 -> 9 - else -> it + val trimmedPreviousText = previousText.trim().replace(nonDigitsRemovalRegex, "") + + // Shift the cursor position to skip the whitespaces. Distinguish between the cases + // when the user adds or removes a digit. In the former case, the trimmed version + // of the previous text is shorted than the trimmed current text. + var cursorPosition = if (trimmedPreviousText.length < trimmedText.length) + when (val it = binding.combov2PinEntryEdit.selectionStart) { + // In these cases, we shift the cursor position, since we just entered the + // first digit of the next digit group, and the input has been adjusted to + // automatically include a whitespace. For example, we already had entered + // digits "123". The user entered the fourth digit, yielding "1234". The + // code above turned this into "123 4". + 4, 8 -> + it + 1 + else -> + it + } + else + when (val it = binding.combov2PinEntryEdit.selectionStart) { + // Similar to the block above, but in reverse: At these positions, removing + // the digit will also remove the automatically inserted whitespace, so we + // have to skip that one. For example, previously, the text on screen was + // "123 4", now we press backspace, and get "123 ". The code above turns + // this into "123". + 4, 8 -> + it - 1 + else -> + it + } + + // Failsafe in case the calculations above are off for some reason. This is not + // clean; however, it is better than letting all of AndroidAPS crash. + if (cursorPosition > processedText.length) { + aapsLogger.warn( + LTag.PUMP, + "Incorrect cursor position $cursorPosition (processed text length ${processedText.length}); fixing" + ) + cursorPosition = processedText.length } binding.combov2PinEntryEdit.setText(processedText) diff --git a/pump/combov2/src/main/res/values/strings.xml b/pump/combov2/src/main/res/values/strings.xml index 51d7d2c2b7..a471e6e0fb 100644 --- a/pump/combov2/src/main/res/values/strings.xml +++ b/pump/combov2/src/main/res/values/strings.xml @@ -129,4 +129,8 @@ buttons at the same time to cancel pairing)\n Autodetect and automatically enter battery change Insulin reservoir change inserted automatically by combov2 driver Battery change inserted automatically by combov2 driver + Timezone changed + Date and/or time changed + Daylight savings time (DST) started + Daylight savings time (DST) ended