Rework charts to completely fix crash in charts activity #277

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/AbstractChartFragment.java

@@ -335,6 +335,8 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
     }
 
     protected void configureChartDefaults(Chart<?> chart) {
+        chart.setDescription("");
+
         // if enabled, the chart will always start at zero on the y-axis
         chart.setNoDataText(getString(R.string.chart_no_data_synchronize));
 
@@ -343,6 +345,8 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
 
         // enable touch gestures
         chart.setTouchEnabled(true);
+
+        setupLegend(chart);
     }
 
     protected void configureBarLineChartDefaults(BarLineChartBase<?> chart) {
@@ -380,9 +384,9 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
     /**
      * This method reads the data from the database, analyzes and prepares it for
      * the charts. This will be called from a background task, so there must not be
-     * any UI access. #renderCharts will be automatically called after this method.
+     * any UI access. #updateChartsInUIThread and #renderCharts will be automatically called after this method.
      */
-    protected abstract void refreshInBackground(DBHandler db, GBDevice device);
+    protected abstract ChartsData refreshInBackground(ChartsHost chartsHost, DBHandler db, GBDevice device);
 
     /**
      * Triggers the actual (re-) rendering of the chart.
@@ -390,7 +394,7 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
      */
     protected abstract void renderCharts();
 
-    protected void refresh(GBDevice gbDevice, BarLineChartBase chart, List<ActivitySample> samples) {
+    protected DefaultChartsData refresh(GBDevice gbDevice, List<ActivitySample> samples) {
         Calendar cal = GregorianCalendar.getInstance();
         cal.clear();
         Date date;
@@ -398,6 +402,7 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
         String dateStringTo = "";
 
         LOG.info("" + getTitle() + ": number of samples:" + samples.size());
+        CombinedData combinedData;
         if (samples.size() > 1) {
             boolean annotate = true;
             boolean use_steps_as_movement;
@@ -486,11 +491,11 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
                 xLabels.add(xLabel);
             }
 
-            chart.getXAxis().setValues(xLabels);
+//            chart.getXAxis().setValues(xLabels);
 
             BarDataSet activitySet = createActivitySet(activityEntries, colors, "Activity");
             // create a data object with the datasets
-            CombinedData combinedData = new CombinedData(xLabels);
+            combinedData = new CombinedData(xLabels);
             List<IBarDataSet> list = new ArrayList<>();
             list.add(activitySet);
             BarData barData = new BarData(xLabels, list);
@@ -503,17 +508,13 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
                 combinedData.setData(lineData);
             }
 
-            chart.setDescription("");
 //            chart.setDescription(getString(R.string.sleep_activity_date_range, dateStringFrom, dateStringTo));
 //            chart.setDescriptionPosition(?, ?);
-
-            setupLegend(chart);
-
-            chart.setData(combinedData);
         } else {
-            CombinedData data = new CombinedData(Collections.<String>emptyList());
+            combinedData = new CombinedData(Collections.<String>emptyList());
-            chart.setData(data);
         }
+
+        return new DefaultChartsData(combinedData);
     }
 
     protected boolean isValidHeartRateValue(int value) {
@@ -622,6 +623,8 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
     }
 
     public class RefreshTask extends DBAccess {
+        private ChartsData chartsData;
+
         public RefreshTask(String task, Context context) {
             super(task, context);
         }
@@ -630,7 +633,9 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
         protected void doInBackground(DBHandler db) {
             ChartsHost chartsHost = getChartsHost();
             if (chartsHost != null) {
-                refreshInBackground(db, chartsHost.getDevice());
+                chartsData = refreshInBackground(chartsHost, db, chartsHost.getDevice());
+            } else {
+                cancel(true);
             }
         }
 
@@ -639,6 +644,7 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
             super.onPostExecute(o);
             FragmentActivity activity = getActivity();
             if (activity != null && !activity.isFinishing() && !activity.isDestroyed()) {
+                updateChartsnUIThread(chartsData);
                 renderCharts();
             } else {
                 LOG.info("Not rendering charts because activity is not available anymore");
@@ -646,6 +652,8 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
         }
     }
 
+    protected abstract void updateChartsnUIThread(ChartsData chartsData);
+
     /**
      * Returns true if the date was successfully shifted, and false if the shift
      * was ignored, e.g. when the to-value is in the future.
@@ -689,4 +697,16 @@ public abstract class AbstractChartFragment extends AbstractGBFragment {
     private int toTimestamp(Date date) {
         return (int) ((date.getTime() / 1000));
     }
+
+    public static class DefaultChartsData extends ChartsData{
+        private final CombinedData combinedData;
+
+        public DefaultChartsData(CombinedData combinedData) {
+            this.combinedData = combinedData;
+        }
+
+        public CombinedData getCombinedData() {
+            return combinedData;
+        }
+    }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ActivitySleepChartFragment.java

@@ -106,11 +106,16 @@ public class ActivitySleepChartFragment extends AbstractChartFragment {
     }
 
     @Override
-    protected void refreshInBackground(DBHandler db, GBDevice device) {
+    protected ChartsData refreshInBackground(ChartsHost chartsHost, DBHandler db, GBDevice device) {
         List<ActivitySample> samples = getSamples(db, device);
-        refresh(device, mChart, samples);
+        return refresh(device, samples);
+    }
 
+    @Override
+    protected void updateChartsnUIThread(ChartsData chartsData) {
+        DefaultChartsData dcd = (DefaultChartsData) chartsData;
         mChart.getLegend().setTextColor(LEGEND_TEXT_COLOR);
+        mChart.setData(dcd.getCombinedData());
     }
 
     protected void renderCharts() {

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ChartsData.java

@@ -0,0 +1,4 @@
+package nodomain.freeyourgadget.gadgetbridge.activities.charts;
+
+public abstract class ChartsData {
+}

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/LiveActivityFragment.java

@@ -402,7 +402,13 @@ public class LiveActivityFragment extends AbstractChartFragment {
     }
 
     @Override
-    protected void refreshInBackground(DBHandler db, GBDevice device) {
+    protected ChartsData refreshInBackground(ChartsHost chartsHost, DBHandler db, GBDevice device) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    protected void updateChartsnUIThread(ChartsData chartsData) {
+        throw new UnsupportedOperationException();
     }
 
     @Override

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/SleepChartFragment.java

@@ -48,14 +48,16 @@ public class SleepChartFragment extends AbstractChartFragment {
     private int mSmartAlarmGoneOff = -1;
 
     @Override
-    protected void refreshInBackground(DBHandler db, GBDevice device) {
+    protected ChartsData refreshInBackground(ChartsHost chartsHost, DBHandler db, GBDevice device) {
         List<ActivitySample> samples = getSamples(db, device);
 
-        refresh(device, mActivityChart, samples);
+        MySleepChartsData mySleepChartsData = refreshSleepAmounts(device, samples);
-        refreshSleepAmounts(device, mSleepAmountChart, samples);
+        DefaultChartsData chartsData = refresh(device, samples);
+
+        return new MyChartsData(mySleepChartsData, chartsData);
     }
 
-    private void refreshSleepAmounts(GBDevice mGBDevice, PieChart pieChart, List<ActivitySample> samples) {
+    private MySleepChartsData refreshSleepAmounts(GBDevice mGBDevice, List<ActivitySample> samples) {
         ActivityAnalysis analysis = new ActivityAnalysis();
         ActivityAmounts amounts = analysis.calculateActivityAmounts(samples);
         PieData data = new PieData();
@@ -73,7 +75,6 @@ public class SleepChartFragment extends AbstractChartFragment {
             }
         }
         String totalSleep = DateTimeUtils.formatDurationHoursMinutes(totalSeconds, TimeUnit.SECONDS);
-        pieChart.setCenterText(totalSleep);
         PieDataSet set = new PieDataSet(entries, "");
         set.setValueFormatter(new ValueFormatter() {
             @Override
@@ -83,10 +84,18 @@ public class SleepChartFragment extends AbstractChartFragment {
         });
         set.setColors(colors);
         data.setDataSet(set);
-        pieChart.setData(data);
 
-        pieChart.getLegend().setEnabled(false);
         //setupLegend(pieChart);
+        return new MySleepChartsData(totalSleep, data);
+    }
+
+    @Override
+    protected void updateChartsnUIThread(ChartsData chartsData) {
+        MyChartsData mcd = (MyChartsData) chartsData;
+        mSleepAmountChart.setCenterText(mcd.getPieData().getTotalSleep());
+        mSleepAmountChart.setData(mcd.getPieData().getPieData());
+
+        mActivityChart.setData(mcd.getChartsData().getCombinedData());
     }
 
     @Override
@@ -132,6 +141,7 @@ public class SleepChartFragment extends AbstractChartFragment {
         mSleepAmountChart.setDescription("");
         mSleepAmountChart.setNoDataTextDescription("");
         mSleepAmountChart.setNoDataText("");
+        mSleepAmountChart.getLegend().setEnabled(false);
     }
 
     private void setupActivityChart() {
@@ -194,4 +204,40 @@ public class SleepChartFragment extends AbstractChartFragment {
         mActivityChart.animateX(ANIM_TIME, Easing.EasingOption.EaseInOutQuart);
         mSleepAmountChart.invalidate();
     }
+
+    private static class MySleepChartsData extends ChartsData {
+        private String totalSleep;
+        private final PieData pieData;
+
+        public MySleepChartsData(String totalSleep, PieData pieData) {
+            this.totalSleep = totalSleep;
+            this.pieData = pieData;
+        }
+
+        public PieData getPieData() {
+            return pieData;
+        }
+
+        public CharSequence getTotalSleep() {
+            return totalSleep;
+        }
+    }
+
+    private static class MyChartsData extends ChartsData {
+        private final DefaultChartsData chartsData;
+        private final MySleepChartsData pieData;
+
+        public MyChartsData(MySleepChartsData pieData, DefaultChartsData chartsData) {
+            this.pieData = pieData;
+            this.chartsData = chartsData;
+        }
+
+        public MySleepChartsData getPieData() {
+            return pieData;
+        }
+
+        public DefaultChartsData getChartsData() {
+            return chartsData;
+        }
+    }
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/WeekStepsChartFragment.java

@@ -42,19 +42,30 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
     private Locale mLocale;
     private int mTargetSteps = 10000;
 
-    private CombinedChart mWeekStepsChart;
     private PieChart mTodayStepsChart;
+    private CombinedChart mWeekStepsChart;
 
     @Override
-    protected void refreshInBackground(DBHandler db, GBDevice device) {
+    protected ChartsData refreshInBackground(ChartsHost chartsHost, DBHandler db, GBDevice device) {
-        ChartsHost chartsHost = getChartsHost();
+        Calendar day = Calendar.getInstance();
-        if (chartsHost != null) {
+        day.setTime(chartsHost.getEndDate());
-            Calendar day = Calendar.getInstance();
+        //NB: we could have omitted the day, but this way we can move things to the past easily
-            day.setTime(chartsHost.getEndDate());
+        DaySteps daySteps = refreshDaySteps(db, day, device);
-            //NB: we could have omitted the day, but this way we can move things to the past easily
+        DefaultChartsData weekBeforeStepsData = refreshWeekBeforeSteps(db, mWeekStepsChart, day, device);
-            refreshDaySteps(db, mTodayStepsChart, day, device);
+
-            refreshWeekBeforeSteps(db, mWeekStepsChart, day, device);
+        return new MyChartsData(daySteps, weekBeforeStepsData);
-        }
+    }
+
+    @Override
+    protected void updateChartsnUIThread(ChartsData chartsData) {
+        MyChartsData mcd = (MyChartsData) chartsData;
+
+//        setupLegend(mWeekStepsChart);
+        mTodayStepsChart.setCenterText(NumberFormat.getNumberInstance(mLocale).format(mcd.getDaySteps().totalSteps));
+        mTodayStepsChart.setData(mcd.getDaySteps().data);
+
+        mWeekStepsChart.setData(mcd.getWeekBeforeStepsData().getCombinedData());
+        mWeekStepsChart.getLegend().setEnabled(false);
     }
 
     @Override
@@ -63,7 +74,7 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
         mTodayStepsChart.invalidate();
     }
 
-    private void refreshWeekBeforeSteps(DBHandler db, CombinedChart combinedChart, Calendar day, GBDevice device) {
+    private DefaultChartsData refreshWeekBeforeSteps(DBHandler db, CombinedChart combinedChart, Calendar day, GBDevice device) {
 
         ActivityAnalysis analysis = new ActivityAnalysis();
 
@@ -90,18 +101,16 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
 
         CombinedData combinedData = new CombinedData(labels);
         combinedData.setData(barData);
-
+        return new DefaultChartsData(combinedData);
-        setupLegend(combinedChart);
-        combinedChart.setData(combinedData);
-        combinedChart.getLegend().setEnabled(false);
     }
 
-    private void refreshDaySteps(DBHandler db, PieChart pieChart, Calendar day, GBDevice device) {
+
+
+    private DaySteps refreshDaySteps(DBHandler db, Calendar day, GBDevice device) {
         ActivityAnalysis analysis = new ActivityAnalysis();
 
         int totalSteps = analysis.calculateTotalSteps(getSamplesOfDay(db, day, device));
 
-        pieChart.setCenterText(NumberFormat.getNumberInstance(mLocale).format(totalSteps));
         PieData data = new PieData();
         List<Entry> entries = new ArrayList<>();
         List<Integer> colors = new ArrayList<>();
@@ -123,9 +132,8 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
         data.setDataSet(set);
         //this hides the values (numeric) added to the set. These would be shown aside the strings set with addXValue above
         data.setDrawValues(false);
-        pieChart.setData(data);
 
-        pieChart.getLegend().setEnabled(false);
+        return new DaySteps(data, totalSteps);
     }
 
     @Override
@@ -164,6 +172,8 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
         mTodayStepsChart.setDescription(getContext().getString(R.string.weeksteps_today_steps_description, mTargetSteps));
         mTodayStepsChart.setNoDataTextDescription("");
         mTodayStepsChart.setNoDataText("");
+        mTodayStepsChart.getLegend().setEnabled(false);
+//        setupLegend(mTodayStepsChart);
     }
 
     private void setupWeekStepsChart() {
@@ -196,12 +206,12 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
     }
 
     protected void setupLegend(Chart chart) {
-        List<Integer> legendColors = new ArrayList<>(1);
+//        List<Integer> legendColors = new ArrayList<>(1);
-        List<String> legendLabels = new ArrayList<>(1);
+//        List<String> legendLabels = new ArrayList<>(1);
-        legendColors.add(akActivity.color);
+//        legendColors.add(akActivity.color);
-        legendLabels.add(getContext().getString(R.string.chart_steps));
+//        legendLabels.add(getContext().getString(R.string.chart_steps));
-        chart.getLegend().setCustom(legendColors, legendLabels);
+//        chart.getLegend().setCustom(legendColors, legendLabels);
-        chart.getLegend().setTextColor(LEGEND_TEXT_COLOR);
+//        chart.getLegend().setTextColor(LEGEND_TEXT_COLOR);
     }
 
     private List<ActivitySample> getSamplesOfDay(DBHandler db, Calendar day, GBDevice device) {
@@ -226,4 +236,32 @@ public class WeekStepsChartFragment extends AbstractChartFragment {
     protected List<ActivitySample> getSamples(DBHandler db, GBDevice device, int tsFrom, int tsTo) {
         return super.getAllSamples(db, device, tsFrom, tsTo);
     }
+
+    private static class DaySteps {
+        private final PieData data;
+        private final int totalSteps;
+
+        public DaySteps(PieData data, int totalSteps) {
+            this.data = data;
+            this.totalSteps = totalSteps;
+        }
+    }
+
+    private static class MyChartsData extends ChartsData {
+        private final DefaultChartsData weekBeforeStepsData;
+        private final DaySteps daySteps;
+
+        public MyChartsData(DaySteps daySteps, DefaultChartsData weekBeforeStepsData) {
+            this.daySteps = daySteps;
+            this.weekBeforeStepsData = weekBeforeStepsData;
+        }
+
+        public DaySteps getDaySteps() {
+            return daySteps;
+        }
+
+        public DefaultChartsData getWeekBeforeStepsData() {
+            return weekBeforeStepsData;
+        }
+    }
 }