From 1edd0ce02663069a60378276b4e0b8c0c251c1c2 Mon Sep 17 00:00:00 2001 From: Phillip Hsu Date: Tue, 14 Jun 2016 19:36:14 -0700 Subject: [PATCH] Use LocalBroadcast to finish RingtoneActivity from other components --- .../clock2/editalarm/EditAlarmActivity.java | 5 +- .../clock2/ringtone/RingtoneActivity.java | 97 +++++++------------ .../clock2/ringtone/RingtoneService.java | 93 ++++++------------ .../clock2/util/LocalBroadcastHelper.java | 18 ++++ 4 files changed, 88 insertions(+), 125 deletions(-) create mode 100644 app/src/main/java/com/philliphsu/clock2/util/LocalBroadcastHelper.java diff --git a/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java b/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java index 7142208..9075302 100644 --- a/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java +++ b/app/src/main/java/com/philliphsu/clock2/editalarm/EditAlarmActivity.java @@ -30,6 +30,7 @@ import com.philliphsu.clock2.SharedPreferencesHelper; import com.philliphsu.clock2.model.AlarmsRepository; import com.philliphsu.clock2.ringtone.RingtoneActivity; import com.philliphsu.clock2.util.AlarmUtils; +import com.philliphsu.clock2.util.LocalBroadcastHelper; import java.util.Date; @@ -435,9 +436,7 @@ public class EditAlarmActivity extends BaseActivity implements AlarmNumpad.KeyLi public void cancelAlarm(Alarm alarm, boolean showToast) { AlarmUtils.cancelAlarm(this, alarm, showToast); if (RingtoneActivity.isAlive()) { - Intent intent = new Intent(this, RingtoneActivity.class) - .setAction(RingtoneActivity.ACTION_UNBIND); - startActivity(intent); + LocalBroadcastHelper.sendBroadcast(this, RingtoneActivity.ACTION_FINISH); } } 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 8fae4fc..e9ca66b 100644 --- a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java +++ b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneActivity.java @@ -1,11 +1,11 @@ package com.philliphsu.clock2.ringtone; -import android.content.ComponentName; +import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; -import android.content.ServiceConnection; +import android.content.IntentFilter; import android.os.Bundle; -import android.os.IBinder; +import android.support.v4.content.LocalBroadcastManager; import android.support.v7.app.AppCompatActivity; import android.util.Log; import android.view.View; @@ -24,20 +24,18 @@ import static com.philliphsu.clock2.util.Preconditions.checkNotNull; * status bar and navigation/system bar) with user interaction. * * TODO: Make this abstract and make appropriate subclasses for Alarms and Timers. - * TODO: Implement dismiss and extend logic here. */ -public class RingtoneActivity extends AppCompatActivity implements RingtoneService.RingtoneCallback { +public class RingtoneActivity extends AppCompatActivity { private static final String TAG = "RingtoneActivity"; // Shared with RingtoneService public static final String EXTRA_ITEM_ID = "com.philliphsu.clock2.ringtone.extra.ITEM_ID"; - public static final String ACTION_UNBIND = "com.philliphsu.clock2.ringtone.action.UNBIND"; - - private Alarm mAlarm; - private boolean mBound = false; + public static final String ACTION_FINISH = "com.philliphsu.clock2.ringtone.action.UNBIND"; private static boolean sIsAlive = false; + private Alarm mAlarm; + @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); @@ -60,8 +58,9 @@ 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); - Intent intent = new Intent(this, RingtoneService.class); - bindService(intent, mConnection, Context.BIND_AUTO_CREATE); + Intent intent = new Intent(this, RingtoneService.class) + .putExtra(EXTRA_ITEM_ID, id); + startService(intent); // TODO: Butterknife binding Button snooze = (Button) findViewById(R.id.btn_snooze); @@ -80,27 +79,29 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi }); } + @Override + protected void onResume() { + super.onResume(); + LocalBroadcastManager.getInstance(this).registerReceiver( + mFinishReceiver, new IntentFilter(ACTION_FINISH)); + } + + @Override + protected void onPause() { + super.onPause(); + LocalBroadcastManager.getInstance(this).unregisterReceiver(mFinishReceiver); + } + @Override protected void onNewIntent(Intent intent) { //super.onNewIntent(intent); // Not needed since no fragments hosted? - if (mBound) { - if (ACTION_UNBIND.equals(intent.getAction())) { - // Someone else wants us to unbind - sIsAlive = false; // don't wait until onDestroy() to reset - //dismiss(); - unbindAndFinish(); - } else { - mBoundService.interrupt(); // prepare to notify the alarm was missed - /* - // Cannot rely on finish() to call onDestroy() on time before the activity is restarted. - unbindService(); - // Calling recreate() would recreate this with its current intent, not the new intent passed in here. - finish(); - */ - unbindAndFinish(); - startActivity(intent); - } - } + + // Notifies alarm missed and stops the service + // TODO: Find a better, cleaner way? Starting the service just to set a flag and stop itself + // is not very clear from this code. Perhaps the same Broadcast concept as done here. + startService(new Intent(this, RingtoneService.class).setAction(RingtoneService.ACTION_NOTIFY_MISSED)); + finish(); + startActivity(intent); } @Override @@ -136,15 +137,9 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi @Override protected void onDestroy() { super.onDestroy(); - unbindService(); sIsAlive = false; } - @Override - public void onServiceFinish() { - unbindAndFinish(); - } - public static boolean isAlive() { return sIsAlive; } @@ -153,41 +148,23 @@ public class RingtoneActivity extends AppCompatActivity implements RingtoneServi AlarmUtils.snoozeAlarm(this, mAlarm); // Can't call dismiss() because we don't want to also call cancelAlarm()! Why? For example, // we don't want the alarm, if it has no recurrence, to be turned off right now. - unbindAndFinish(); + stopAndFinish(); } private void dismiss() { AlarmUtils.cancelAlarm(this, mAlarm, false); // TODO do we really need to cancel the intent and alarm? - unbindAndFinish(); + stopAndFinish(); } - private void unbindService() { - if (mBound) { - unbindService(mConnection); - mBound = false; - } - } - - private void unbindAndFinish() { - unbindService(); // don't wait for finish() to call onDestroy() + private void stopAndFinish() { + stopService(new Intent(this, RingtoneService.class)); finish(); } - private RingtoneService mBoundService; - - private ServiceConnection mConnection = new ServiceConnection() { + private final BroadcastReceiver mFinishReceiver = new BroadcastReceiver() { @Override - public void onServiceConnected(ComponentName name, IBinder service) { - mBoundService = ((RingtoneService.RingtoneBinder) service).getService(); - mBoundService.playRingtone(mAlarm); - mBoundService.setRingtoneCallback(RingtoneActivity.this); - mBound = true; - } - - @Override - public void onServiceDisconnected(ComponentName name) { - mBoundService = null; - mBound = false; + public void onReceive(Context context, Intent intent) { + stopAndFinish(); } }; } \ No newline at end of file diff --git a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java index 910a755..8abca31 100644 --- a/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java +++ b/app/src/main/java/com/philliphsu/clock2/ringtone/RingtoneService.java @@ -9,7 +9,6 @@ import android.media.AudioManager; import android.media.Ringtone; import android.media.RingtoneManager; import android.net.Uri; -import android.os.Binder; import android.os.Handler; import android.os.IBinder; import android.os.Vibrator; @@ -22,6 +21,7 @@ import com.philliphsu.clock2.Alarm; import com.philliphsu.clock2.R; import com.philliphsu.clock2.model.AlarmsRepository; import com.philliphsu.clock2.util.AlarmUtils; +import com.philliphsu.clock2.util.LocalBroadcastHelper; import static com.philliphsu.clock2.util.DateFormatUtils.formatTime; import static com.philliphsu.clock2.util.Preconditions.checkNotNull; @@ -33,6 +33,8 @@ import static com.philliphsu.clock2.util.Preconditions.checkNotNull; * of the RingtoneService will be tied to that of its RingtoneActivity because users are not likely to * navigate away from the Activity without making an action. But if they do accidentally navigate away, * they have plenty of time to make the desired action via the notification. + * + * TOneverDO: Change this to not be a started service! */ public class RingtoneService extends Service { // TODO: abstract this, make subclasses private static final String TAG = "RingtoneService"; @@ -40,6 +42,8 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc /* TOneverDO: not private */ private static final String ACTION_SNOOZE = "com.philliphsu.clock2.ringtone.action.SNOOZE"; private static final String ACTION_DISMISS = "com.philliphsu.clock2.ringtone.action.DISMISS"; + // public okay + public static final String ACTION_NOTIFY_MISSED = "com.philliphsu.clock2.ringtone.action.NOTIFY_MISSED"; // TODO: Same value as RingtoneActivity.EXTRA_ITEM_ID. Is it important enough to define a different constant? private static final String EXTRA_ITEM_ID = "com.philliphsu.clock2.ringtone.extra.ITEM_ID"; @@ -49,7 +53,6 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc private Alarm mAlarm; private String mNormalRingTime; private boolean mAutoSilenced = false; - private RingtoneCallback mRingtoneCallback; // TODO: Using Handler for this is ill-suited? Alarm ringing could outlast the // application's life. Use AlarmManager API instead. private final Handler mSilenceHandler = new Handler(); @@ -57,51 +60,37 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc @Override public void run() { mAutoSilenced = true; - // don't wait for activity to finish and unbind - // TODO: Is it really crucial for us to stop these immediately? User will probably not notice - // if this alarm gets to the point of auto-silencing. - mRingtone.stop(); - if (mVibrator != null) { - mVibrator.cancel(); - } AlarmUtils.cancelAlarm(RingtoneService.this, mAlarm, false); // TODO do we really need to cancel the alarm and intent? - if (mRingtoneCallback != null) { - // Finish the activity, which fires onDestroy() and then unbinds itself from this service. - // All clients must be unbound before stopSelf() (and stopService()?) will succeed. - // See https://developer.android.com/guide/components/bound-services.html#Lifecycle - // Figure 1 regarding the lifecycle of started and bound services. - mRingtoneCallback.onServiceFinish(); - } + finishActivity(); + stopSelf(); } }; - private final IBinder mBinder = new RingtoneBinder(); @Override public int onStartCommand(Intent intent, int flags, int startId) { - // Although this is a bound service, we override this method because this class is reused for - // handling the notification actions for the presently ringing alarm. - // Although the docs of Context#startService() says this: - // "Using startService() overrides the default service lifetime that is managed by - // bindService(Intent, ServiceConnection, int): it requires the service to remain running until - // stopService(Intent) [or stopSelf()] is called, regardless of whether any clients are connected to it." - // I have found the regardless part does not apply here. You MUST also unbind any clients from this service - // at the same time you stop this service! long id = intent.getLongExtra(EXTRA_ITEM_ID, -1); if (id < 0) throw new IllegalStateException("No item id set"); Alarm alarm = checkNotNull(AlarmsRepository.getInstance(this).getItem(id)); - if (ACTION_SNOOZE.equals(intent.getAction())) { - AlarmUtils.snoozeAlarm(this, alarm); - } else if (ACTION_DISMISS.equals(intent.getAction())) { - AlarmUtils.cancelAlarm(this, alarm, false); // TODO do we really need to cancel the intent and alarm? + // TODO: Refactor to use switch block + if (intent.getAction() == null || intent.getAction().isEmpty()) { + playRingtone(alarm); + } else if (ACTION_NOTIFY_MISSED.equals(intent.getAction())) { + mAutoSilenced = true; + stopSelf(startId); + // Activity finishes itself } else { - throw new UnsupportedOperationException(); - } - // ========================================================================== - stopSelf(startId); - if (mRingtoneCallback != null) { - mRingtoneCallback.onServiceFinish(); // tell client to unbind from this service + if (ACTION_SNOOZE.equals(intent.getAction())) { + AlarmUtils.snoozeAlarm(this, alarm); + } else if (ACTION_DISMISS.equals(intent.getAction())) { + AlarmUtils.cancelAlarm(this, alarm, false); // TODO do we really need to cancel the intent and alarm? + } else { + throw new UnsupportedOperationException(); + } + // ========================================================================== + stopSelf(startId); + finishActivity(); } return START_NOT_STICKY; // If killed while started, don't recreate. Should be sufficient. @@ -135,10 +124,10 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc @Override public IBinder onBind(Intent intent) { - return mBinder; + return null; } - public void playRingtone(@NonNull Alarm alarm) { + private void playRingtone(@NonNull Alarm alarm) { if (mAudioManager == null && mRingtone == null) { mAlarm = checkNotNull(alarm); // TODO: The below call requires a notification, and there is no way to provide one suitable @@ -193,36 +182,16 @@ public class RingtoneService extends Service { // TODO: abstract this, make subc } } - public void setRingtoneCallback(RingtoneCallback callback) { - mRingtoneCallback = callback; - } - - /** - * A way for clients to interrupt the currently running instance of this service. Interrupting - * the service is akin to prematurely auto silencing the ringtone right now. Clients MUST - * unbind from this service immediately after interrupting. - */ - public void interrupt() { - mAutoSilenced = true; - } - - // Needed so clients can get the Service instance and e.g. call setRingtoneCallback(). - public class RingtoneBinder extends Binder { - RingtoneService getService() { - return RingtoneService.this; // Precludes the class from being static! - } - } - - public interface RingtoneCallback { - void onServiceFinish(); - } - // TODO: For Timers, update the foreground notification to say "timer expired". Also, // if Alarms and Timers will have distinct settings for the minutes to silence after, then consider // doing this in the respective subclass of this service. private void scheduleAutoSilence() { int minutes = AlarmUtils.minutesToSilenceAfter(this); - mSilenceHandler.postDelayed(mSilenceRunnable, minutes * 60000); + mSilenceHandler.postDelayed(mSilenceRunnable, /*minutes * 60000*/10000); // TODO: uncomment + } + + private void finishActivity() { + LocalBroadcastHelper.sendBroadcast(this, RingtoneActivity.ACTION_FINISH); } private PendingIntent getPendingIntent(@NonNull String action, Alarm alarm) { diff --git a/app/src/main/java/com/philliphsu/clock2/util/LocalBroadcastHelper.java b/app/src/main/java/com/philliphsu/clock2/util/LocalBroadcastHelper.java new file mode 100644 index 0000000..dfbdc79 --- /dev/null +++ b/app/src/main/java/com/philliphsu/clock2/util/LocalBroadcastHelper.java @@ -0,0 +1,18 @@ +package com.philliphsu.clock2.util; + +import android.content.Context; +import android.content.Intent; +import android.support.v4.content.LocalBroadcastManager; + +/** + * Created by Phillip Hsu on 6/14/2016. + */ +public final class LocalBroadcastHelper { + + /** Sends a local broadcast using an intent with the action specified */ + public static void sendBroadcast(Context context, String action) { + LocalBroadcastManager.getInstance(context).sendBroadcast(new Intent(action)); + } + + private LocalBroadcastHelper() {} +}