Fix #828: do not use an extra thread to update the data

There is no need to create an extra thread just to signal a handler.
The handler can signal itself periodically by using delayed messages.

2 files changed, 87 insertions, 44 deletions

diff --git a/main/src/cgeo/geocaching/cgeonavigate.java b/main/src/cgeo/geocaching/cgeonavigate.java
index 01a9e74..f34e772 100644
--- a/main/src/cgeo/geocaching/cgeonavigate.java
+++ b/main/src/cgeo/geocaching/cgeonavigate.java
@@ -6,14 +6,13 @@ import cgeo.geocaching.geopoint.HumanDistance;
 import cgeo.geocaching.geopoint.IConversion;
 import cgeo.geocaching.maps.CGeoMap;
 import cgeo.geocaching.ui.CompassView;
+import cgeo.geocaching.utils.PeriodicHandler;
 
 import org.apache.commons.lang3.StringUtils;
 
 import android.content.Context;
 import android.content.Intent;
 import android.os.Bundle;
-import android.os.Handler;
-import android.os.Message;
 import android.os.PowerManager;
 import android.util.Log;
 import android.view.Menu;
@@ -51,19 +50,15 @@ public class cgeonavigate extends AbstractActivity {
     private TextView distanceView = null;
     private TextView headingView = null;
     private CompassView compassView = null;
-    private updaterThread updater = null;
-    private Handler updaterHandler = new Handler() {
+    private PeriodicHandler updaterHandler = new PeriodicHandler(20) {
 
         @Override
-        public void handleMessage(Message msg) {
-            try {
-                if (compassView != null && northHeading != null) {
-                    compassView.updateNorth(northHeading, cacheHeading);
-                }
-            } catch (Exception e) {
-                Log.e(Settings.tag, "cgeonavigate.updaterHandler: " + e.toString());
+        public void act() {
+            if (compassView != null && northHeading != null) {
+                compassView.updateNorth(northHeading, cacheHeading);
             }
         }
+
     };
     private String geocode;
 
@@ -125,10 +120,12 @@ public class cgeonavigate extends AbstractActivity {
 
         // get textviews once
         compassView = (CompassView) findViewById(R.id.rose);
+    }
 
-        // start updater thread
-        updater = new updaterThread(updaterHandler);
-        updater.start();
+    @Override
+    public void onStart() {
+        super.onStart();
+        updaterHandler.start();
     }
 
     @Override
@@ -149,12 +146,6 @@ public class cgeonavigate extends AbstractActivity {
         if (pm == null) {
             pm = (PowerManager) getSystemService(Context.POWER_SERVICE);
         }
-
-        // updater thread
-        if (updater == null) {
-            updater = new updaterThread(updaterHandler);
-            updater.start();
-        }
     }
 
     private void setGo4CacheAction() {
@@ -174,6 +165,8 @@ public class cgeonavigate extends AbstractActivity {
             dir = app.removeDir();
         }
 
+        updaterHandler.stop();
+
         super.onStop();
     }
 
@@ -410,30 +403,6 @@ public class cgeonavigate extends AbstractActivity {
         }
     }
 
-    private static class updaterThread extends Thread {
-
-        private Handler handler = null;
-
-        public updaterThread(Handler handlerIn) {
-            handler = handlerIn;
-        }
-
-        @Override
-        public void run() {
-            while (!Thread.currentThread().isInterrupted()) {
-                if (handler != null) {
-                    handler.sendMessage(Message.obtain());
-                }
-
-                try {
-                    Thread.sleep(20);
-                } catch (Exception e) {
-                    Thread.currentThread().interrupt();
-                }
-            }
-        }
-    }
-
     public static void startActivity(final Context context, final String geocode, final String displayedName, final Geopoint coords, final Collection<cgCoord> coordinatesWithType) {
         coordinates.clear();
         if (coordinatesWithType != null) { // avoid possible NPE
diff --git a/main/src/cgeo/geocaching/utils/PeriodicHandler.java b/main/src/cgeo/geocaching/utils/PeriodicHandler.java
new file mode 100644
index 0000000..3f6c345
--- /dev/null
+++ b/main/src/cgeo/geocaching/utils/PeriodicHandler.java
@@ -0,0 +1,74 @@
+package cgeo.geocaching.utils;
+
+import android.os.Handler;
+import android.os.Message;
+
+/**
+ * A PeriodicHandler class helps with the implementation of a periodic
+ * action embedded in a thread with a looper such as the UI thread.
+ * The act() method will be called periodically. The clock may drift
+ * as the implementation does not target real-time actions.
+ *
+ * The handler will be interrupted if the device goes to sleep.
+ *
+ */
+abstract public class PeriodicHandler extends Handler {
+
+    final static private int START = 0;
+    final static private int STOP = 1;
+    final static private int ACT = 2;
+
+    final private long period;
+
+    /**
+     * Create a new PeriodicHandler object.
+     *
+     * @param period
+     *            The period in milliseconds.
+     */
+    public PeriodicHandler(final long period) {
+        this.period = period;
+    }
+
+    /**
+     * Subclasses of PeriodicHandler must implement this method.
+     */
+    abstract public void act();
+
+    @Override
+    public void handleMessage(final Message msg) {
+        switch (msg.what) {
+            case START:
+                removeMessages(ACT);
+                sendEmptyMessage(ACT);
+                break;
+            case STOP:
+                removeMessages(ACT);
+                break;
+            case ACT:
+                sendEmptyMessageDelayed(ACT, period);
+                act();
+        }
+    }
+
+    /**
+     * Start the periodic handler.
+     *
+     * Restarting a handler that is already started will only
+     * reset its clock.
+     */
+    public void start() {
+        sendEmptyMessage(START);
+    }
+
+    /**
+     * Stop the periodic handler.
+     *
+     * If this method is called from the looper thread, this call is
+     * guaranteed to be synchronous.
+     */
+    public void stop() {
+        sendMessageAtFrontOfQueue(obtainMessage(STOP));
+    }
+
+}