From: Nathaniel Wesley Filardo Date: Wed, 15 Feb 2017 06:21:10 +0000 (-0500) Subject: So yeah, those likely bugs? Bugs. X-Git-Tag: release-1.2~49 X-Git-Url: https://hydra-www.ietfng.org/gitweb/?a=commitdiff_plain;h=89cedd91cb2678d2282440ee08bd1b5b37c4c4e0;p=acmetensortoys-ctfws-android So yeah, those likely bugs? Bugs. Observees now fire off synthetic events when observers are added, so that it should be sufficent to draw the link between two components, as it were. Improve some commentary throughout. MainActivity onDestroy() unregisterService(), since that kind of seems like the right thing to do? --- diff --git a/lib/src/main/java/com/acmetensortoys/ctfwstimer/lib/CtFwSGameState.java b/lib/src/main/java/com/acmetensortoys/ctfwstimer/lib/CtFwSGameState.java index c6f2ef1..5a086ad 100644 --- a/lib/src/main/java/com/acmetensortoys/ctfwstimer/lib/CtFwSGameState.java +++ b/lib/src/main/java/com/acmetensortoys/ctfwstimer/lib/CtFwSGameState.java @@ -204,9 +204,20 @@ public class CtFwSGameState { // Observer interface public interface Observer { + // Called when the game configuration parameters change void onCtFwSConfigure(CtFwSGameState game); + // Called when the game parameters change and at round boundaries during the game; + // this is an excellent thing to hook for UI update (including updating one's own, + // more finely-grained timers). The Now argument captures the state of the game + // immediately prior to the dispatch of callbacks. void onCtFwSNow(CtFwSGameState game, Now now); + // Called when a flag message arrives. void onCtFwSFlags(CtFwSGameState game); + // Called when a human-readable message arrives or when a new game starts (to + // empty the list), and is given the entire set of messages received this game + // (or since the last), even though usually one only cares about the most recent + // entry on the list. We reserve the right to trim this list in the future, but + // at the moment we do not. Callees should not alter the list in any way. void onCtFwSMessage(CtFwSGameState game, List msgs); } final private Set mObsvs = new HashSet<>(); @@ -249,7 +260,17 @@ public class CtFwSGameState { } } public void registerObserver(Observer d) { - synchronized(this) { mObsvs.add(d); } + synchronized(this) { + if (mObsvs.add(d)) { + // Synchronize observer with game state as of right now. + d.onCtFwSConfigure(this); + d.onCtFwSMessage(this, msgs); + if (configured) { + d.onCtFwSFlags(this); + d.onCtFwSNow(this, getNow(mT.wallMS())); + } + } + } } public void unregisterObserver(Observer d) { synchronized(this) { mObsvs.remove(d); } diff --git a/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainActivity.java b/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainActivity.java index 190dc05..0a94e21 100644 --- a/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainActivity.java +++ b/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainActivity.java @@ -13,6 +13,7 @@ import android.support.annotation.StringRes; import android.support.v4.app.DialogFragment; import android.support.v7.app.AppCompatActivity; import android.os.Bundle; +import android.util.Log; import android.view.Menu; import android.view.MenuInflater; import android.view.MenuItem; @@ -81,31 +82,34 @@ public class MainActivity extends AppCompatActivity { mCdl = new CtFwSDisplayLocal(this, new Handler()); } + private ServiceConnection ctfwssc = new ServiceConnection() { + @Override + public void onServiceConnected(ComponentName name, IBinder service) { + mSrvBinder = (MainService.LocalBinder) service; + mSrvBinder.getGameState().registerObserver(mCdl); + mSrvBinder.registerObserver(mSrvObs); + mabh.onStart(MainActivity.this, mSrvBinder); + mSrvBinder.connect(false); + } + + @Override + public void onServiceDisconnected(ComponentName name) { + mSrvBinder = null; + } + }; + @Override public void onStart() { + Log.d("CtFwS", "onStart"); super.onStart(); - bindService(new Intent(this, MainService.class), new ServiceConnection() { - @Override - public void onServiceConnected(ComponentName name, IBinder service) { - mSrvBinder = (MainService.LocalBinder) service; - mSrvBinder.getGameState().registerObserver(mCdl); - mSrvBinder.registerObserver(mSrvObs); - // Fake an initial server event so we draw some text - mSrvObs.onMqttServerEvent(mSrvBinder, mSrvBinder.getServerState()); - mabh.onStart(MainActivity.this, mSrvBinder); - mSrvBinder.connect(false); - } - - @Override - public void onServiceDisconnected(ComponentName name) { - mSrvBinder = null; - } - }, Context.BIND_AUTO_CREATE); + Intent si = new Intent(this, MainService.class); + bindService(si, ctfwssc, Context.BIND_AUTO_CREATE); } @Override protected void onStop() { + Log.d("CtFwS", "onStop"); if (mSrvBinder != null) { mSrvBinder.getGameState().unregisterObserver(mCdl); mSrvBinder.unregisterObserver(mSrvObs); @@ -114,6 +118,14 @@ public class MainActivity extends AppCompatActivity { super.onStop(); } + @Override + protected void onDestroy() { + Log.d("CtFwS", "onDestroy"); + unbindService(ctfwssc); + + super.onDestroy(); + } + // Every good application needs an easter egg private boolean egg = false; @SuppressLint({"SetTextI18n"}) diff --git a/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainService.java b/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainService.java index 467ac8d..096146d 100644 --- a/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainService.java +++ b/mobile/src/main/java/com/acmetensortoys/ctfwstimer/MainService.java @@ -2,7 +2,10 @@ package com.acmetensortoys.ctfwstimer; import android.app.PendingIntent; import android.app.Service; +import android.content.ComponentName; +import android.content.Context; import android.content.Intent; +import android.content.ServiceConnection; import android.content.SharedPreferences; import android.os.Binder; import android.os.Handler; @@ -17,6 +20,7 @@ import com.acmetensortoys.ctfwstimer.lib.CtFwSGameState; import org.eclipse.paho.android.service.MqttAndroidClient; import org.eclipse.paho.android.service.MqttTraceHandler; import org.eclipse.paho.client.mqttv3.IMqttActionListener; +import org.eclipse.paho.client.mqttv3.IMqttAsyncClient; import org.eclipse.paho.client.mqttv3.IMqttDeliveryToken; import org.eclipse.paho.client.mqttv3.IMqttToken; import org.eclipse.paho.client.mqttv3.MqttCallbackExtended; @@ -114,7 +118,6 @@ public class MainService extends Service { public void connectionLost(Throwable cause) { Log.d("CtFwS", "Conn Lost: " + cause, cause); setMSE(MqttServerEvent.MSE_DISCONN); - } @Override @@ -133,30 +136,53 @@ public class MainService extends Service { @Override public void onSuccess(IMqttToken asyncActionToken) { Log.d("CtFwS", "Conn OK 1"); - setMSE(MqttServerEvent.MSE_CONN); + + IMqttAsyncClient c = asyncActionToken.getClient(); + if (c.equals(mMqc)) { + setMSE(MqttServerEvent.MSE_CONN); + } else { + Log.d("Service", "IS STALE CONN"); + try { + c.disconnect().waitForCompletion(); + } catch (MqttException me) { + // Drop it, we've already dropped the client handle + } + } } @Override public void onFailure(IMqttToken asyncActionToken, Throwable exception) { Log.e("CtFws", "Conn Fail", exception); - setMSE(MqttServerEvent.MSE_DISCONN); + if (asyncActionToken.getClient().equals(mMqc)) { + setMSE(MqttServerEvent.MSE_DISCONN); + } else { + Log.d("Service", "IS STALE DISCONN"); + } } }; private synchronized void doMqtt(@Nullable String server) { + Log.d("Service", "domqtt"); + // Hang up on an existing connection, if we have one - synchronized (this) { - if (mMqc != null) { - if (mMqc.isConnected()) { - try { - mMqc.disconnect(); - Log.d("CtFwS", "domqtt disconnected"); - } catch (MqttException me) { - Log.e("CtFwS", "domqtt disconn exn", me); - } - } + if (mMqc != null) { + mMqc.setCallback(null); + + // TODO: This is *really* annoying; we might leak a connection here because + // .disconnect() is so @#*@&#*@^#*&@^ asynchronous it hurts. There appears + // to be no way to force its hand, and adding .waitforcompletion() here just + // causes our app to hang. Bah humbug. Just leak the connection. The server + // will presumably eventually give up on us or something. Man, I have no idea, + // and the documentation for all of this is amazingly lacking. Burn it all down. + try { + mMqc.close(); + } catch (IllegalArgumentException iae) { + Log.d("Service", "domqtt disconn close exn"); + // *&@#&^*#@#&@#&@# } - mMqc = null; + mMqc.unregisterResources(); + } else { + Log.d("Service", "domqtt no client"); } // If we're deliberately disconnecting, tell the service about it. Otherwise, we'll @@ -170,6 +196,7 @@ public class MainService extends Service { // If disconnecting is all we were told to do, we're done. if (server == null) { + mMqc = null; return; } @@ -179,13 +206,11 @@ public class MainService extends Service { // or we won't resubscribe. I think this is github issue eclipse/paho.mqtt.android#170 // but heavens only knows. Whatever, this works for the moment and doesn't leave // stragglers on my server as far as I can tell. - MqttAndroidClient mqc = new MqttAndroidClient(this,server, MqttClient.generateClientId()); - mqc.setCallback(mqttcb); - /* + mMqc = new MqttAndroidClient(this, server, MqttClient.generateClientId()); + mMqc.setCallback(mqttcb); // Debugging aid: trace the paho client internals - mqc.setTraceCallback(mqttth); - mqc.setTraceEnabled(true); - */ + mMqc.setTraceCallback(mqttth); + // mMqc.setTraceEnabled(true); // Ahem. Now then. Connect with *more callbacks*, which will fire off our // subscription requests, which of course have *yet more* callbacks, which @@ -195,18 +220,16 @@ public class MainService extends Service { mco.setCleanSession(true); mco.setAutomaticReconnect(true); mco.setKeepAliveInterval(180); // seconds - synchronized (this) { - if (BuildConfig.DEBUG && mMqc != null) { throw new AssertionError(); } - mMqc = mqc; - } - mqc.connect(mco, null, mqttal); - Log.d("CtFwS", "Connect dispatched"); + mMqc.connect(mco, null, mqttal); + Log.d("Service", "Connect dispatched"); } catch (MqttException e) { - Log.e("CtFwS", "Conn Exn", e); + Log.e("Service", "Conn Exn", e); } } // Must hold strongly since Android only holds weakly once registered. + // TODO: Option for notification persistence during game? + // TODO: Option for polling even after game ends? private final SharedPreferences.OnSharedPreferenceChangeListener mOSPCL = new SharedPreferences.OnSharedPreferenceChangeListener() { @Override @@ -241,6 +264,35 @@ public class MainService extends Service { // User-facing notification // TODO Move to its own display module? + private ServiceConnection userNoteSC; + private void ensureNotification() { + synchronized(this) { + if (userNoteSC == null) { + userNoteSC = new ServiceConnection() { + @Override + public void onServiceConnected(ComponentName name, IBinder service) { + } + + @Override + public void onServiceDisconnected(ComponentName name) { + } + }; + } + bindService(new Intent(MainService.this, MainService.class), userNoteSC, + Context.BIND_AUTO_CREATE); + startForeground(NOTE_ID_USER, userNoteBuilder.build()); + } + } + private void ensureNoNotification() { + synchronized (this) { + if (userNoteSC != null) { + stopForeground(true); + unbindService(userNoteSC); + userNoteSC = null; + } + } + } + private NotificationCompat.Builder userNoteBuilder; private CtFwSGameState.Observer mCgsObserver = new CtFwSGameState.Observer() { @Override @@ -256,10 +308,10 @@ public class MainService extends Service { now.rationale == null ? "Game is afoot!" : now.rationale); userNoteBuilder.setContentText( now.round == 0 ? "Setup phase" : ("Round " + now.round)); - startForeground(NOTE_ID_USER, userNoteBuilder.build()); + ensureNotification(); } else { // game no longer afoot - stopForeground(true); + ensureNoNotification(); } } @@ -276,17 +328,10 @@ public class MainService extends Service { mHandler = new Handler(); - userNoteBuilder = new NotificationCompat.Builder(MainService.this) .setSmallIcon(R.drawable.shield1) .setContentIntent(PendingIntent.getActivity(MainService.this, 0, new Intent(MainService.this, MainActivity.class), 0)); - - SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(this); - synchronized(this) { - sp.registerOnSharedPreferenceChangeListener(mOSPCL); - doMqtt(sp.getString("server", null)); - } } public class LocalBinder extends Binder { @@ -297,18 +342,29 @@ public class MainService extends Service { return mMSE; } - // It should not be necessary to call this execpt at the beginning or to force a reconnect; + // It should not be necessary to call this except at the beginning or to force a reconnect; // most everything else you might want in a connect method is handled by the // OnSharedPreferenceChangeListener listener above. void connect(boolean force) { - if (force || mMSE != MqttServerEvent.MSE_CONN) { - doMqtt(PreferenceManager - .getDefaultSharedPreferences(MainService.this) - .getString("server", null)); + if (force || mMSE != MqttServerEvent.MSE_SUB) { + SharedPreferences sp = PreferenceManager + .getDefaultSharedPreferences(MainService.this); + sp.registerOnSharedPreferenceChangeListener(mOSPCL); + doMqtt(sp.getString("server", null)); } } void registerObserver(Observer o) { - synchronized(this) { mObsvs.add(o); } + synchronized(this) { + if (mObsvs.add(o)) { + // Fire off synthetic deltas to bring the observer up to date. + if (mMqc == null) { + o.onMqttServerChanged(mBinder, null); + } else { + o.onMqttServerChanged(mBinder, mMqc.getServerURI()); + } + o.onMqttServerEvent(mBinder, mMSE); + } + } } void unregisterObserver(Observer o) { synchronized(this) { mObsvs.remove(o); } @@ -318,6 +374,22 @@ public class MainService extends Service { @Override public IBinder onBind(Intent intent) { + Log.d("Service", "onBind: " + intent); return mBinder; } + + @Override + public void onRebind(Intent intent) { + Log.d("Service", "onRebind" + intent); + } + + @Override + public boolean onUnbind(Intent intent) { + Log.d("Service", "onUnbind: " + intent); + // All clients have been unbound, go ahead and disconnect and stop ourselves. + PreferenceManager.getDefaultSharedPreferences(this) + .unregisterOnSharedPreferenceChangeListener(mOSPCL); + doMqtt(null); + return true; + } }