From c37e9438cfa16b2d2a8196afc4f57e1d1c4bb1cd Mon Sep 17 00:00:00 2001 From: Phillip Hsu Date: Mon, 13 Jun 2016 03:11:06 -0700 Subject: [PATCH] Recurring alarms working --- .../java/com/philliphsu/clock2/Alarm.java | 91 ++++--------------- .../clock2/alarms/dummy/DummyContent.java | 5 +- .../clock2/editalarm/EditAlarmPresenter.java | 10 +- .../clock2/ringtone/RingtoneActivity.java | 13 ++- 4 files changed, 31 insertions(+), 88 deletions(-) diff --git a/app/src/main/java/com/philliphsu/clock2/Alarm.java b/app/src/main/java/com/philliphsu/clock2/Alarm.java index be19f32..2415631 100644 --- a/app/src/main/java/com/philliphsu/clock2/Alarm.java +++ b/app/src/main/java/com/philliphsu/clock2/Alarm.java @@ -11,6 +11,7 @@ import org.json.JSONObject; import java.util.Calendar; import java.util.GregorianCalendar; +import java.util.concurrent.atomic.AtomicLong; import static com.philliphsu.clock2.DaysOfWeek.NUM_DAYS; import static com.philliphsu.clock2.DaysOfWeek.SATURDAY; @@ -26,44 +27,23 @@ public abstract class Alarm implements JsonSerializable { // JSON property names private static final String KEY_SNOOZING_UNTIL_MILLIS = "snoozing_until_millis"; private static final String KEY_ENABLED = "enabled"; + private static final String KEY_RECURRING_DAYS = "recurring_days"; //private static final String KEY_ID = "id"; // Defined in JsonSerializable private static final String KEY_HOUR = "hour"; private static final String KEY_MINUTES = "minutes"; - private static final String KEY_RECURRING_DAYS = "recurring_days"; private static final String KEY_LABEL = "label"; private static final String KEY_RINGTONE = "ringtone"; private static final String KEY_VIBRATES = "vibrates"; - private static final String KEY_RECURRENCE_IDS = "recurrence_ids"; // ========= MUTABLE ============== private long snoozingUntilMillis; private boolean enabled; - - // ------------------------------------------ TODO -------------------------------------------- - // The problem with using a counter to assign the unique ids is that you need to restore the counter - // between application sessions to the last value used. This is something that can be easily forgotten, - // or worse, you can botch the code for the restoration, because it does feel hacky. Especially since - // you are now implementing recurring alarms with their own ids, restoring the counter to the correct - // value can be elusive to get right. Even if you do get it right, you had to painstakingly make sure - // your thought process was correct and know which value/object you would have to read the last value from. - // - // An alternative solution is to use UUID hashcodes as the unique ids. For our purposes, we shouldn't need - // to worry about the truncation of 128-bits to 32-bits because, practically, there aren't going to be that - // many Alarms out in memory to worry about id collisions. A significant pro to this solution is that you - // don't need to reset a counter to its previous value from an earlier session when you deserialize Alarms. - // Once you recreate an instance, the hashcode would be set and it's a done deal. - private final long[] recurrenceIds = new long[NUM_DAYS]; private final boolean[] recurringDays = new boolean[NUM_DAYS]; // ================================ - //public abstract long id(); // TODO: Find the time to change to int? + //public abstract long id(); public abstract int hour(); public abstract int minutes(); - @SuppressWarnings("mutable") - // TODO: Consider using an immutable collection instead - // TODO: Consider maintaining this yourself. There's no practical value to having an array passed - // in during building. - public abstract boolean[] recurringDays(); // array itself is immutable, but elements are not public abstract String label(); public abstract String ringtone(); public abstract boolean vibrates(); @@ -72,29 +52,22 @@ public abstract class Alarm implements JsonSerializable { public static Alarm create(JSONObject jsonObject) { try { - JSONArray a = (JSONArray) jsonObject.get(KEY_RECURRING_DAYS); - boolean[] recurringDays = new boolean[a.length()]; - for (int i = 0; i < recurringDays.length; i++) { - recurringDays[i] = a.getBoolean(i); - } - - a = (JSONArray) jsonObject.get(KEY_RECURRENCE_IDS); - long[] recurrenceIds = new long[a.length()]; - for (int i = 0; i < recurrenceIds.length; i++) { - recurrenceIds[i] = a.getLong(i); - } - Alarm alarm = new AutoValue_Alarm.Builder() .id(jsonObject.getLong(KEY_ID)) .hour(jsonObject.getInt(KEY_HOUR)) .minutes(jsonObject.getInt(KEY_MINUTES)) - .recurringDays(recurringDays) .label(jsonObject.getString(KEY_LABEL)) .ringtone(jsonObject.getString(KEY_RINGTONE)) .vibrates(jsonObject.getBoolean(KEY_VIBRATES)) .rebuild(); alarm.setEnabled(jsonObject.getBoolean(KEY_ENABLED)); alarm.snoozingUntilMillis = jsonObject.getLong(KEY_SNOOZING_UNTIL_MILLIS); + + JSONArray a = (JSONArray) jsonObject.get(KEY_RECURRING_DAYS); + for (int i = 0; i < a.length(); i++) { + alarm.recurringDays[i] = a.getBoolean(i); + } + return alarm; } catch (JSONException e) { throw new RuntimeException(e); @@ -104,12 +77,10 @@ public abstract class Alarm implements JsonSerializable { public static Builder builder() { // Unfortunately, default values must be provided for generated Builders. // Fields that were not set when build() is called will throw an exception. - // TODO: How can QualityMatters get away with not setting defaults????? return new AutoValue_Alarm.Builder() .id(-1) .hour(0) .minutes(0) - .recurringDays(new boolean[NUM_DAYS]) .label("") .ringtone("") .vibrates(false); @@ -145,14 +116,18 @@ public abstract class Alarm implements JsonSerializable { return enabled; } + public boolean[] recurringDays() { + return recurringDays; + } + public void setRecurring(int day, boolean recurring) { checkDay(day); - recurringDays()[day] = recurring; + recurringDays[day] = recurring; } public boolean isRecurring(int day) { checkDay(day); - return recurringDays()[day]; + return recurringDays[day]; } public boolean hasRecurrence() { @@ -161,7 +136,7 @@ public abstract class Alarm implements JsonSerializable { public int numRecurringDays() { int count = 0; - for (boolean b : recurringDays()) + for (boolean b : recurringDays) if (b) count++; return count; } @@ -247,10 +222,10 @@ public abstract class Alarm implements JsonSerializable { return new JSONObject() .put(KEY_SNOOZING_UNTIL_MILLIS, snoozingUntilMillis) .put(KEY_ENABLED, enabled) + .put(KEY_RECURRING_DAYS, new JSONArray(recurringDays)) .put(KEY_ID, id()) .put(KEY_HOUR, hour()) .put(KEY_MINUTES, minutes()) - .put(KEY_RECURRING_DAYS, new JSONArray(recurringDays())) .put(KEY_LABEL, label()) .put(KEY_RINGTONE, ringtone()) .put(KEY_VIBRATES, vibrates()); @@ -261,29 +236,12 @@ public abstract class Alarm implements JsonSerializable { @AutoValue.Builder public abstract static class Builder { - private static long idCount = 0; // TODO: change to AtomicLong? - // Builder is mutable, so these are inherently setter methods. + private static final AtomicLong ID_COUNTER = new AtomicLong(0); // By omitting the set- prefix, we reduce the number of changes required to define the Builder // class after copying and pasting the accessor fields here. public abstract Builder id(long id); public abstract Builder hour(int hour); public abstract Builder minutes(int minutes); - // TODO: If using an immutable collection instead, can use its Builder instance - // and provide an "accumulating" method - /*abstract boolean[] recurringDays(); - public final Builder setRecurring(int day, boolean recurs) { - checkDay(day) - recurringDays()[day] = recurs; - return this; - } - */ - public abstract Builder recurringDays(boolean[] recurringDays); - /* - public final Builder recurringDay(boolean[] recurringDays) { - this.recurringDays = Arrays.copyOf(recurringDays, NUM_DAYS); - return this; - } - */ public abstract Builder label(String label); public abstract Builder ringtone(String ringtone); public abstract Builder vibrates(boolean vibrates); @@ -293,8 +251,7 @@ public abstract class Alarm implements JsonSerializable { /*not public*/abstract Alarm autoBuild(); public Alarm build() { - this.id(++idCount); // TOneverDO: change to post-increment without also adding offset of 1 to idCount in rebuild() - // TODO: Set each recurrenceId in a loop + this.id(ID_COUNTER.incrementAndGet()); // TOneverDO: change to getAndIncrement() without also adding offset of 1 in rebuild() Alarm alarm = autoBuild(); doChecks(alarm); return alarm; @@ -303,8 +260,7 @@ public abstract class Alarm implements JsonSerializable { /** Should only be called when recreating an instance from JSON */ private Alarm rebuild() { Alarm alarm = autoBuild(); - //idCount = alarm.id(); // prevent future instances from id collision - idCount = alarm.recurrenceIds[6]; // the last id set + ID_COUNTER.set(alarm.id()); // prevent future instances from id collision doChecks(alarm); return alarm; } @@ -312,7 +268,6 @@ public abstract class Alarm implements JsonSerializable { private static void doChecks(Alarm alarm) { checkTime(alarm.hour(), alarm.minutes()); - checkRecurringDaysArrayLength(alarm.recurringDays()); } private static void checkDay(int day) { @@ -326,10 +281,4 @@ public abstract class Alarm implements JsonSerializable { throw new IllegalStateException("Hour and minutes invalid"); } } - - private static void checkRecurringDaysArrayLength(boolean[] b) { - if (b.length != NUM_DAYS) { - throw new IllegalStateException("Invalid length for recurring days array"); - } - } } diff --git a/app/src/main/java/com/philliphsu/clock2/alarms/dummy/DummyContent.java b/app/src/main/java/com/philliphsu/clock2/alarms/dummy/DummyContent.java index 5d0872b..e6c1c48 100644 --- a/app/src/main/java/com/philliphsu/clock2/alarms/dummy/DummyContent.java +++ b/app/src/main/java/com/philliphsu/clock2/alarms/dummy/DummyContent.java @@ -1,7 +1,6 @@ package com.philliphsu.clock2.alarms.dummy; import com.philliphsu.clock2.Alarm; -import com.philliphsu.clock2.DaysOfWeek; import java.util.ArrayList; import java.util.List; @@ -37,9 +36,7 @@ public class DummyContent { if (position % 2 == 0) { b.hour(21).minutes(0); } - boolean[] recurrences = new boolean[DaysOfWeek.NUM_DAYS]; - recurrences[0] = true; - Alarm a = b.id(position).recurringDays(recurrences).build(); + Alarm a = b.id(position).build(); a.setEnabled(true); return a; } diff --git a/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmPresenter.java b/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmPresenter.java index 63acb69..2855b4b 100644 --- a/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmPresenter.java +++ b/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmPresenter.java @@ -11,7 +11,6 @@ import com.philliphsu.clock2.model.Repository; import java.util.Date; -import static com.philliphsu.clock2.DaysOfWeek.NUM_DAYS; import static com.philliphsu.clock2.DaysOfWeek.SATURDAY; import static com.philliphsu.clock2.DaysOfWeek.SUNDAY; import static com.philliphsu.clock2.util.Preconditions.checkNotNull; @@ -61,19 +60,18 @@ public class EditAlarmPresenter implements EditAlarmContract.Presenter { return; } - boolean[] days = new boolean[NUM_DAYS]; - for (int i = SUNDAY; i <= SATURDAY; i++) { - days[i] = mView.isRecurringDay(i); - } Alarm a = Alarm.builder() .hour(hour) .minutes(minutes) .ringtone(mView.getRingtone()) - .recurringDays(days) // TODO: See https://github.com/google/auto/blob/master/value/userguide/howto.md#mutable_property .label(mView.getLabel()) .vibrates(mView.vibrates()) .build(); a.setEnabled(mView.isEnabled()); + for (int i = SUNDAY; i <= SATURDAY; i++) { + a.setRecurring(i, mView.isRecurringDay(i)); + } + if (mAlarm != null) { if (mAlarm.isEnabled()) { Log.d(TAG, "Cancelling old alarm first"); diff --git a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java index b6e7a8c..26dc55c 100644 --- a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java +++ b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java @@ -60,13 +60,6 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi // This could be the case if we're starting a new instance of this activity after leaving the first launch. AlarmUtils.removeUpcomingAlarmNotification(this, mAlarm); - // As of API 19, alarms scheduled with AlarmManager.setRepeating() are inexact. The recommended - // workaround is to schedule one-time exact alarms, and reschedule each time after handling - // an alarm delivery. - if (mAlarm.hasRecurrence()) { - AlarmUtils.scheduleAlarm(this, mAlarm, false /*show toast?*/); - } - Intent intent = new Intent(this, RingtoneService.class); bindService(intent, mConnection, Context.BIND_AUTO_CREATE); @@ -165,6 +158,12 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi private void dismiss() { AlarmUtils.cancelAlarm(this, mAlarm, false); + // As of API 19, alarms scheduled with AlarmManager.setRepeating() are inexact. The recommended + // workaround is to schedule one-time exact alarms, and reschedule each time after handling + // an alarm delivery. + if (mAlarm.hasRecurrence()) { + AlarmUtils.scheduleAlarm(this, mAlarm, false /*show toast?*/); + } unbindAndFinish(); }