From 7ab31514dc62e81dc191da1a900f1585af8f6b70 Mon Sep 17 00:00:00 2001 From: cpfeiffer Date: Mon, 11 Apr 2016 23:18:12 +0200 Subject: [PATCH] Rework charts to completely fix crash in charts activity #277 --- .../charts/AbstractChartFragment.java | 46 +++++++--- .../charts/ActivitySleepChartFragment.java | 9 +- .../activities/charts/ChartsData.java | 4 + .../charts/LiveActivityFragment.java | 8 +- .../activities/charts/SleepChartFragment.java | 60 +++++++++++-- .../charts/WeekStepsChartFragment.java | 88 +++++++++++++------ 6 files changed, 167 insertions(+), 48 deletions(-) create mode 100644 app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ChartsData.java diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/AbstractChartFragment.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/AbstractChartFragment.java index f163ba65..c32752d3 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/AbstractChartFragment.java +++ b/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 samples) { + protected DefaultChartsData refresh(GBDevice gbDevice, List 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 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.emptyList()); - chart.setData(data); + combinedData = new CombinedData(Collections.emptyList()); } + + 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; + } + } } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ActivitySleepChartFragment.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ActivitySleepChartFragment.java index cd45452d..7c58693e 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ActivitySleepChartFragment.java +++ b/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 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() { diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ChartsData.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/ChartsData.java new file mode 100644 index 00000000..4285951f --- /dev/null +++ b/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 { +} diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/LiveActivityFragment.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/LiveActivityFragment.java index d7b2ca24..f69e2ae5 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/LiveActivityFragment.java +++ b/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 diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/SleepChartFragment.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/SleepChartFragment.java index b0836eeb..4f18495d 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/SleepChartFragment.java +++ b/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 samples = getSamples(db, device); - refresh(device, mActivityChart, samples); - refreshSleepAmounts(device, mSleepAmountChart, samples); + MySleepChartsData mySleepChartsData = refreshSleepAmounts(device, samples); + DefaultChartsData chartsData = refresh(device, samples); + + return new MyChartsData(mySleepChartsData, chartsData); } - private void refreshSleepAmounts(GBDevice mGBDevice, PieChart pieChart, List samples) { + private MySleepChartsData refreshSleepAmounts(GBDevice mGBDevice, List 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; + } + } } \ No newline at end of file diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/WeekStepsChartFragment.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/WeekStepsChartFragment.java index 927a499b..2e271f28 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/charts/WeekStepsChartFragment.java +++ b/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) { - ChartsHost chartsHost = getChartsHost(); - if (chartsHost != null) { - Calendar day = Calendar.getInstance(); - day.setTime(chartsHost.getEndDate()); - //NB: we could have omitted the day, but this way we can move things to the past easily - refreshDaySteps(db, mTodayStepsChart, day, device); - refreshWeekBeforeSteps(db, mWeekStepsChart, day, device); - } + protected ChartsData refreshInBackground(ChartsHost chartsHost, DBHandler db, GBDevice device) { + Calendar day = Calendar.getInstance(); + day.setTime(chartsHost.getEndDate()); + //NB: we could have omitted the day, but this way we can move things to the past easily + DaySteps daySteps = refreshDaySteps(db, day, device); + DefaultChartsData weekBeforeStepsData = 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); - - setupLegend(combinedChart); - combinedChart.setData(combinedData); - combinedChart.getLegend().setEnabled(false); + return new DefaultChartsData(combinedData); } - 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 entries = new ArrayList<>(); List 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 legendColors = new ArrayList<>(1); - List legendLabels = new ArrayList<>(1); - legendColors.add(akActivity.color); - legendLabels.add(getContext().getString(R.string.chart_steps)); - chart.getLegend().setCustom(legendColors, legendLabels); - chart.getLegend().setTextColor(LEGEND_TEXT_COLOR); +// List legendColors = new ArrayList<>(1); +// List legendLabels = new ArrayList<>(1); +// legendColors.add(akActivity.color); +// legendLabels.add(getContext().getString(R.string.chart_steps)); +// chart.getLegend().setCustom(legendColors, legendLabels); +// chart.getLegend().setTextColor(LEGEND_TEXT_COLOR); } private List getSamplesOfDay(DBHandler db, Calendar day, GBDevice device) { @@ -226,4 +236,32 @@ public class WeekStepsChartFragment extends AbstractChartFragment { protected List 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; + } + } }