Use LocalBroadcast to finish RingtoneActivity from other components

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)
+            LocalBroadcastHelper.sendBroadcast(this, RingtoneActivity.ACTION_FINISH);
-                    .setAction(RingtoneActivity.ACTION_UNBIND);
-            startActivity(intent);
         }
     }
 

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";
+    public static final String ACTION_FINISH = "com.philliphsu.clock2.ringtone.action.UNBIND";
-
-    private Alarm mAlarm;
-    private boolean mBound = false;
 
     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);
+        Intent intent = new Intent(this, RingtoneService.class)
-        bindService(intent, mConnection, Context.BIND_AUTO_CREATE);
+                .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())) {
+        // Notifies alarm missed and stops the service
-                // Someone else wants us to unbind
+        // TODO: Find a better, cleaner way? Starting the service just to set a flag and stop itself
-                sIsAlive = false; // don't wait until onDestroy() to reset
+        // is not very clear from this code. Perhaps the same Broadcast concept as done here.
-                //dismiss();
+        startService(new Intent(this, RingtoneService.class).setAction(RingtoneService.ACTION_NOTIFY_MISSED));
-                unbindAndFinish();
+        finish();
-            } else {
+        startActivity(intent);
-                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);
-            }
-        }
     }
 
     @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() {
+    private void stopAndFinish() {
-        if (mBound) {
+        stopService(new Intent(this, RingtoneService.class));
-            unbindService(mConnection);
-            mBound = false;
-        }
-    }
-
-    private void unbindAndFinish() {
-        unbindService(); // don't wait for finish() to call onDestroy()
         finish();
     }
 
-    private RingtoneService mBoundService;
+    private final BroadcastReceiver mFinishReceiver = new BroadcastReceiver() {
-
-    private ServiceConnection mConnection = new ServiceConnection() {
         @Override
-        public void onServiceConnected(ComponentName name, IBinder service) {
+        public void onReceive(Context context, Intent intent) {
-            mBoundService = ((RingtoneService.RingtoneBinder) service).getService();
+            stopAndFinish();
-            mBoundService.playRingtone(mAlarm);
-            mBoundService.setRingtoneCallback(RingtoneActivity.this);
-            mBound = true;
-        }
-
-        @Override
-        public void onServiceDisconnected(ComponentName name) {
-            mBoundService = null;
-            mBound = false;
         }
     };
 }

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) {
+            finishActivity();
-                // Finish the activity, which fires onDestroy() and then unbinds itself from this service.
+            stopSelf();
-                // 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();
-            }
         }
     };
-    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())) {
+        // TODO: Refactor to use switch block
-            AlarmUtils.snoozeAlarm(this, alarm);
+        if (intent.getAction() == null || intent.getAction().isEmpty()) {
-        } else if (ACTION_DISMISS.equals(intent.getAction())) {
+            playRingtone(alarm);
-            AlarmUtils.cancelAlarm(this, alarm, false); // TODO do we really need to cancel the intent and alarm?
+        } else if (ACTION_NOTIFY_MISSED.equals(intent.getAction())) {
+            mAutoSilenced = true;
+            stopSelf(startId);
+            // Activity finishes itself
         } else {
-            throw new UnsupportedOperationException();
+            if (ACTION_SNOOZE.equals(intent.getAction())) {
-        }
+                AlarmUtils.snoozeAlarm(this, alarm);
-        // ==========================================================================
+            } else if (ACTION_DISMISS.equals(intent.getAction())) {
-        stopSelf(startId);
+                AlarmUtils.cancelAlarm(this, alarm, false); // TODO do we really need to cancel the intent and alarm?
-        if (mRingtoneCallback != null) {
+            } else {
-            mRingtoneCallback.onServiceFinish(); // tell client to unbind from this service
+                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. <b>Clients MUST
-     * unbind from this service immediately after interrupting.</b>
-     */
-    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) {

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() {}
+}