altosdroid: Deal with bluetooth connection failures better
authorKeith Packard <keithp@keithp.com>
Sat, 7 Mar 2015 18:18:57 +0000 (10:18 -0800)
committerKeith Packard <keithp@keithp.com>
Sat, 7 Mar 2015 18:18:57 +0000 (10:18 -0800)
Remember when we've closed the bluetooth connection so that we stop
operations, including reporting connection status messages or even
starting a connection attempt.

Pass the AltosBluetooth object back in connection status messages so
that TelemetryService can tell when messages from closed objects get
delivered. There's a queue between the two, so the above fix catches
most of these instances, but not all of them.

Stick a delay during reconnect -- if the TeleBT device is getting
power-cycled, it will need a few seconds to reconfigure the device at
startup, if AltosDroid manages to connect during that time, the
configuration commands will be ignored.

Unlock the AltosBluetooth device while we connect so that cancel
calls will actually work.

Signed-off-by: Keith Packard <keithp@keithp.com>
altosdroid/src/org/altusmetrum/AltosDroid/AltosBluetooth.java
altosdroid/src/org/altusmetrum/AltosDroid/AltosDroid.java
altosdroid/src/org/altusmetrum/AltosDroid/TelemetryService.java

index 973250a567c6f754b22dbca84d8d725ec83e4593..da75ffddaf5ffdbd0a8c82049a16cba7f650e751 100644 (file)
@@ -45,24 +45,36 @@ public class AltosBluetooth extends AltosLink {
        private Handler          handler;
 
        private BluetoothAdapter adapter;
-       private BluetoothDevice  device;
        private BluetoothSocket  socket;
        private InputStream      input;
        private OutputStream     output;
 
        // Constructor
-       public AltosBluetooth(BluetoothDevice in_device, Handler in_handler) {
+       public AltosBluetooth(BluetoothDevice device, Handler handler) {
 //             set_debug(D);
                adapter = BluetoothAdapter.getDefaultAdapter();
-               device = in_device;
-               handler = in_handler;
+               this.handler = handler;
 
-               connect_thread = new ConnectThread(device);
+               create_socket(device);
+               connect_thread = new ConnectThread();
                connect_thread.start();
+       }
+
+       private Object closed_lock = new Object();
+       private boolean closed = false;
 
+       private boolean closed() {
+               synchronized(closed_lock) {
+                       return closed;
+               }
        }
 
        private void connected() {
+               if (closed()) {
+                       if (D) Log.d(TAG, "connected after closed");
+                       return;
+               }
+
                try {
                        synchronized(this) {
                                if (socket != null) {
@@ -79,7 +91,7 @@ public class AltosBluetooth extends AltosLink {
 
                                        /* Let TelemetryService know we're connected
                                         */
-                                       handler.obtainMessage(TelemetryService.MSG_CONNECTED).sendToTarget();
+                                       handler.obtainMessage(TelemetryService.MSG_CONNECTED, this).sendToTarget();
 
                                        /* Notify other waiting threads that we're connected now
                                         */
@@ -92,97 +104,104 @@ public class AltosBluetooth extends AltosLink {
        }
 
        private void connect_failed() {
-               synchronized (this) {
-                       if (socket != null) {
-                               try {
-                                       socket.close();
-                               } catch (IOException e2) {
-                                       if (D) Log.e(TAG, "ConnectThread: Failed to close() socket after failed connection");
-                               }
-                               socket = null;
-                       }
-                       input = null;
-                       output = null;
-                       handler.obtainMessage(TelemetryService.MSG_CONNECT_FAILED).sendToTarget();
-                       if (D) Log.e(TAG, "ConnectThread: Failed to establish connection");
+               if (closed()) {
+                       if (D) Log.d(TAG, "connect_failed after closed");
+                       return;
                }
-       }
 
-       private Object closing_lock = new Object();
-       private boolean closing = false;
+               close_socket();
+               input = null;
+               output = null;
+               handler.obtainMessage(TelemetryService.MSG_CONNECT_FAILED, this).sendToTarget();
+               if (D) Log.e(TAG, "ConnectThread: Failed to establish connection");
+       }
 
        private void disconnected() {
-               synchronized(closing_lock) {
-                       if (D) Log.e(TAG, String.format("Connection lost during I/O. Closing  %b", closing));
-                       if (!closing) {
-                               if (D) Log.d(TAG, "Sending disconnected message");
-                               handler.obtainMessage(TelemetryService.MSG_DISCONNECTED).sendToTarget();
-                       }
+               if (closed()) {
+                       if (D) Log.d(TAG, "disconnected after closed");
+                       return;
                }
+
+               if (D) Log.d(TAG, "Sending disconnected message");
+               handler.obtainMessage(TelemetryService.MSG_DISCONNECTED, this).sendToTarget();
        }
 
-       private class ConnectThread extends Thread {
-               private final UUID SPP_UUID = UUID.fromString("00001101-0000-1000-8000-00805F9B34FB");
+       private void close_socket() {
+               BluetoothSocket tmp_socket;
 
-               public ConnectThread(BluetoothDevice device) {
-                       BluetoothSocket tmp_socket = null;
+               synchronized(this) {
+                       tmp_socket = socket;
+                       socket = null;
+               }
 
+               if (tmp_socket != null) {
                        try {
-                               tmp_socket = device.createInsecureRfcommSocketToServiceRecord(SPP_UUID);
+                               tmp_socket.close();
                        } catch (IOException e) {
-                               e.printStackTrace();
+                               if (D) Log.e(TAG, "close_socket failed");
                        }
+               }
+       }
+
+       private final UUID SPP_UUID = UUID.fromString("00001101-0000-1000-8000-00805F9B34FB");
+
+       private void create_socket(BluetoothDevice  device) {
+
+               BluetoothSocket tmp_socket = null;
+
+               try {
+                       tmp_socket = device.createInsecureRfcommSocketToServiceRecord(SPP_UUID);
+               } catch (IOException e) {
+                       e.printStackTrace();
+               }
+               if (socket != null) {
+                       if (D) Log.d(TAG, String.format("Socket already allocated %s", socket.toString()));
+                       close_socket();
+               }
+               synchronized (this) {
                        socket = tmp_socket;
                }
+       }
+
+       private class ConnectThread extends Thread {
 
                public void run() {
                        if (D) Log.d(TAG, "ConnectThread: BEGIN");
                        setName("ConnectThread");
 
                        // Always cancel discovery because it will slow down a connection
-                       adapter.cancelDiscovery();
+                       try {
+                               adapter.cancelDiscovery();
+                       } catch (Exception e) {
+                               if (D) Log.d(TAG, String.format("cancelDiscovery exception %s", e.toString()));
+                       }
 
-                       BluetoothSocket local_socket;
+                       BluetoothSocket local_socket = null;
 
-                       try {
-                               synchronized (AltosBluetooth.this) {
+                       synchronized (AltosBluetooth.this) {
+                               if (!closed())
                                        local_socket = socket;
-                               }
+                       }
 
-                               if (local_socket != null) {
+                       if (local_socket != null) {
+                               try {
                                        // Make a connection to the BluetoothSocket
                                        // This is a blocking call and will only return on a
                                        // successful connection or an exception
                                        local_socket.connect();
+                               } catch (IOException e) {
+                                       if (D) Log.d(TAG, String.format("Connect exception %s", e.toString()));
+                                       local_socket = null;
                                }
+                       }
 
+                       if (local_socket != null) {
                                connected();
-
-                       } catch (IOException e) {
+                       } else {
                                connect_failed();
                        }
 
-                       synchronized (AltosBluetooth.this) {
-                               /* Reset the ConnectThread because we're done
-                                */
-                               connect_thread = null;
-                       }
-                       if (D) Log.d(TAG, "ConnectThread: Connect completed");
-               }
-
-               public void cancel() {
-                       try {
-                               BluetoothSocket local_socket;
-                               synchronized(AltosBluetooth.this) {
-                                       local_socket = socket;
-                                       socket = null;
-                               }
-                               if (local_socket != null)
-                                       local_socket.close();
-
-                       } catch (IOException e) {
-                               if (D) Log.e(TAG, "ConnectThread: close() of connect socket failed", e);
-                       }
+                       if (D) Log.d(TAG, "ConnectThread: completed");
                }
        }
 
@@ -246,6 +265,19 @@ public class AltosBluetooth extends AltosLink {
        private int buffer_len = 0;
        private int buffer_off = 0;
 
+       private byte[] debug_chars = new byte[buffer_size];
+       private int debug_off;
+
+       private void debug_input(byte b) {
+               if (b == '\n') {
+                       Log.d(TAG, "            " + new String(debug_chars, 0, debug_off));
+                       debug_off = 0;
+               } else {
+                       if (debug_off < buffer_size)
+                               debug_chars[debug_off++] = b;
+               }
+       }
+
        public int getchar() {
                while (buffer_off == buffer_len) {
                        try {
@@ -262,13 +294,15 @@ public class AltosBluetooth extends AltosLink {
                                return AltosLink.ERROR;
                        }
                }
+               if (D)
+                       debug_input(buffer[buffer_off]);
                return buffer[buffer_off++];
        }
 
        public void closing() {
-               synchronized(closing_lock) {
-                       if (D) Log.d(TAG, "Marked closing true");
-                       closing = true;
+               synchronized(closed_lock) {
+                       if (D) Log.d(TAG, "Marked closed true");
+                       closed = true;
                }
        }
 
@@ -278,19 +312,10 @@ public class AltosBluetooth extends AltosLink {
 
                closing();
 
+               close_socket();
+
                synchronized(this) {
-                       if (D) Log.d(TAG, "close(): synched");
 
-                       if (socket != null) {
-                               if (D) Log.d(TAG, "close(): Closing socket");
-                               try {
-                                       socket.close();
-                               } catch (IOException e) {
-                                       if (D) Log.e(TAG, "close(): unable to close() socket");
-                               }
-                               socket = null;
-                       }
-                       connect_thread = null;
                        if (input_thread != null) {
                                if (D) Log.d(TAG, "close(): stopping input_thread");
                                try {
@@ -307,14 +332,6 @@ public class AltosBluetooth extends AltosLink {
                }
        }
 
-
-       // We override this method so that we can add some debugging. Not 100% elegant, but more useful
-       // than debugging one char at a time above in getchar()!
-       public void add_reply(AltosLine line) throws InterruptedException {
-               if (D) Log.d(TAG, String.format("Got REPLY: %s", line.line));
-               super.add_reply(line);
-       }
-
        //public void flush_output() { super.flush_output(); }
 
        // Stubs of required methods when extending AltosLink
index 41045f036cf00d61fcca9df035b6d3f6f27b7541..4e7bdd6b331dc7b2d3bd10274d6f75e00f81aef0 100644 (file)
@@ -550,10 +550,12 @@ public class AltosDroid extends FragmentActivity implements AltosUnitsListener {
                        String address = data.getExtras().getString(DeviceListActivity.EXTRA_DEVICE_ADDRESS);
                        String name = data.getExtras().getString(DeviceListActivity.EXTRA_DEVICE_NAME);
 
-                       if (D) Log.d(TAG, "Connecting to " + address + name);
+                       if (D) Log.d(TAG, "Connecting to " + address + " " + name);
                        DeviceAddress   a = new DeviceAddress(address, name);
                        mService.send(Message.obtain(null, TelemetryService.MSG_CONNECT, a));
+                       if (D) Log.d(TAG, "Sent connecting message");
                } catch (RemoteException e) {
+                       if (D) Log.e(TAG, "connect device message failed");
                }
        }
 
index 5f1389725e8aa240e23b77d86f63790fd60d966c..65eabf1166a394f5fdeb4a584dddbd765594b385 100644 (file)
@@ -94,6 +94,7 @@ public class TelemetryService extends Service implements LocationListener {
                @Override
                public void handleMessage(Message msg) {
                        TelemetryService s = service.get();
+                       AltosBluetooth bt = null;
                        if (s == null)
                                return;
                        switch (msg.what) {
@@ -109,7 +110,7 @@ public class TelemetryService extends Service implements LocationListener {
                                if (D) Log.d(TAG, "Connect command received");
                                DeviceAddress address = (DeviceAddress) msg.obj;
                                AltosDroidPreferences.set_active_device(address);
-                               s.start_altos_bluetooth(address);
+                               s.start_altos_bluetooth(address, false);
                                break;
                        case MSG_DISCONNECT:
                                if (D) Log.d(TAG, "Disconnect command received");
@@ -143,6 +144,13 @@ public class TelemetryService extends Service implements LocationListener {
                                 *Messages from AltosBluetooth
                                 */
                        case MSG_CONNECTED:
+                               Log.d(TAG, "MSG_CONNECTED");
+                               bt = (AltosBluetooth) msg.obj;
+
+                               if (bt != s.altos_bluetooth) {
+                                       if (D) Log.d(TAG, "Stale message");
+                                       break;
+                               }
                                if (D) Log.d(TAG, "Connected to device");
                                try {
                                        s.connected();
@@ -150,18 +158,31 @@ public class TelemetryService extends Service implements LocationListener {
                                }
                                break;
                        case MSG_CONNECT_FAILED:
+                               Log.d(TAG, "MSG_CONNECT_FAILED");
+                               bt = (AltosBluetooth) msg.obj;
+
+                               if (bt != s.altos_bluetooth) {
+                                       if (D) Log.d(TAG, "Stale message");
+                                       break;
+                               }
                                if (s.address != null) {
                                        if (D) Log.d(TAG, "Connection failed... retrying");
-                                       s.start_altos_bluetooth(s.address);
+                                       s.start_altos_bluetooth(s.address, true);
                                } else {
                                        s.stop_altos_bluetooth(true);
                                }
                                break;
                        case MSG_DISCONNECTED:
                                Log.d(TAG, "MSG_DISCONNECTED");
+                               bt = (AltosBluetooth) msg.obj;
+
+                               if (bt != s.altos_bluetooth) {
+                                       if (D) Log.d(TAG, "Stale message");
+                                       break;
+                               }
                                if (s.address != null) {
                                        if (D) Log.d(TAG, "Connection lost... retrying");
-                                       s.start_altos_bluetooth(s.address);
+                                       s.start_altos_bluetooth(s.address, true);
                                } else {
                                        s.stop_altos_bluetooth(true);
                                }
@@ -217,7 +238,7 @@ public class TelemetryService extends Service implements LocationListener {
                 */
                if (address != null && telemetry_state.connect == TelemetryState.CONNECT_DISCONNECTED) {
                        if (D) Log.d(TAG, "Reconnecting now...");
-                       start_altos_bluetooth(address);
+                       start_altos_bluetooth(address, false);
                }
        }
 
@@ -292,11 +313,17 @@ public class TelemetryService extends Service implements LocationListener {
                }
        }
 
-       private void start_altos_bluetooth(DeviceAddress address) {
+       private void start_altos_bluetooth(DeviceAddress address, boolean pause) {
                // Get the BLuetoothDevice object
                BluetoothDevice device = bluetooth_adapter.getRemoteDevice(address.address);
 
                stop_altos_bluetooth(false);
+               if (pause) {
+                       try {
+                               Thread.sleep(4000);
+                       } catch (InterruptedException e) {
+                       }
+               }
                this.address = address;
                if (D) Log.d(TAG, String.format("start_altos_bluetooth(): Connecting to %s (%s)", device.getName(), device.getAddress()));
                altos_bluetooth = new AltosBluetooth(device, handler);
@@ -316,7 +343,14 @@ public class TelemetryService extends Service implements LocationListener {
                } catch (TimeoutException e) {
                        // If this timed out, then we really want to retry it, but
                        // probably safer to just retry the connection from scratch.
-                       handler.obtainMessage(MSG_CONNECT_FAILED).sendToTarget();
+                       if (D) Log.d(TAG, "connected timeout");
+                       if (address != null) {
+                               if (D) Log.d(TAG, "connected timeout, retrying");
+                               start_altos_bluetooth(address, true);
+                       } else {
+                               handler.obtainMessage(MSG_CONNECT_FAILED).sendToTarget();
+                               stop_altos_bluetooth(true);
+                       }
                        return;
                }
 
@@ -372,7 +406,7 @@ public class TelemetryService extends Service implements LocationListener {
 
                DeviceAddress address = AltosDroidPreferences.active_device();
                if (address != null)
-                       start_altos_bluetooth(address);
+                       start_altos_bluetooth(address, false);
        }
 
        @Override