Major cleanup and some new features.

* PumpState.timestamp: round to second (format used in DB)
* Raise notification about wrong pump clock time since setting
  clock isn't possible with current ruffy
* Set TempBasal.pumpId when setting/cancelling TBR
* Checking state of pump on connect
* Better checks whether pump is ready to execute command
* Rework dynamic command timeout
* Confirm benign warnings on connect and turn into notifications
* Some groundwork for reading/setting basal profile
* Check pump history every 15m

TODO-Combo.md

@@ -1,5 +1,5 @@
-- [ ] Bugs
+- [x] Bugs
-  - [ ] No connection can be established anymore
+  - [-] No connection can be established anymore; ruffy issue i can't solve
     - Removing the BT device's bonding (!=pairing) fixes it; nope it doesn't
     - Ruffy logs in BTConnection:163  handler.fail("no connection possible: " + e.getMessage());
     - When developing (and thus killing/restarting AAPS often) this is trigger more frequently, leaving
@@ -8,7 +8,7 @@
   - [x] Bolus deleted in treatments  (marked invalid?!) is re-added when pump reads history
         Probably fixed through other bugfixes, irrelevant though as "pump history records" can't
         be deleted in AAPS
-  - [ ] Issue of creating TBR start date from main menu time, which might be off by a minute
+  - [x] Issue of creating TBR start date from main menu time, which might be off by a minute
         when we read it as a history record. End date time might be slightly off, unless
          CancelTempBasal is updated to read from history (probably not worth it).
          What would happen if while setting the TBR the start time was 12:01 but then
@@ -16,21 +16,33 @@
          Would the former be trimmed to 1m? That'd be acceptable (this edge case occurs
          if between confirm the TBR and reading the main menu date, the second goes
          from 59.9999 to 0)
+        Only in issue if TBR was set on pump. In that case the TBR is cancelled and the
+        resulting history record is read
 - [ ] Tasks
-  - [ ] Main
+  - [x] Main
-    - [ ] on command error: recover by returning to main menu
+    - [x] On command error: recover by returning to main menu
-    - [ ] Taking over alerts
+          check entry and exit points of commands
-      - [ ] On connect
+    - [x] Taking over alerts
-      - [ ] During bolusing
+      - [x] On connect
-      - [ ] Check for errors first thing in runCommand? Whenever analysing a CommandResult?
+      - [x] During bolusing
-    - [ ] Updating time on pump
+        - Can the low warning be set as high as 280 or so? To be able to trigger it with a quick refill? yup.
-      - [ ] Ruffy: support reading date/time menus
+      - [x] Check for errors first thing in runCommand? Whenever analysing a CommandResult?
-    - [ ] Setting pump basal profile
+      - [x] forward all warnings and errors encountered, but only confirm benign ones
-    - [ ] Pairing
+    - [-] Properly reporting back failures in UI, maybe warn if lots of errors (unreachable alert might
-    - [ ] Check dynamic timeout logic in RuffyScripter.runCommand
+          already be enough, since it's based on 'lastSuccessfulConnection', where a connection is
-    - [ ] Run readReservoirAndBolusLevel after SetTbr too so boluses on the pump are caught sooner?
+          considered successful if the command during that connection succeeded.
+          KeepAlive triggered check suffices.
+    - [x] Finish ComboPlugin structure to only have to plug date setting and pump setting in later
+          (actually, just use stub methods)
+    - [-] Updating time on pump
+      - [x] Raise a warning if time clock is off
+      - [-] Ruffy: support reading date/time menus
+    - [-] Setting pump basal profile (20h)
+    - [-] Pairing (and sourcing ruffy) (20h)
+    - [x] Run readReservoirAndBolusLevel after SetTbr too so boluses on the pump are caught sooner?
           Currently the pump gets to know such a record when bolusing or when refresh() is called
           after 15m of no other command taking place. IOB will then be current with next loop
+          checkPumpHistory is now called every 15m the least after executing a command
     - [x] Reading history
       - [x] Bolus
         - [x] Read
@@ -50,10 +62,12 @@
         - [x] Display in UI
     - [x] Optimize reading full history to pass timestamps of last known records to avoid reading known records
           iteration.
-  - [ ] Cleanups
+  - [x] Cleanups
-    - [ ] Finish 'enacted' removal rewrite (esp. cancel tbr)
+    - [x] TBR cancel logic
-    - [ ] ComboPlugin, commands invocation, checks, upadting combo store/cache
+    - [x] Check dynamic timeout logic in RuffyScripter.runCommand
-    - [ ] Finish reconnect, then start testing regular actions are still stable?
+    - [x] Finish 'enacted' removal rewrite (esp. cancel tbr)
+    - [x] ComboPlugin, commands invocation, checks, upadting combo store/cache
+    - [x] Finish reconnect
   - [x] Adrian says: when changing time; last treatments timestamp is  updated??
     - Nope, at least not with a 2014 pump (SW1.06?)
   - [x] Reconnect and auto-retry for commands
@@ -61,7 +75,7 @@
   - [ ] Integrate alarms
     - [x] Remove combo alerter thread
     - [ ] Fix display of alarms on mainscreen (increase height if needed)
-    - [ ] Display errors in combo tab(?)
+    - [-] Display errors in combo tab(?), nope notifications are better suited; also there's the alerts thing already
     - [x] Option to raise overview notifications as android notification with noise (for urgent ones?)
   - [ ] Low prio
     - [ ] Naming is messed up: pump has warnings and errors, which cause alerts; W+E are thus alerts,
@@ -72,36 +86,3 @@
         - Application shut down is broken with PersistentNotification (never shut down) and WearPlugin -
           Android logs it as crashed and restarts it, thereby restarting the app (or just keeping it alive,
           also causes errors with the DB as there were attemtps to open a closed DB instance/ref.
-
-Inbox
-  - [ ] Date syncing
-        If a bolus is given with a wrong time set (while AAPS is not active), then setting
-        time will result in that being in the past and not being detected as active.
-        Read bolus history before setting time and work with relative time to construct
-        actual bolus time? Will that fuck up syncing after setting time? Will the bolus be
-        counted twice (if time correcting was maybe an hour, e.g. daylight saving time switch,
-        resulting in an incorrectly too high IOB (too high might be tolerable in such a rare
-        circumstance)
-  - [ ] Where/when to call checkTbrMisMatch?
-  - [ ] pairing: just start SetupFragment?
-  - [ ] Read history, change time, check if bolus records changed
-  - [ ] Updating clock on pump; see how danar does it
-    - [ ] Update if mins are of by >=2m, also check/update if history has no bolus within the last 24h
-  - [ ] Wakelocks? Never needded them so far ...
-  - [ ] Finish Ruffyscripter simplification
-    - [x] Only reconnect on interruption, return after confirm error.
-    - [x] Bolus: do not check anything. Just bolus and abort. Checking history is done by CP
-    - [ ] TBR: check in command or CP as well?
-    - [ ] Generally, check the command was executed to the end and the MAINMENU was shown afterwards
-  - [ ] General error handling: have RS report back an alert if not confirmed within 10s (since commands can confirm them)
-  - [ ] forced sync after error
-  - [ ] Comm errors
-    - [ ] Retry? The occassional error, but have a treshold to alarm when e.g. 4 of 5 actions fail?
-  - [ ] Reading history, updating DB from it
-    - [ ] Detecting out of sync
-  - [ ] Pump unreachable alert
-    - [ ] Add option "Raise as android notification as well" like the "Forward overview screen messages to wear"?
-  - [ ] Pump warnings
-  - [ ] When suspended, AAPS won't try to read state again? (not tried 30m) should it? currently user must unsuspend in AAPS
-  - [ ] 'last error' in fragment? warning if error rate > 50%?
-  - [ ] How much noise to make when there are errors? Try to kill errors on the pump. If there are persistent issues, alert only after 20m, like xdrip does with missed readings?

app/src/main/java/info/nightscout/androidaps/plugins/PumpCombo/ComboFragment.java

@@ -4,7 +4,6 @@ package info.nightscout.androidaps.plugins.PumpCombo;
 import android.app.Activity;
 import android.graphics.Color;
 import android.os.Bundle;
-import android.support.v4.app.FragmentManager;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -13,9 +12,6 @@ import android.widget.TextView;
 
 import com.squareup.otto.Subscribe;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import de.jotomo.ruffy.spi.PumpState;
 import de.jotomo.ruffy.spi.history.Bolus;
 import info.nightscout.androidaps.R;
@@ -23,7 +19,6 @@ import info.nightscout.androidaps.plugins.Common.SubscriberFragment;
 import info.nightscout.androidaps.plugins.PumpCombo.events.EventComboPumpUpdateGUI;
 import info.nightscout.utils.DateUtil;
 
-// TODO clean up time/date formatting once this stabilizes a bit more
 public class ComboFragment extends SubscriberFragment implements View.OnClickListener, View.OnLongClickListener {
     private TextView stateView;
     private TextView activityView;
@@ -99,7 +94,7 @@ public class ComboFragment extends SubscriberFragment implements View.OnClickLis
                 ComboPlugin plugin = ComboPlugin.getPlugin();
 
                 // state
-                stateView.setText(plugin.getStateSummary()); // todo: inline, coupled with colour
+                stateView.setText(plugin.getStateSummary());
                 PumpState ps = plugin.getPump().state;
                 if (ps.insulinState == PumpState.EMPTY || ps.batteryState == PumpState.EMPTY) {
                     stateView.setTextColor(Color.RED);
@@ -139,14 +134,14 @@ public class ComboFragment extends SubscriberFragment implements View.OnClickLis
                     }
 
                     // last connection
-                    String minAgo = DateUtil.minAgo(plugin.getPump().lastSuccessfulConnection);
+                    String minAgo = DateUtil.minAgo(plugin.getPump().lastSuccessfulCmdTime);
-                    if (plugin.getPump().lastSuccessfulConnection + 60 * 1000 > System.currentTimeMillis()) {
+                    if (plugin.getPump().lastSuccessfulCmdTime + 60 * 1000 > System.currentTimeMillis()) {
                         lastConnectionView.setText(R.string.combo_pump_connected_now);
                         lastConnectionView.setTextColor(Color.WHITE);
-                    } else if (plugin.getPump().lastSuccessfulConnection < System.currentTimeMillis() - 30 * 60 * 1000) {
+                    } else if (plugin.getPump().lastSuccessfulCmdTime + 30 * 60 * 1000 < System.currentTimeMillis()) {
                         lastConnectionView.setText(getString(R.string.combo_no_pump_connection, minAgo));
                         lastConnectionView.setTextColor(Color.RED);
-                    } else if (plugin.getPump().lastConnectionAttempt > plugin.getPump().lastSuccessfulConnection) {
+                    } else if (plugin.getPump().lastCmdTime > plugin.getPump().lastSuccessfulCmdTime) {
                         String lastFailed = minAgo + "\n" + getString(R.string.combo_connect_attempt_failed);
                         lastConnectionView.setText(lastFailed);
                         lastConnectionView.setTextColor(Color.YELLOW);
@@ -161,6 +156,7 @@ public class ComboFragment extends SubscriberFragment implements View.OnClickLis
                         long agoMsc = System.currentTimeMillis() - bolus.timestamp;
                         double bolusMinAgo = agoMsc / 60d / 1000d;
                         double bolusHoursAgo = agoMsc / 60d / 60d / 1000d;
+                        // TODO i18n
                         if ((agoMsc < 60 * 1000)) {
                             lastBolusView.setText(String.format("%.1f U (now)", bolus.amount, (int) bolusMinAgo));
                         } else if (bolusMinAgo < 60) {
@@ -186,11 +182,6 @@ public class ComboFragment extends SubscriberFragment implements View.OnClickLis
                         }
                     }
                     tempBasalText.setText(tbrStr);
-
-
-                    // TODO error ratio?
-                    // display last warning/error?
-                    // last comm error?
                 }
             });
     }

app/src/main/java/info/nightscout/androidaps/plugins/PumpCombo/ComboPlugin.java

@@ -7,20 +7,23 @@ import org.json.JSONObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Calendar;
 import java.util.Date;
 import java.util.List;
 import java.util.Objects;
 
+import de.jotomo.ruffy.spi.BasalProfile;
 import de.jotomo.ruffy.spi.BolusProgressReporter;
 import de.jotomo.ruffy.spi.CommandResult;
 import de.jotomo.ruffy.spi.PumpState;
+import de.jotomo.ruffy.spi.PumpWarningCodes;
 import de.jotomo.ruffy.spi.RuffyCommands;
 import de.jotomo.ruffy.spi.history.Bolus;
 import de.jotomo.ruffy.spi.history.PumpError;
+import de.jotomo.ruffy.spi.history.PumpHistory;
 import de.jotomo.ruffy.spi.history.PumpHistoryRequest;
 import de.jotomo.ruffy.spi.history.Tbr;
 import de.jotomo.ruffy.spi.history.Tdd;
+import de.jotomo.ruffy.spi.history.WarningOrErrorCode;
 import de.jotomo.ruffyscripter.RuffyCommandsV1Impl;
 import info.nightscout.androidaps.BuildConfig;
 import info.nightscout.androidaps.MainApp;
@@ -59,35 +62,10 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
     private boolean fragmentEnabled = false;
     private boolean fragmentVisible = false;
 
-    private PumpDescription pumpDescription = new PumpDescription();
+    private final static PumpDescription pumpDescription = new PumpDescription();
 
-    @NonNull
+    static {
-    private final RuffyCommands ruffyScripter;
+        // these properties can't be changed on the pump, some via desktop configuration software
-
-    // TODO access to pump (and its members) is chaotic and needs an update
-    private static ComboPump pump = new ComboPump();
-
-    private volatile boolean bolusInProgress;
-    private volatile boolean cancelBolus;
-    private Bolus lastRequestedBolus;
-
-    public static ComboPlugin getPlugin() {
-        if (plugin == null)
-            plugin = new ComboPlugin();
-        return plugin;
-    }
-
-    private static PumpEnactResult OPERATION_NOT_SUPPORTED = new PumpEnactResult()
-            .success(false).enacted(false).comment(MainApp.sResources.getString(R.string.combo_pump_unsupported_operation));
-
-    private ComboPlugin() {
-        definePumpCapabilities();
-        ruffyScripter = RuffyCommandsV1Impl.getInstance(MainApp.instance());
-    }
-
-    private void definePumpCapabilities() {
-        // these properties are static; they can't be changed on the pump, but only via
-        // desktop configuration software
         pumpDescription.isBolusCapable = true;
         pumpDescription.bolusStep = 0.1d;
 
@@ -113,6 +91,29 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         pumpDescription.isRefillingCapable = true;
     }
 
+    @NonNull
+    private final RuffyCommands ruffyScripter;
+
+    private static ComboPump pump = new ComboPump();
+
+    private volatile boolean bolusInProgress;
+    private volatile boolean cancelBolus;
+    private Bolus lastRequestedBolus;
+    private long pumpHistoryLastChecked;
+
+    public static ComboPlugin getPlugin() {
+        if (plugin == null)
+            plugin = new ComboPlugin();
+        return plugin;
+    }
+
+    private static PumpEnactResult OPERATION_NOT_SUPPORTED = new PumpEnactResult()
+            .success(false).enacted(false).comment(MainApp.sResources.getString(R.string.combo_pump_unsupported_operation));
+
+    private ComboPlugin() {
+        ruffyScripter = RuffyCommandsV1Impl.getInstance(MainApp.instance());
+    }
+
     public ComboPump getPump() {
         return pump;
     }
@@ -213,18 +214,33 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
 
     @Override
     public int setNewBasalProfile(Profile profile) {
-        return FAILED;
+        BasalProfile basalProfile = convertProfileToComboProfile(profile);
+        if (pump.basalProfile.equals(basalProfile)) {
+            return PumpInterface.NOT_NEEDED;
+        }
+        CommandResult commandResult = runCommand(MainApp.sResources.getString(R.string.combo_activity_setting_basal_profile), 2,
+                () -> ruffyScripter.setBasalProfile(basalProfile));
+        return commandResult.success ? PumpInterface.SUCCESS : PumpInterface.FAILED;
     }
 
     @Override
     public boolean isThisProfileSet(Profile profile) {
-        return false;
+        return pump.basalProfile.equals(convertProfileToComboProfile(profile));
+    }
+
+    @NonNull
+    private BasalProfile convertProfileToComboProfile(Profile profile) {
+        BasalProfile basalProfile = new BasalProfile();
+        for (int i = 0; i < 24; i++) {
+            basalProfile.hourlyRates[i] = profile.getBasal(i * 60 * 60);
+        }
+        return basalProfile;
     }
 
     @NonNull
     @Override
     public Date lastDataTime() {
-        return new Date(pump.lastSuccessfulConnection);
+        return new Date(pump.lastSuccessfulCmdTime);
     }
 
     /**
@@ -252,20 +268,28 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
             return;
         }
 
-        checkForTbrMismatch(stateResult.state);
+        // ensure time and date(!) are current
-
+        /* menu not supported by ruffy
-        int minutesOfDayNow = Calendar.getInstance().get(Calendar.HOUR) + Calendar.getInstance().get(Calendar.MINUTE) * 60;
+        if (!pump.initialized) {
-        if (!pump.initialized || (Math.abs(stateResult.state.pumpTimeMinutesOfDay - minutesOfDayNow) >= 2)) {
             if (!runCommand("Updating pump clock", 2, ruffyScripter::setDateAndTime).success) {
                 return;
             }
         }
+        */
 
+        // read basal profile into cache, update pump profile if needed
+        /* not implemented by ruffiscripter
         if (!pump.initialized) {
-            if (!runCommand("Reading basal profile", 2, ruffyScripter::readBasalProfile).success) {
+            CommandResult readBasalResult = runCommand("Reading basal profile", 2, ruffyScripter::readBasalProfile);
+            if (!readBasalResult.success) {
                 return;
             }
+            Profile profile = MainApp.getConfigBuilder().getProfile();
+            setNewBasalProfile(profile);
+
+            pump.basalProfile = readBasalResult.basalProfile;
         }
+        */
 
         if (!checkPumpHistory()) {
             return;
@@ -281,8 +305,6 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         runCommand(null, 0, ruffyScripter::readPumpState);
     }
 
-
-    // TODO currently testing if updating this after each command works well
     private void updateLocalData(CommandResult result) {
         if (result.reservoirLevel != PumpState.UNKNOWN) {
             pump.reservoirLevel = result.reservoirLevel;
@@ -296,11 +318,15 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         MainApp.bus().post(new EventComboPumpUpdateGUI());
     }
 
-    // TODO uses profile values for the time being
     @Override
     public double getBaseBasalRate() {
+        Profile profile = MainApp.getConfigBuilder().getProfile();
+        Double basal = profile.getBasal();
+        log.trace("getBaseBasalrate returning " + basal);
+        return basal;
+
 /*        if (pump.basalProfile == null) {
-// TODO when to force refresh this?
+           // when to force refresh this?
             CommandResult result = runCommand("Reading basal profile", new CommandExecution() {
                 @Override
                 public CommandResult execute() {
@@ -308,15 +334,10 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
                 }
             });
             pump.basalProfile = result.basalProfile;
-            // TODO error handling ...
+            // error handling ...
         }
         return pump.basalProfile.hourlyRates[Calendar.getInstance().get(Calendar.HOUR_OF_DAY)];
 */
-
-        Profile profile = MainApp.getConfigBuilder().getProfile();
-        Double basal = profile.getBasal();
-        log.trace("getBaseBasalrate returning " + basal);
-        return basal;
     }
 
     private static BolusProgressReporter nullBolusProgressReporter = (state, percent, delivered) -> {
@@ -353,8 +374,6 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
      */
     @Override
     public PumpEnactResult deliverTreatment(DetailedBolusInfo detailedBolusInfo) {
-        // TODO safety check: reject bolus if same type and amount requested within the
-        // same minute;
         try {
             if (detailedBolusInfo.insulin == 0 && detailedBolusInfo.carbs == 0) {
                 // neither carbs nor bolus requested
@@ -402,7 +421,6 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
                     ruffyScripter::readReservoirLevelAndLastBolus);
             if (!reservoirBolusResult.success) {
                 return new PumpEnactResult().success(false).enacted(false);
-                // todo anything else needed? persistent connection problems; escalated after 30m, interactive bolus gives generic error, should be fine
             }
 
             // check enough insulin left for bolus
@@ -412,7 +430,6 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
             }
 
             // verify we're update to date and know the most recent bolus
-            // TODO should we check against pump.lastBolus or rather the DB ...
             if (!Objects.equals(pump.lastBolus, reservoirBolusResult.lastBolus)) {
                 new Thread(this::checkPumpHistory).start();
                 return new PumpEnactResult().success(false).enacted(false)
@@ -446,7 +463,6 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
             if (cancelBolus) {
                 // if cancellation was requested, the delivered bolus is allowed to differ from requested
             } else if (lastPumpBolus == null || lastPumpBolus.amount != detailedBolusInfo.insulin) {
-                // TODO schedule forced history sync
                 return new PumpEnactResult().success(false).enacted(false).
                         comment(MainApp.sResources.getString(R.string.combo_pump_bolus_verification_failed));
             }
@@ -517,9 +533,7 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
             adjustedPercent = rounded.intValue();
         }
 
-        // TODO testing how well errors on background jobs are reported back
+        int finalAdjustedPercent = adjustedPercent;
-//        CommandResult commandResult = runCommand(MainApp.sResources.getString(R.string.combo_pump_action_setting_tbr), 3, () -> ruffyScripter.setTbr(800, durationInMinutes));
-        final int finalAdjustedPercent = adjustedPercent;
         CommandResult commandResult = runCommand(MainApp.sResources.getString(R.string.combo_pump_action_setting_tbr), 3, () -> ruffyScripter.setTbr(finalAdjustedPercent, durationInMinutes));
         if (!commandResult.success) {
             return new PumpEnactResult().success(false).enacted(false);
@@ -528,16 +542,13 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         PumpState state = commandResult.state;
         if (state.tbrActive && state.tbrPercent == percent
                 && (state.tbrRemainingDuration == durationInMinutes || state.tbrRemainingDuration == durationInMinutes - 1)) {
-            // TODO timestamp: can this cause problems? setting TBR while a minute flips over?
+            TemporaryBasal tempStart = new TemporaryBasal();
-            //     might that be the cause if the TBR duration instantly flips from e.g. 0:30 -> 0:29 ?
+            tempStart.date = state.timestamp / (60 * 1000) * (60 * 1000);
-            // rounds to full minute; TODO should use time displayed on pump screen, but check and be lenient around midnight to not get the day wrong ...
-            TemporaryBasal tempStart = new TemporaryBasal(state.timestamp / (60 * 1000) * (60 * 1000));
-            // TODO commandResult.state.tbrRemainingDuration might already display 29 if 30 was set, since 29:59 is shown as 29 ...
-            // we should check this, but really ... something must be really screwed up if that number was anything different
             tempStart.durationInMinutes = durationInMinutes;
             tempStart.percentRate = adjustedPercent;
             tempStart.isAbsolute = false;
             tempStart.source = Source.USER;
+            tempStart.pumpId = tempStart.date;
             ConfigBuilderPlugin treatmentsInterface = MainApp.getConfigBuilder();
             treatmentsInterface.addToHistoryTempBasal(tempStart);
 
@@ -554,22 +565,22 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         return OPERATION_NOT_SUPPORTED;
     }
 
-    // TODO review and test, esp. aaps/pump mismatch situations
     @Override
     public PumpEnactResult cancelTempBasal(boolean userRequested) {
         log.debug("cancelTempBasal called");
         final TemporaryBasal activeTemp = MainApp.getConfigBuilder().getTempBasalFromHistory(System.currentTimeMillis());
-        if (activeTemp == null || userRequested) {
+        if (activeTemp == null) {
-            // TODO
+            return new PumpEnactResult().success(false).enacted(false);
-            /* v1 compatibility to sync DB to pump if they diverged (activeTemp == null) */
+        }
+        if (userRequested) {
             log.debug("cancelTempBasal: hard-cancelling TBR since user requested");
             CommandResult commandResult = runCommand(MainApp.sResources.getString(R.string.combo_pump_action_cancelling_tbr), 2, ruffyScripter::cancelTbr);
-
             if (!commandResult.state.tbrActive) {
-                // TODO pump menu time, midnight, blabla
+                TemporaryBasal tempBasal = new TemporaryBasal();
-                TemporaryBasal tempBasal = new TemporaryBasal(commandResult.state.timestamp / ( 60 * 1000) * (60 * 1000));
+                tempBasal.date = commandResult.state.timestamp;
                 tempBasal.durationInMinutes = 0;
                 tempBasal.source = Source.USER;
+                tempBasal.pumpId = activeTemp.pumpId;
                 MainApp.getConfigBuilder().addToHistoryTempBasal(tempBasal);
                 return new PumpEnactResult().isTempCancel(true).success(true).enacted(true);
             } else {
@@ -591,9 +602,11 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
 
             if (commandResult.state.tbrActive && commandResult.state.tbrPercent == percentage
                     && (commandResult.state.tbrRemainingDuration == 15 || commandResult.state.tbrRemainingDuration == 14)) {
-                TemporaryBasal tempBasal = new TemporaryBasal(System.currentTimeMillis());
+                TemporaryBasal tempBasal = new TemporaryBasal();
+                tempBasal.date = System.currentTimeMillis() / (60 * 1000) * (60 * 1000);
                 tempBasal.durationInMinutes = 15;
                 tempBasal.source = Source.USER;
+                tempBasal.pumpId = tempBasal.date;
                 tempBasal.percentRate = percentage;
                 tempBasal.isAbsolute = false;
                 MainApp.getConfigBuilder().addToHistoryTempBasal(tempBasal);
@@ -608,20 +621,12 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         CommandResult execute();
     }
 
-    // TODO remove this method, make stuff explicit (state updates, activity updates)
-    // and only have a withRetries() method that simply retries a command?
-
     /**
      * Runs a command, sets an activity if provided, retries if requested and updates fields
      * concerned with last connection.
      * NO history, reservoir level fields are updated, this make be done separately if desired.
      */
     private synchronized CommandResult runCommand(String activity, int retries, CommandExecution commandExecution) {
-        // TODO keep stats of how many commansd failed; if >50% fail raise an alert;
-        // otherwise all commands could fail, but since we can connect to the pump no 'pump unrechable alert' would be raised.
-
-        // TODO check and update date. can this cause any issues for running commands, so that this should be checked explicitely insstead?
-
         CommandResult commandResult;
         try {
             if (activity != null) {
@@ -629,7 +634,12 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
                 MainApp.bus().post(new EventComboPumpUpdateGUI());
             }
 
-            // TOOD check for active warning and transfer if benign (should then also work with "Refresh" button
+            if (!ruffyScripter.isConnected()) {
+                CommandResult preCheckError = runOnConnectChecks();
+                if (preCheckError != null) {
+                   return preCheckError;
+                }
+            }
 
             commandResult = commandExecution.execute();
 
@@ -640,28 +650,94 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
                 }
             }
 
+            for (Integer forwardedWarning : commandResult.forwardedWarnings) {
+                notifyAboutPumpWarning(new WarningOrErrorCode(forwardedWarning, null));
+            }
+
             if (commandResult.success) {
                 updateLocalData(commandResult);
             }
 
-            checkForUnsafeUsage(commandResult);
-
             pump.lastCmdResult = commandResult;
-            pump.lastConnectionAttempt = System.currentTimeMillis();
+            pump.lastCmdTime = System.currentTimeMillis();
             if (commandResult.success) {
-                // TOdO is this valid? saying a successful command execution means a successful connect?
+                pump.lastSuccessfulCmdTime = pump.lastCmdTime;
-                // or is the distinction between being able to connect and execute a command successfuly not really meaningful here?
-                pump.lastSuccessfulConnection = pump.lastConnectionAttempt;
             }
+
         } finally {
             if (activity != null) {
                 pump.activity = null;
                 MainApp.bus().post(new EventComboPumpUpdateGUI());
             }
         }
+
+        if (pump.initialized && pumpHistoryLastChecked + 15 * 60 * 1000 < System.currentTimeMillis()) {
+            checkPumpHistory();
+        }
+
+        checkForUnsafeUsage(commandResult);
+
         return commandResult;
     }
 
+    private CommandResult runOnConnectChecks() {
+        // connect, get status and check if an alarm is active
+        CommandResult preCheckResult = ruffyScripter.readPumpState();
+        if (!preCheckResult.success) {
+            return preCheckResult;
+        }
+
+        WarningOrErrorCode activeAlert = preCheckResult.state.activeAlert;
+        // note if multiple alerts are active this will and should fail; e.g. if pump was stopped
+        // due to empty cartridge alert, which might also trigger TBR cancelled alert
+        if (activeAlert != null) {
+            if (activeAlert.warningCode != null
+                    && (activeAlert.warningCode == PumpWarningCodes.CARTRIDGE_LOW ||
+                    activeAlert.warningCode == PumpWarningCodes.BATTERY_LOW)) {
+                // turn benign warnings into notifications
+                notifyAboutPumpWarning(activeAlert);
+                ruffyScripter.confirmAlert(activeAlert.warningCode);
+            } else {
+                Notification notification = new Notification();
+                notification.date = new Date();
+                notification.id = Notification.COMBO_PUMP_ALARM;
+                notification.level = Notification.URGENT;
+                notification.text = MainApp.sResources.getString(R.string.combo_is_in_error_state);
+                MainApp.bus().post(new EventNewNotification(notification));
+                return preCheckResult.success(false);
+            }
+        }
+
+        // check if TBR was cancelled or set by user
+        boolean mismatch = checkForTbrMismatch(preCheckResult.state);
+        if (mismatch) checkPumpHistory();
+
+        // raise notification if clock is off (setting clock is not supported by ruffy)
+        Date now = new Date();
+        int minutesOfDayNow = now.getHours() * 60 + now.getMinutes();
+        if ((Math.abs(preCheckResult.state.pumpTimeMinutesOfDay - minutesOfDayNow) > 3)) {
+            Notification notification = new Notification(Notification.COMBO_PUMP_ALARM, "Check pump clock", Notification.NORMAL);
+            MainApp.bus().post(new EventNewNotification(notification));
+//                    runCommand("Updating pump clock", 2, ruffyScripter::setDateAndTime);
+        }
+
+        return null;
+    }
+
+    private void notifyAboutPumpWarning(WarningOrErrorCode activeAlert) {
+        if (activeAlert.warningCode == null || (activeAlert.warningCode.equals(PumpWarningCodes.CARTRIDGE_LOW) && activeAlert.warningCode.equals(PumpWarningCodes.BATTERY_LOW))) {
+            throw new IllegalArgumentException(activeAlert.toString());
+        }
+        Notification notification = new Notification();
+        notification.date = new Date();
+        notification.id = Notification.COMBO_PUMP_ALARM;
+        notification.level = Notification.NORMAL;
+        notification.text = activeAlert.warningCode == PumpWarningCodes.CARTRIDGE_LOW
+                ? MainApp.sResources.getString(R.string.combo_pump_cartridge_low_warrning)
+                : MainApp.sResources.getString(R.string.combo_pump_battery_low_warrning);
+        MainApp.bus().post(new EventNewNotification(notification));
+    }
+
     private void checkForUnsafeUsage(CommandResult commandResult) {
         long lastViolation = 0;
         if (commandResult.state.unsafeUsageDetected) {
@@ -679,8 +755,6 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         if (lastViolation > 0) {
             closedLoopDisabledUntil = lastViolation + 6 * 60 * 60 * 1000;
             if (closedLoopDisabledUntil > System.currentTimeMillis() && violationWarningRaisedFor != closedLoopDisabledUntil) {
-                // TODO add message to either Combo tab or its errors popup
-                // TODO warn once; after that the user gets suggesiotn from open loop mode as a reminder...
                 Notification n = new Notification(Notification.COMBO_PUMP_ALARM,
                         MainApp.sResources.getString(R.string.combo_force_disabled),
                         Notification.URGENT);
@@ -695,40 +769,39 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
      * Checks the main screen to determine if TBR on pump matches app state.
      */
     private boolean checkForTbrMismatch(PumpState state) {
-        // detectTbrMismatch(): 'quick' check with no overhead on the pump side
-        // TODO check if this works with pump suspend, esp. around pump suspend there'll be syncing to do;
-
-        // TODO we need to tolerate differences of 1-2 minutes due to the time it takes to programm a tbr
-        // mismatching a 5m interval etc
-
         TemporaryBasal aapsTbr = MainApp.getConfigBuilder().getTempBasalFromHistory(System.currentTimeMillis());
         boolean sync = false;
         if (aapsTbr == null && state.tbrActive) {
             // pump runs TBR AAPS is unaware off
-            log.debug("Pump runs TBR AAPS is unaware of, reading last 3h of pump TBR history");
+            log.debug("Pump runs TBR AAPS is unaware of, cancelling TBR so it can be read from history properly");
+            runCommand(null, 0, ruffyScripter::cancelTbr);
             sync = true;
         } else if (aapsTbr != null && !state.tbrActive) {
             // AAPS has a TBR but the pump isn't running a TBR
-            log.debug("AAPS shows TBR but pump isn't running a TBR; deleting TBR in AAPS and reading last 3h of pump TBR history");
+            log.debug("AAPS shows TBR but pump isn't running a TBR; deleting TBR in AAPS and reading pump history");
             MainApp.getDbHelper().delete(aapsTbr);
             sync = true;
         } else if (aapsTbr != null && state.tbrActive) {
             // both AAPS and pump have a TBR ...
             if (aapsTbr.percentRate != state.tbrPercent) {
                 // ... but they have different percentages
-                log.debug("TBR percentage differs between AAPS and pump; deleting TBR in AAPS and reading last 3h of pump TBR history");
+                log.debug("TBR percentage differs between AAPS and pump; deleting TBR in AAPS and reading pump history");
                 MainApp.getDbHelper().delete(aapsTbr);
                 sync = true;
             }
             int durationDiff = Math.abs(aapsTbr.getPlannedRemainingMinutes() - state.tbrRemainingDuration);
             if (durationDiff > 2) {
                 // ... but they have different runtimes
-                log.debug("TBR duration differs between AAPS and pump; deleting TBR in AAPS and reading last 3h of pump TBR history");
+                log.debug("TBR duration differs between AAPS and pump; deleting TBR in AAPS and reading pump history");
                 MainApp.getDbHelper().delete(aapsTbr);
                 sync = true;
             }
         }
 
+        if (sync) {
+            log.debug("Sync requested from checkTbrMismatch");
+        }
+
         return sync;
     }
 
@@ -736,7 +809,11 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
      * Checks if there are any changes on the pump not in the DB yet and if so, issues a history
      * read to get the DB up to date.
      */
-    private boolean checkPumpHistory() {
+    private synchronized boolean checkPumpHistory() {
+        // set here, rather at the end so that if this runs into an error it's not tried
+        // over and over since it's called at the end of runCommand
+        pumpHistoryLastChecked = System.currentTimeMillis();
+
         CommandResult commandResult = runCommand(MainApp.sResources.getString(R.string.combo_pump_action_checking_history), 3, () ->
                 ruffyScripter.readHistory(
                         new PumpHistoryRequest()
@@ -774,7 +851,7 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         }
 
         // last TBR (TBRs are added to history upon completion so here we don't need to care about TBRs cancelled
-        //           by the user, checkTbrMismtach() takes care of that)
+        //           by the user, checkTbrMismatch() takes care of that)
         Tbr pumpTbr = null;
         List<Tbr> tbrHistory = commandResult.history.tbrHistory;
         if (!tbrHistory.isEmpty()) {
@@ -786,36 +863,11 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
             aapsTbr = MainApp.getDbHelper().getTemporaryBasalsDataByDate(pumpTbr.timestamp);
         }
 
-        // TODO check possible deviations in start/duration due to imprecision of combo
         if (pumpTbr != null && (aapsTbr == null || pumpTbr.percent != aapsTbr.percentRate || pumpTbr.duration != aapsTbr.durationInMinutes)) {
             log.debug("Last TBR on pump is unknown, refreshing TBR history");
             request.tbrHistory = aapsTbr == null ? PumpHistoryRequest.FULL : aapsTbr.date;
         }
 
-        /* This is loaded on demand
-        // last error
-        PumpError pumpError = null;
-        List<PumpError> pumpErrorHistory = commandResult.history.pumpErrorHistory;
-        if (!pumpErrorHistory.isEmpty()){
-            pumpError = pumpErrorHistory.get(0);
-        }
-        if (pumpError != null && !pump.history.pumpErrorHistory.contains(pumpError)) {
-            log.debug("Last pump error is unknown, refreshing error history");
-            request.pumpErrorHistory = pump.history.pumpErrorHistory.isEmpty() ? PumpHistoryRequest.FULL : pump.history.pumpErrorHistory.get(0).timestamp;
-        }
-
-        // last TDD
-        Tdd pumpTdd = null;
-        List<Tdd> pumpTddHistory = commandResult.history.tddHistory;
-        if (!pumpTddHistory.isEmpty()) {
-            pumpTdd = pumpTddHistory.get(0);
-        }
-        if (pumpTdd != null && !pump.history.tddHistory.contains(pumpTdd)) {
-            log.debug("Last TDD is unknown, refreshing TDD history");
-            request.tddHistory = pump.history.tddHistory.isEmpty() ? PumpHistoryRequest.FULL : pump.history.tddHistory.get(0).timestamp;
-        }
-        */
-
         if (request.bolusHistory != PumpHistoryRequest.SKIP
                 || request.tbrHistory != PumpHistoryRequest.SKIP
                 || request.pumpErrorHistory != PumpHistoryRequest.SKIP
@@ -827,69 +879,66 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
         return true;
     }
 
-    /** Reads the pump's history and updates the DB accordingly. */
+    /**
+     * Reads the pump's history and updates the DB accordingly.
+     */
     private synchronized boolean readHistory(final PumpHistoryRequest request) {
         CommandResult historyResult = runCommand(MainApp.sResources.getString(R.string.combo_activity_reading_pump_history), 3, () -> ruffyScripter.readHistory(request));
         if (!historyResult.success) {
             return false;
         }
 
-        // TODO deleting boluses/TBRs that that exist in AAPS' DB but not on the pump??
+        updateDbFromPumpHistory(historyResult.history);
+        CommandResult reservoirBolusResult = runCommand(null, 3, ruffyScripter::readReservoirLevelAndLastBolus);
+        return historyResult.success && reservoirBolusResult.success;
+    }
 
+    private synchronized void updateDbFromPumpHistory(@NonNull PumpHistory history) {
         DatabaseHelper dbHelper = MainApp.getDbHelper();
         // boluses
-        for (Bolus pumpBolus : historyResult.history.bolusHistory) {
+        for (Bolus pumpBolus : history.bolusHistory) {
-            // TODO there's a possibility of two TBR in one minute, which would be stored as one in db
-            // (date is key/unique). Later TBR would override former and the later one would run
-            // longer (former one would probably be one that's immediately overridden), so not an issue?
-            // (TDD has same issue: multiple alarms of the same type in one minute or seen as one)
             Treatment aapsBolus = dbHelper.getTreatmentByDate(pumpBolus.timestamp);
             if (aapsBolus == null) {
                 log.debug("Creating bolus record from pump bolus: " + pumpBolus);
                 DetailedBolusInfo dbi = new DetailedBolusInfo();
                 dbi.date = pumpBolus.timestamp;
+                dbi.pumpId = pumpBolus.timestamp;
+                dbi.source = Source.PUMP;
                 dbi.insulin = pumpBolus.amount;
                 dbi.eventType = CareportalEvent.CORRECTIONBOLUS;
-                dbi.source = Source.PUMP;
-                dbi.pumpId = pumpBolus.timestamp;
                 MainApp.getConfigBuilder().addToHistoryTreatment(dbi);
             }
         }
 
         // TBRs
-        // TODO tolerance for start/end deviations? temp start is based on pump time, but in ms; round to seconds, so we can find it here more easily?
+        for (Tbr pumpTbr : history.tbrHistory) {
-        for (Tbr pumpTbr : historyResult.history.tbrHistory) {
             TemporaryBasal aapsTbr = dbHelper.getTemporaryBasalsDataByDate(pumpTbr.timestamp);
             if (aapsTbr == null) {
                 log.debug("Creating TBR from pump TBR: " + pumpTbr);
                 TemporaryBasal temporaryBasal = new TemporaryBasal();
                 temporaryBasal.date = pumpTbr.timestamp;
+                temporaryBasal.pumpId = pumpTbr.timestamp;
+                temporaryBasal.source = Source.PUMP;
                 temporaryBasal.percentRate = pumpTbr.percent;
                 temporaryBasal.durationInMinutes = pumpTbr.duration;
                 temporaryBasal.isAbsolute = false;
-                temporaryBasal.source = Source.PUMP;
-                temporaryBasal.pumpId = pumpTbr.timestamp;
                 MainApp.getConfigBuilder().addToHistoryTempBasal(temporaryBasal);
             }
         }
 
         // errors (not persisted in DB, read from pump on start up and cached in plugin)
-        for (PumpError pumpError : historyResult.history.pumpErrorHistory) {
+        for (PumpError pumpError : history.pumpErrorHistory) {
             if (!pump.history.pumpErrorHistory.contains(pumpError)) {
                 pump.history.pumpErrorHistory.add(pumpError);
             }
         }
 
         // TDDs (not persisted in DB, read from pump on start up and cached in plugin)
-        for (Tdd tdd : historyResult.history.tddHistory) {
+        for (Tdd tdd : history.tddHistory) {
             if (!pump.history.tddHistory.contains(tdd)) {
                 pump.history.tddHistory.add(tdd);
             }
         }
-
-        CommandResult reservoirBolusResult = runCommand(null, 3, ruffyScripter::readReservoirLevelAndLastBolus);
-
-        return historyResult.success && reservoirBolusResult.success;
     }
 
     void forceFullHistoryRead() {
@@ -913,12 +962,12 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
 
         try {
             JSONObject pumpJson = new JSONObject();
-            pumpJson.put("clock", DateUtil.toISOString(pump.lastSuccessfulConnection));
+            pumpJson.put("clock", DateUtil.toISOString(pump.lastSuccessfulCmdTime));
             pumpJson.put("reservoir", pump.reservoirLevel);
 
             JSONObject statusJson = new JSONObject();
             statusJson.put("status", getStateSummary());
-            statusJson.put("timestamp", pump.lastSuccessfulConnection);
+            statusJson.put("timestamp", pump.lastSuccessfulCmdTime);
             pumpJson.put("status", statusJson);
 
             JSONObject extendedJson = new JSONObject();
@@ -934,8 +983,8 @@ public class ComboPlugin implements PluginBase, PumpInterface, ConstraintsInterf
                 extendedJson.put("TempBasalPercent", ps.tbrPercent);
                 extendedJson.put("TempBasalRemaining", ps.tbrRemainingDuration);
             }
-            if (ps.alertCodes != null && ps.alertCodes.errorCode != null) {
+            if (ps.activeAlert != null && ps.activeAlert.errorCode != null) {
-                extendedJson.put("ErrorCode", ps.alertCodes.errorCode);
+                extendedJson.put("ErrorCode", ps.activeAlert.errorCode);
             }
             pumpJson.put("extended", extendedJson);
 

app/src/main/java/info/nightscout/androidaps/plugins/PumpCombo/ComboPump.java

@@ -10,12 +10,9 @@ import de.jotomo.ruffy.spi.history.Bolus;
 import de.jotomo.ruffy.spi.history.PumpHistory;
 
 class ComboPump {
-    // TODO all non-state (==main screen) data is overriden by commands, no? put them seperately
-    // at least skim over how dana does it!
     boolean initialized = false;
-    // TODO actually ... this isn't about successful command execution, but whether we could connect to the pump at all
+    volatile long lastSuccessfulCmdTime;
-    volatile long lastSuccessfulConnection;
+    volatile long lastCmdTime;
-    volatile long lastConnectionAttempt;
 
     @Nullable
     volatile CommandResult lastCmdResult;
@@ -24,12 +21,10 @@ class ComboPump {
     volatile PumpState state = new PumpState();
     volatile int reservoirLevel = -1;
     volatile Bolus lastBolus = null;
-    @Nullable
+    @NonNull
-    volatile BasalProfile basalProfile;
+    volatile BasalProfile basalProfile = new BasalProfile();
     @NonNull
     volatile PumpHistory history = new PumpHistory();
-    /**
+    /** Time the active TBR was set (if any). Needed to calculate remaining time in fragment */
-     * Time the active TBR was set (if any). Needed to calculate remaining time in fragment
-     */
     long tbrSetTime;
 }

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

@@ -768,14 +768,11 @@
     <string name="combo_stats">Stats</string>
     <string name="combo_pump_state_label">State</string>
     <string name="combo_pump_activity">Activity</string>
-    <string name="combo_no_pump_connection">No connection for %s</string>
+    <string name="combo_no_pump_connection">No successful connection for %s</string>
-    <!-- TODO v2 this is more like 'last command' failed -->
     <string name="combo_connect_attempt_failed">Last connect attempt failed</string>
     <string name="combo_last_connection_time">%s</string>
     <string name="combo_tbr_remaining">%d%% (%d min remaining)</string>
     <string name="combo_last_bolus">%.1f U (%s, %s)</string>
-    <!-- TODO v2 better name, 'disconnected' but may confusing since the loop can be disconnected
-        and technically the pump is disconnected when no command is running -->
     <string name="combo_pump_state_disconnected">Disconnected</string>
     <string name="combo_pump_state_suspended_due_to_error">Suspended due to error</string>
     <string name="combo_pump_state_suspended_by_user">Suspended by user</string>
@@ -800,5 +797,9 @@
     <string name="combo_activity_reading_pump_history">Reading pump history</string>
     <string name="pump_history">pump history</string>
     <string name="combo_pump_alerts">Alerts</string>
+    <string name="combo_activity_setting_basal_profile">Setting basal profile</string>
+    <string name="combo_pump_cartridge_low_warrning">Pump cartridge level is low</string>
+    <string name="combo_pump_battery_low_warrning">Pump battery is low</string>
+    <string name="combo_is_in_error_state">Pump is in an error state, please check the pump</string>
 </resources>
 

ruffy-spi/src/main/java/de/jotomo/ruffy/spi/BasalProfile.java

@@ -1,9 +1,30 @@
 package de.jotomo.ruffy.spi;
 
+import java.util.Arrays;
+
 public class BasalProfile {
     public final double[] hourlyRates;
 
+    public BasalProfile() {
+        this.hourlyRates = new double[24];
+    }
+
     public BasalProfile( double[] hourlyRates) {
         this.hourlyRates = hourlyRates;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+
+        BasalProfile that = (BasalProfile) o;
+
+        return Arrays.equals(hourlyRates, that.hourlyRates);
+    }
+
+    @Override
+    public int hashCode() {
+        return Arrays.hashCode(hourlyRates);
+    }
 }

ruffy-spi/src/main/java/de/jotomo/ruffy/spi/CommandResult.java

@@ -2,14 +2,15 @@ package de.jotomo.ruffy.spi;
 
 import android.support.annotation.Nullable;
 
+import java.util.LinkedList;
+import java.util.List;
+
 import de.jotomo.ruffy.spi.history.Bolus;
 import de.jotomo.ruffy.spi.history.PumpHistory;
 
 public class CommandResult {
     /** Whether the command was executed successfully. */
     public boolean success;
-    /** Null unless an unhandled exception was raised. */
-    public Exception exception;
     /** State of the pump *after* command execution. */
     public PumpState state;
     /** History if requested by the command. */
@@ -20,13 +21,9 @@ public class CommandResult {
     /** Total duration the command took. */
     public String duration;
 
-    /** Whether an alert (warning only) was confirmed. This can happen during boluses.
+    /** Warnings raised on the pump that are forwarded to AAPS to be turned into AAPS
-     * Request error history to see which errors occurred. */
+     * notifications. */
-    // TODO check usage
+    public List<Integer> forwardedWarnings = new LinkedList<>();
-    // TODO return alerts to display? 'forwardedAlert'?
-    public boolean alertConfirmed;
-    /** BolusCommand: if a cancel request was successful */
-    public boolean wasSuccessfullyCancelled;
 
     public int reservoirLevel = -1;
 
@@ -43,11 +40,6 @@ public class CommandResult {
         return this;
     }
 
-    public CommandResult exception(Exception exception) {
-        this.exception = exception;
-        return this;
-    }
-
     public CommandResult state(PumpState state) {
         this.state = state;
         return this;
@@ -67,13 +59,11 @@ public class CommandResult {
     public String toString() {
         return "CommandResult{" +
                 "success=" + success +
-                ", exception=" + exception +
                 ", state=" + state +
                 ", history=" + history +
                 ", basalProfile=" + basalProfile +
                 ", duration='" + duration + '\'' +
-                ", alertConfirmed='" + alertConfirmed + '\'' +
+                ", forwardedWarnings='" + forwardedWarnings + '\'' +
-                ", wasSuccessfullyCancelled='" + wasSuccessfullyCancelled + '\'' +
                 '}';
     }
 }

ruffy-spi/src/main/java/de/jotomo/ruffy/spi/PumpState.java

@@ -4,7 +4,7 @@ import de.jotomo.ruffy.spi.history.WarningOrErrorCode;
 
 /** State displayed on the main screen of the pump. */
 public class PumpState {
-    /** Time the state was captured */
+    /** Time the state was captured, rounded to a full second */
     public long timestamp;
     /** The time displayed on the main menu */
     public long pumpTimeMinutesOfDay;
@@ -22,7 +22,7 @@ public class PumpState {
 
     /** Warning or error code displayed if a warning or alert alert is active,
      * see {@link PumpWarningCodes}, {@link PumpErrorCodes} */
-    public WarningOrErrorCode alertCodes;
+    public WarningOrErrorCode activeAlert;
 
     public static final int UNKNOWN = -1;
     public static final int LOW = 1;

ruffy-spi/src/main/java/de/jotomo/ruffy/spi/history/WarningOrErrorCode.java

@@ -15,5 +15,13 @@ public class WarningOrErrorCode {
         this.warningCode = warningCode;
         this.errorCode = errorCode;
     }
+
+    @Override
+    public String toString() {
+        return "WarningOrErrorCode{" +
+                "warningCode=" + warningCode +
+                ", errorCode=" + errorCode +
+                '}';
+    }
 }
 

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/RuffyScripter.java

@@ -54,8 +54,6 @@ public class RuffyScripter implements RuffyCommands {
     private static final Logger log = LoggerFactory.getLogger(RuffyScripter.class);
 
     private IRuffyService ruffyService;
-    // TODO never written
-    private String unrecoverableError = null;
 
     @Nullable
     private volatile Menu currentMenu;
@@ -178,7 +176,7 @@ public class RuffyScripter implements RuffyCommands {
     private Thread idleDisconnectMonitorThread = new Thread(new Runnable() {
         @Override
         public void run() {
-            while (unrecoverableError == null) {
+            while (!Thread.currentThread().isInterrupted()) {
                 try {
                     long now = System.currentTimeMillis();
                     long connectionTimeOutMs = 5000;
@@ -233,7 +231,9 @@ public class RuffyScripter implements RuffyCommands {
         }
     }
 
-    /** Always returns a CommandResult, never throws */
+    /**
+     * Always returns a CommandResult, never throws
+     */
     private CommandResult runCommand(final Command cmd) {
         log.debug("Attempting to run cmd: " + cmd);
 
@@ -243,7 +243,6 @@ public class RuffyScripter implements RuffyCommands {
             return new CommandResult().success(false).state(readPumpStateInternal());
         }
 
-        // TODO simplify, hard to reason about exists
         synchronized (RuffyScripter.class) {
             try {
                 activeCmd = cmd;
@@ -252,21 +251,34 @@ public class RuffyScripter implements RuffyCommands {
                 log.debug("Connection ready to execute cmd " + cmd);
                 Thread cmdThread = new Thread(() -> {
                     try {
-                        if (getCurrentMenu().getType() == MenuType.STOP) {
+                        // check pump in a suitable state to run the requested command
-                            if (cmd.needsRunMode()) {
+                        if (cmd instanceof ReadPumpStateCommand) {
+                            // always allowed, state is set at the end of runCommand method
+                        } else if (getCurrentMenu().getType() == MenuType.STOP && cmd.needsRunMode()) {
                             log.error("Requested command requires run mode, but pump is suspended");
                             activeCmd.getResult().success = false;
                             return;
-                            }
+                        } else if (getCurrentMenu().getType() == MenuType.WARNING_OR_ERROR && !(cmd instanceof ConfirmAlertCommand)) {
-                        }
-                        if (getCurrentMenu().getType() == MenuType.WARNING_OR_ERROR
-                                && !(cmd instanceof ConfirmAlertCommand)) {
                             log.warn("Warning/alert active on pump, but requested command is not ConfirmAlertCommand");
                             activeCmd.getResult().success = false;
                             return; // active alert is returned as part of PumpState
+                        } else if (getCurrentMenu().getType() != MenuType.MAIN_MENU) {
+                            log.debug("Pump is unexpectedly not on main menu but " + getCurrentMenuName());
+                            activeCmd.getResult().success = false;
+                            return;
                         }
+
+                        // if everything broke before, the pump might still be delivering a bolus, if that's the case, wait for bolus to finish
+                        Double bolusRemaining = (Double) getCurrentMenu().getAttribute(MenuAttribute.BOLUS_REMAINING);
+                        while (ruffyService.isConnected() && bolusRemaining != null) {
+                            log.debug("Waiting for bolus from previous connection to complete, remaining: " + bolusRemaining);
+                            waitForScreenUpdate();
+                        }
+
                         PumpState pumpState = readPumpStateInternal();
                         log.debug("Pump state before running command: " + pumpState);
+
+                        // execute the command
                         cmd.setScripter(RuffyScripter.this);
                         long cmdStartTime = System.currentTimeMillis();
                         cmd.execute();
@@ -285,54 +297,40 @@ public class RuffyScripter implements RuffyCommands {
                 long executionStart = System.currentTimeMillis();
                 cmdThread.start();
 
-                // time out if nothing has been happening for more than 90s or after 4m
+                long overallTimeout = System.currentTimeMillis() + 4 * 60 * 1000;
-                // (to fail before the next loop iteration issues the next command)
-                long dynamicTimeout = calculateCmdInactivityTimeout();
-                long overallTimeout = calculateOverallCmdTimeout();
                 while (cmdThread.isAlive()) {
                     if (!ruffyService.isConnected()) {
-                        // on connection lose try to reconnect, confirm warning alerts caused by
+                        // on connection loss try to reconnect, confirm warning alerts caused by
                         // the disconnected and then return the command as failed (the caller
                         // can retry if needed).
                         cmdThread.interrupt();
                         activeCmd.getResult().success = false;
-                        for(int attempts = 4; attempts > 0; attempts--) {
+                        for (int attempts = 4; attempts > 0; attempts--) {
                             boolean reconnected = recoverFromConnectionLoss();
                             if (reconnected) {
                                 break;
                             }
-                            // try again in 20 sec
+                            // connect attempt times out after 30s, shortly wait and then retry;
-                            SystemClock.sleep(20 * 1000);
+                            // (30s timeout + 5s wait) * 4 attempts = 140s
+                            SystemClock.sleep(5 * 1000);
                         }
                     }
 
-                    long now = System.currentTimeMillis();
+                    // abort if there has been no transmission from the pump for 15s
-                    if (now > dynamicTimeout) {
+                    if (!(System.currentTimeMillis() < menuLastUpdated + 15 * 1000)) {
-                        boolean menuRecentlyUpdated = now < menuLastUpdated + 5 * 1000;
-                        boolean idlingInMainMenu = getCurrentMenu().getType() == MenuType.MAIN_MENU
-                                && !getCurrentMenu().attributes().contains(MenuAttribute.BOLUS_REMAINING);
-                        // TODO this triggers falsely if command finished and idles in main menu until
-                        // conneciton is closed; probably better since wait is moved to bottom of loop
-                        // so the while(cmdThread.isAlive) check and this isn't 500ms apart.
-                        // still, think this through; maybe wait a fec seconds and recheck if command is
-                        // active and menu is still main menu ...
-                        if (menuRecentlyUpdated && !idlingInMainMenu) {
-                            // command still working (or waiting for pump to complete, e.g. bolus delivery)
-                            dynamicTimeout = now + 30 * 1000;
-                        } else {
                         log.error("Dynamic timeout running command " + activeCmd);
                         cmdThread.interrupt();
-                            SystemClock.sleep(5000);
-                            log.error("Timed out thread dead yet? " + cmdThread.isAlive());
                         activeCmd.getResult().success = false;
                         break;
                     }
-                    }
+
-                    if (now > overallTimeout) {
+                    if (System.currentTimeMillis() > overallTimeout) {
                         log.error("Command " + cmd + " timed out");
+                        cmdThread.interrupt();
                         activeCmd.getResult().success = false;
                         break;
                     }
+
                     log.trace("Waiting for running command to complete");
                     SystemClock.sleep(500);
                 }
@@ -346,7 +344,6 @@ public class RuffyScripter implements RuffyCommands {
                     log.debug("Connect: " + connectDurationSec + "s, execution: " + executionDurationSec + "s");
                 }
                 return result;
-                // TOD under which circumstances can these occur?
             } catch (CommandException e) {
                 log.error("CommandException while executing command", e);
                 recoverFromCommandFailure();
@@ -354,9 +351,10 @@ public class RuffyScripter implements RuffyCommands {
             } catch (Exception e) {
                 log.error("Unexpected exception communication with ruffy", e);
                 recoverFromCommandFailure();
-                return activeCmd.getResult().success(false).exception(e).state(readPumpStateInternal());
+                return activeCmd.getResult().success(false).state(readPumpStateInternal());
             } finally {
-                if (activeCmd.getResult().success) {
+                Menu menu = this.currentMenu;
+                if (activeCmd.getResult().success && menu != null && menu.getType() != MenuType.MAIN_MENU) {
                     log.warn("Command " + activeCmd + " successful, but finished leaving pump on menu " + getCurrentMenuName());
                 }
                 activeCmd = null;
@@ -364,16 +362,8 @@ public class RuffyScripter implements RuffyCommands {
         }
     }
 
-    private long calculateCmdInactivityTimeout() {
-        return System.currentTimeMillis() + 5 * 1000;
-    }
-
-    private long calculateOverallCmdTimeout() {
-        return System.currentTimeMillis() + 4 * 60 * 1000;
-    }
-
     /**
-     * On connection lose the pump raises an alert immediately (when setting a TBR or giving a bolus) -
+     * On connection loss the pump raises an alert immediately (when setting a TBR or giving a bolus) -
      * there's no timeout before that happens. But: a reconnect is still possible which can then
      * confirm the alert.
      *
@@ -405,16 +395,18 @@ public class RuffyScripter implements RuffyCommands {
                 waitForScreenUpdate();
             }
             boolean connected = ruffyService.isConnected();
-            log.debug("Recovery from connection loss successful:" + connected);
+            log.debug("Recovery from connection loss " + (connected ? "succeeded" : "failed"));
             return connected;
         } catch (RemoteException e) {
-            log.debug("Recovery from connection loss failed");
+            log.debug("Recovery from connection loss failed", e);
             return false;
         }
     }
 
-    /** Returns to the main menu (if possible) after a command failure, so that subsequent commands
+    /**
-     * reusing the connection won't fail. */
+     * Returns to the main menu (if possible) after a command failure, so that subsequent commands
+     * reusing the connection won't fail.
+     */
     private void recoverFromCommandFailure() {
         Menu menu = this.currentMenu;
         if (menu == null) {
@@ -430,7 +422,9 @@ public class RuffyScripter implements RuffyCommands {
         }
     }
 
-    /** If there's an issue, this times out eventually and throws a CommandException */
+    /**
+     * If there's an issue, this times out eventually and throws a CommandException
+     */
     private void ensureConnected() {
         try {
             if (ruffyService.isConnected()) {
@@ -456,16 +450,14 @@ public class RuffyScripter implements RuffyCommands {
         }
     }
 
-    // TODO v2 add remaining info we can extract from the main menu, low battery and low
-    // cartridge warnings, running extended bolus (how does that look if a TBR is active as well?)
-
     /**
      * This reads the state of the, which is whatever is currently displayed on the display,
      * no actions are performed.
      */
     public PumpState readPumpStateInternal() {
         PumpState state = new PumpState();
-        state.timestamp = System.currentTimeMillis();
+        // round timestamp to full second
+        state.timestamp = System.currentTimeMillis() / (60 * 1000) * (60 * 1000);
         Menu menu = currentMenu;
         if (menu == null) {
             return state;
@@ -519,11 +511,6 @@ public class RuffyScripter implements RuffyCommands {
                 ? new WarningOrErrorCode(warningCode, errorCode) : null;
     }
 
-// below: methods to be used by commands
-// TODO move into a new Operations(scripter) class commands can delegate to,
-// so this class can focus on providing a connection to run commands
-// (or maybe reconsider putting it into a base class)
-
     public static class Key {
         public static byte NO_KEY = (byte) 0x00;
         public static byte MENU = (byte) 0x03;
@@ -547,7 +534,7 @@ public class RuffyScripter implements RuffyCommands {
     }
 
     @Nullable
-    public String getCurrentMenuName() {
+    private String getCurrentMenuName() {
         Menu menu = this.currentMenu;
         return menu != null ? menu.getType().toString() : "<none>";
     }
@@ -576,7 +563,7 @@ public class RuffyScripter implements RuffyCommands {
         log.debug("Releasing menu key");
     }
 
-    public void pressBackKey() {
+    private void pressBackKey() {
         log.debug("Pressing back key");
         pressKey(Key.BACK);
         log.debug("Releasing back key");
@@ -781,7 +768,6 @@ public class RuffyScripter implements RuffyCommands {
             if (getCurrentMenu().getType() == MenuType.WARNING_OR_ERROR) {
                 WarningOrErrorCode warningOrErrorCode = readWarningOrErrorCode();
                 if (warningOrErrorCode.errorCode != null) {
-                    // TODO proper way to display such things in the UI;
                     throw new CommandException("Pump is in error state");
                 }
                 Integer displayedWarningCode = warningOrErrorCode.warningCode;

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/BaseCommand.java

@@ -22,9 +22,6 @@ public abstract class BaseCommand implements Command {
         return false;
     }
 
-    // TODO i18n; can we work with error codes instead of messages? Like W07? that way we're language agnostic
-    // error message ist still needed to cancel TBR though, let next-gen ruffy take care of that?
-
     /**
      * A warning id (or null) caused by a disconnect we can safely confirm on reconnect,
      * knowing it's not severe as it was caused by this command.

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/BolusCommand.java

@@ -34,7 +34,6 @@ public class BolusCommand extends BaseCommand {
     public BolusCommand(double bolus, BolusProgressReporter bolusProgressReporter) {
         this.bolus = bolus;
         this.bolusProgressReporter = bolusProgressReporter;
-        this.result = new CommandResult();
     }
 
     @Override
@@ -123,14 +122,12 @@ public class BolusCommand extends BaseCommand {
                     if (warningCode == PumpWarningCodes.BOLUS_CANCELLED) {
                         scripter.confirmAlert(PumpWarningCodes.BOLUS_CANCELLED, 2000);
                         bolusProgressReporter.report(STOPPED, 0, 0);
-                        result.wasSuccessfullyCancelled = true;
-                        result.alertConfirmed = true;
                     } else if (warningCode == PumpWarningCodes.CARTRIDGE_LOW) {
                         scripter.confirmAlert(PumpWarningCodes.CARTRIDGE_LOW, 2000);
-                        result.alertConfirmed = true;
+                        result.forwardedWarnings.add(PumpWarningCodes.CARTRIDGE_LOW);
                     } else if (warningCode == PumpWarningCodes.BATTERY_LOW) {
                         scripter.confirmAlert(PumpWarningCodes.BATTERY_LOW, 2000);
-                        result.alertConfirmed = true;
+                        result.forwardedWarnings.add(PumpWarningCodes.BATTERY_LOW);
                     } else {
                         throw new CommandException("Pump is showing exotic warning: " + warningCode);
                     }

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/CancelTbrCommand.java

@@ -7,9 +7,6 @@ import org.slf4j.LoggerFactory;
 import de.jotomo.ruffy.spi.PumpState;
 import de.jotomo.ruffy.spi.PumpWarningCodes;
 
-// TODO robustness: can a TBR run out, whilst we're trying to cancel it?
-// Hm, we could just ignore TBRs that run out within the next 60s (0:01 or even 0:02
-// given we need some time to process the request).
 public class CancelTbrCommand extends BaseCommand {
     private static final Logger log = LoggerFactory.getLogger(CancelTbrCommand.class);
 

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/ReadBasalProfileCommand.java

@@ -5,8 +5,10 @@ import de.jotomo.ruffy.spi.BasalProfile;
 public class ReadBasalProfileCommand extends BaseCommand {
     @Override
     public void execute() {
+        if (1==1) throw new RuntimeException("No implemented yet");
         // TODO
         result.basalProfile(new BasalProfile(new double[] {0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d, 0.5d}));
+        //scripter.returnToRootMenu();
         result.success = true;
     }
 }

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/ReadHistoryCommand.java

@@ -100,7 +100,6 @@ public class ReadHistoryCommand extends BaseCommand {
                 }
             }
 
-            scripter.pressBackKey();
             scripter.returnToRootMenu();
             scripter.verifyRootMenuIsDisplayed();
         }

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/SetBasalProfileCommand.java

@@ -15,6 +15,7 @@ public class SetBasalProfileCommand extends BaseCommand {
     @Override
     public void execute() {
         // TODO
+        throw new RuntimeException("Not implemented yet");
     }
 
     @Override

ruffyscripter/src/main/java/de/jotomo/ruffyscripter/commands/SetDateAndTimeCommand.java

@@ -6,16 +6,19 @@ import org.monkey.d.ruffy.ruffy.driver.display.MenuType;
 
 import de.jotomo.ruffy.spi.CommandResult;
 
-/**
- * Created by joe on 08/11/17.
- */
-
 public class SetDateAndTimeCommand extends BaseCommand {
     @Override
     public void execute() {
+        throw new RuntimeException("Not implemented yet");
 /*        scripter.navigateToMenu(MenuType.DATE_AND_TIME_MENU);
         scripter.pressCheckKey();
-        // TODO ruffy does'n support date/time menu yet*/
+        // TODO ruffy does'n support date/time menu yet
         result.success = true;
+*/
+    }
+
+    @Override
+    public String toString() {
+        return "SetDateAndTimeCommand{}";
     }
 }