From: Keith Packard Date: Sat, 7 Mar 2015 18:18:57 +0000 (-0800) Subject: altosdroid: Deal with bluetooth connection failures better X-Git-Tag: 1.6.0.3~107 X-Git-Url: https://git.gag.com/?p=fw%2Faltos;a=commitdiff_plain;h=cdd7ad469728fde178c69b9c99d70d6e0ab3f12d;hp=d446c90dab0aca7e501a0228f24c210758d84a1d altosdroid: Deal with bluetooth connection failures better 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 --- diff --git a/altosdroid/src/org/altusmetrum/AltosDroid/AltosBluetooth.java b/altosdroid/src/org/altusmetrum/AltosDroid/AltosBluetooth.java index 973250a5..da75ffdd 100644 --- a/altosdroid/src/org/altusmetrum/AltosDroid/AltosBluetooth.java +++ b/altosdroid/src/org/altusmetrum/AltosDroid/AltosBluetooth.java @@ -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 diff --git a/altosdroid/src/org/altusmetrum/AltosDroid/AltosDroid.java b/altosdroid/src/org/altusmetrum/AltosDroid/AltosDroid.java index 41045f03..4e7bdd6b 100644 --- a/altosdroid/src/org/altusmetrum/AltosDroid/AltosDroid.java +++ b/altosdroid/src/org/altusmetrum/AltosDroid/AltosDroid.java @@ -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"); } } diff --git a/altosdroid/src/org/altusmetrum/AltosDroid/TelemetryService.java b/altosdroid/src/org/altusmetrum/AltosDroid/TelemetryService.java index 5f138972..65eabf11 100644 --- a/altosdroid/src/org/altusmetrum/AltosDroid/TelemetryService.java +++ b/altosdroid/src/org/altusmetrum/AltosDroid/TelemetryService.java @@ -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