combov2: Remove obsolete TODOs and clarify others

Signed-off-by: Carlos Rafael Giani <crg7475@mailbox.org>
This commit is contained in:
Carlos Rafael Giani 2022-11-26 23:04:41 +01:00
parent 999321b495
commit 2d7b9b5d81

View file

@ -307,11 +307,13 @@ class ComboV2Plugin @Inject constructor (
// Setup coroutine to enable/disable the pair and unpair // Setup coroutine to enable/disable the pair and unpair
// preferences depending on the pairing state. // preferences depending on the pairing state.
preferenceFragment.run { preferenceFragment.run {
// TODO: Verify that the lifecycle and coroutinescope are correct here. // We use the fragment's lifecyle instead of the fragment view's, since the latter
// We want to avoid duplicate coroutine launches and premature coroutine terminations. // is initialized in onCreateView(), and we reach this point here _before_ that
// The viewLifecycle does not work here since this is called before onCreateView() is, // method is called. In other words, the fragment view does not exist at this point.
// and it is questionable whether the viewLifecycle is even the one to use - verify // repeatOnLifecycle() is a utility function that runs its block when the lifecycle
// that lifecycle instead of viewLifecycle is the correct choice. // 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.coroutineScope.launch {
lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
val pairPref: Preference? = findPreference(rh.gs(R.string.key_combov2_pair_with_pump)) val pairPref: Preference? = findPreference(rh.gs(R.string.key_combov2_pair_with_pump))
@ -355,9 +357,10 @@ class ComboV2Plugin @Inject constructor (
// NOTE: Even though the Combo is technically already connected by the // NOTE: Even though the Combo is technically already connected by the
// time the DriverState.CheckingPump state is reached, do not return // time the DriverState.CheckingPump state is reached, do not return
// true then. That's because the pump still tries to issue commands // true then. That's because the pump still tries to issue commands
// during that state even though isBusy() returns true. Worse, it // during that state. isBusy() informs about the pump being busy during
// might try to call connect()! // that state, but that function is not always called before commands
// TODO: Check why this happens. // are dispatched, so we announce to the queue thread that we aren't
// connected yet.
DriverState.Ready, DriverState.Ready,
DriverState.Suspended, DriverState.Suspended,
is DriverState.ExecutingCommand -> true is DriverState.ExecutingCommand -> true
@ -875,10 +878,6 @@ class ComboV2Plugin @Inject constructor (
} }
reportFinishedBolus(rh.gs(R.string.bolus_delivered, detailedBolusInfo.insulin), pumpEnactResult, succeeded = true) 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) { } catch (e: CancellationException) {
// Cancellation is not an error, but it also means // Cancellation is not an error, but it also means
// that the profile update was not enacted. // that the profile update was not enacted.
@ -1005,11 +1004,6 @@ class ComboV2Plugin @Inject constructor (
} }
override fun cancelTempBasal(enforceNew: Boolean): PumpEnactResult { 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) val pumpEnactResult = PumpEnactResult(injector)
pumpEnactResult.isPercent = true pumpEnactResult.isPercent = true
pumpEnactResult.isTempCancel = enforceNew pumpEnactResult.isTempCancel = enforceNew
@ -1302,8 +1296,6 @@ class ComboV2Plugin @Inject constructor (
override fun timezoneOrDSTChanged(timeChangeType: TimeChangeType) { override fun timezoneOrDSTChanged(timeChangeType: TimeChangeType) {
// Currently just logging this; the ComboCtl.Pump code will set the new datetime // Currently just logging this; the ComboCtl.Pump code will set the new datetime
// (as localtime) as part of the on-connect checks automatically. // (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") aapsLogger.info(LTag.PUMP, "Time, Date and/or TimeZone changed. Time change type = $timeChangeType")
} }