From 666d44971ba2e60ef5c20e12d7178eae3a512a83 Mon Sep 17 00:00:00 2001 From: Valter Costa Date: Fri, 16 Dec 2022 12:08:01 +0000 Subject: [PATCH] Improve ant reliability (#62) * Gracefully disconnect before destroy * Formatted code; Add check for service bind * Improve handle channel death and ANT timeouts * Made executor service not static --- .../java/com/valterc/ki2/ant/AntManager.java | 38 ++--- .../ant/connection/AntDeviceConnection.java | 137 ++++++++++++------ .../com/valterc/ki2/services/Ki2Service.java | 2 + 3 files changed, 117 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/com/valterc/ki2/ant/AntManager.java b/app/src/main/java/com/valterc/ki2/ant/AntManager.java index f85cf5d7..209ccb45 100644 --- a/app/src/main/java/com/valterc/ki2/ant/AntManager.java +++ b/app/src/main/java/com/valterc/ki2/ant/AntManager.java @@ -57,7 +57,7 @@ public void onServiceConnected(ComponentName componentName, IBinder service) { public void onServiceDisconnected(ComponentName componentName) { antService = null; antChannelProvider = null; - Timber.i("ANT service disconnected"); + Timber.w("ANT service disconnected"); triggerStateChange(); } }; @@ -78,11 +78,15 @@ public AntManager(Context context, IAntStateListener stateListener) { this.stateListener = stateListener; antServiceBound = AntService.bindService(context, antServiceConnection); - Timber.i("ANT service bind: %s", antServiceBound); + Timber.i("ANT service bound: %s", antServiceBound); + + if (!antServiceBound) { + throw new RuntimeException("Unable to bound to ANT service"); + } } private void triggerStateChange() { - if (stateListener != null){ + if (stateListener != null) { stateListener.onAntStateChange(isReady()); } } @@ -110,9 +114,8 @@ public void dispose() { * @return Number of available channels from this provider. * @throws Exception If the ANT service becomes unavailable. */ - public int getAvailableChannelCount() throws Exception{ - if (!isReady()) - { + public int getAvailableChannelCount() throws Exception { + if (!isReady()) { throw new RuntimeException("ANT channel provider is not available"); } @@ -123,8 +126,8 @@ public int getAvailableChannelCount() throws Exception{ * Get an ANT channel. The channel will be open. * * @param channelConfiguration Channel configuration. - * @param channelEventHandler Optional channel event handler. - * @param adapterEventHandler Optional adapter event handler. + * @param channelEventHandler Optional channel event handler. + * @param adapterEventHandler Optional adapter event handler. * @return ANT channel wrapper. * @throws Exception If the ANT service becomes unavailable. */ @@ -132,8 +135,7 @@ public AntChannelWrapper getAntChannel( ChannelConfiguration channelConfiguration, IAntChannelEventHandler channelEventHandler, IAntAdapterEventHandler adapterEventHandler) throws Exception { - if (!isReady()) - { + if (!isReady()) { throw new RuntimeException("ANT channel provider is not available"); } @@ -143,7 +145,7 @@ public AntChannelWrapper getAntChannel( NetworkKey networkKey = channelConfiguration.getNetworkKey(); if (networkKey != null) { antChannel = antChannelProvider.acquireChannelOnPrivateNetwork(context, networkKey); - }else { + } else { antChannel = antChannelProvider.acquireChannel(this.context, PredefinedNetwork.ANT_PLUS); } } catch (Exception e) { @@ -188,18 +190,16 @@ public AntChannelWrapper getAntChannel( * Get an ANT scan channel. The channel will be open. * * @param scanChannelConfiguration Scan channel configuration. - * @param channelEventHandler Optional channel event handler. - * @param adapterEventHandler Optional adapter event handler. + * @param channelEventHandler Optional channel event handler. + * @param adapterEventHandler Optional adapter event handler. * @return ANT channel wrapper. * @throws Exception If the ANT service becomes unavailable. */ public AntChannelWrapper getScanAntChannel( ScanChannelConfiguration scanChannelConfiguration, IAntChannelEventHandler channelEventHandler, - IAntAdapterEventHandler adapterEventHandler) throws Exception - { - if (!isReady()) - { + IAntAdapterEventHandler adapterEventHandler) throws Exception { + if (!isReady()) { throw new RuntimeException("ANT channel provider is not available"); } @@ -214,7 +214,7 @@ public AntChannelWrapper getScanAntChannel( NetworkKey networkKey = scanChannelConfiguration.getNetworkKey(); if (networkKey != null) { antChannel = antChannelProvider.acquireChannelOnPrivateNetwork(context, networkKey, capabilities); - }else { + } else { antChannel = antChannelProvider.acquireChannel(this.context, PredefinedNetwork.ANT_PLUS, capabilities); } } catch (Exception e) { @@ -255,7 +255,7 @@ public AntChannelWrapper getScanAntChannel( * * @return True if ANT is ready, False otherwise. */ - public boolean isReady(){ + public boolean isReady() { return antChannelProvider != null; } diff --git a/app/src/main/java/com/valterc/ki2/ant/connection/AntDeviceConnection.java b/app/src/main/java/com/valterc/ki2/ant/connection/AntDeviceConnection.java index 0e847e82..0d2e4b8c 100644 --- a/app/src/main/java/com/valterc/ki2/ant/connection/AntDeviceConnection.java +++ b/app/src/main/java/com/valterc/ki2/ant/connection/AntDeviceConnection.java @@ -21,62 +21,123 @@ import java.util.LinkedList; import java.util.Queue; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; import timber.log.Timber; public class AntDeviceConnection implements IAntDeviceConnection, IDeviceConnectionListener { private static final int MAX_RECONNECT_ATTEMPTS = 10; + private static final int TIME_MS_MESSAGE_TIMEOUT = 30_000; private final AntManager antManager; private final DeviceId deviceId; private final ChannelConfiguration channelConfiguration; private final IDeviceConnectionListener deviceConnectionListener; private final Queue> messageQueue; + private final ScheduledExecutorService executorService; private ConnectionStatus connectionStatus; private AntChannelWrapper antChannelWrapper; private ITransportHandler transportHandler; - private int searchReconnectAttempts; + private long timestampLastMessage; + private int reconnectAttempts; + private boolean disconnected; - public AntDeviceConnection(AntManager antManager, DeviceId deviceId, ChannelConfiguration channelConfiguration, IDeviceConnectionListener deviceConnectionListener) throws Exception { + public AntDeviceConnection(AntManager antManager, DeviceId deviceId, ChannelConfiguration channelConfiguration, IDeviceConnectionListener deviceConnectionListener) { this.antManager = antManager; this.deviceId = deviceId; this.channelConfiguration = channelConfiguration; this.deviceConnectionListener = deviceConnectionListener; this.messageQueue = new LinkedList<>(); - onConnectionStatus(deviceId, ConnectionStatus.NEW); - connect(antManager); + this.executorService = Executors.newSingleThreadScheduledExecutor(); + + postConnectionStatus(deviceId, ConnectionStatus.NEW); + executorService.schedule(this::connectionTracker, 1, TimeUnit.MINUTES); + connect(); } - private void connect(AntManager antManager) throws Exception { + private void connectInternal() throws Exception { antChannelWrapper = antManager.getAntChannel(channelConfiguration, new IAntChannelEventHandler() { @Override public void onReceiveMessage(MessageFromAntType messageFromAntType, AntMessageParcel antMessageParcel) { + if (disconnected) { + disconnectInternal(); + return; + } + + timestampLastMessage = System.currentTimeMillis(); forwardMessage(messageFromAntType, antMessageParcel); } @Override public void onChannelDeath() { - Timber.d("Channel died for device %s", deviceId); - - disconnect(); - - if (antManager.isReady()) { - try { - connect(antManager); - } catch (Exception e) { - Timber.e(e, "Unable to restart connection for deviceId %s", deviceId); - } - } + Timber.w("[%s] Channel died", deviceId); + disconnectInternal(); + connect(); } }, null); - onConnectionStatus(deviceId, ConnectionStatus.CONNECTING); transportHandler = new TransportHandler(deviceId, this, this); pushQueuedMessages(); } + private void attemptConnect() { + if (disconnected || connectionStatus == ConnectionStatus.CLOSED) { + return; + } + + try { + Timber.d("[%s] Starting connect procedure", deviceId); + connectInternal(); + } catch (Exception e) { + Timber.w(e, "[%s] Unable to start connect procedure", deviceId); + + if (reconnectAttempts < MAX_RECONNECT_ATTEMPTS) { + reconnectAttempts++; + if (!disconnected && !executorService.isShutdown()) { + Timber.d("[%s] Retrying connection, attempt %d...", deviceId, reconnectAttempts); + executorService.schedule(this::attemptConnect, 2, TimeUnit.SECONDS); + } + } else { + postConnectionStatus(deviceId, ConnectionStatus.CLOSED); + } + } + } + + private void connect() { + if (disconnected || connectionStatus == ConnectionStatus.CLOSED) { + return; + } + + postConnectionStatus(deviceId, ConnectionStatus.CONNECTING); + + if (!disconnected && !executorService.isShutdown()) { + executorService.execute(this::attemptConnect); + } + } + + private void connectionTracker() { + if (disconnected || connectionStatus == ConnectionStatus.CLOSED) { + return; + } + + if (connectionStatus == ConnectionStatus.ESTABLISHED) { + if (System.currentTimeMillis() - timestampLastMessage > TIME_MS_MESSAGE_TIMEOUT) { + timestampLastMessage = System.currentTimeMillis(); + Timber.w("[%s] No ANT messages in last 30 seconds, restarting connection...", deviceId); + disconnectInternal(); + connect(); + } + } + + if (!disconnected && !executorService.isShutdown()) { + executorService.schedule(this::connectionTracker, 1, TimeUnit.MINUTES); + } + } + private void forwardMessage(MessageFromAntType messageFromAntType, AntMessageParcel antMessageParcel) { if (transportHandler == null) { messageQueue.add(new Pair<>(messageFromAntType, antMessageParcel)); @@ -88,7 +149,6 @@ private void forwardMessage(MessageFromAntType messageFromAntType, AntMessagePar private synchronized void pushQueuedMessages() { ITransportHandler transportHandler = this.transportHandler; - if (transportHandler == null) { return; } @@ -101,7 +161,7 @@ private synchronized void pushQueuedMessages() { } } - private void disconnectInternal(){ + private void disconnectInternal() { messageQueue.clear(); AntChannelWrapper antChannelWrapper = this.antChannelWrapper; this.antChannelWrapper = null; @@ -126,29 +186,29 @@ public DeviceId getDeviceId() { @Override public void disconnect() { + disconnected = true; + executorService.shutdownNow(); disconnectInternal(); - searchReconnectAttempts = MAX_RECONNECT_ATTEMPTS; + reconnectAttempts = MAX_RECONNECT_ATTEMPTS; if (this.connectionStatus != ConnectionStatus.CLOSED) { postConnectionStatus(deviceId, ConnectionStatus.CLOSED); } } @Override - public ConnectionStatus getConnectionStatus(){ + public ConnectionStatus getConnectionStatus() { return connectionStatus; } @Override public void sendCommand(CommandType commandType, Parcelable data) { ITransportHandler transportHandler = this.transportHandler; - - if (transportHandler == null){ + if (transportHandler == null) { return; } IDeviceProfileHandler deviceProfileHandler = transportHandler.getDeviceProfileHandler(); - - if (deviceProfileHandler == null){ + if (deviceProfileHandler == null) { return; } @@ -156,14 +216,14 @@ public void sendCommand(CommandType commandType, Parcelable data) { } public void sendAcknowledgedData(byte[] payload) throws RemoteException, AntCommandFailedException { - AntChannelWrapper antChannelWrapper = this.antChannelWrapper; + AntChannelWrapper antChannelWrapper = this.antChannelWrapper; if (antChannelWrapper != null) { antChannelWrapper.sendAcknowledgedData(payload); } } public void setBroadcastData(byte[] payload) throws RemoteException { - AntChannelWrapper antChannelWrapper = this.antChannelWrapper; + AntChannelWrapper antChannelWrapper = this.antChannelWrapper; if (antChannelWrapper != null) { antChannelWrapper.setBroadcastData(payload); } @@ -171,22 +231,16 @@ public void setBroadcastData(byte[] payload) throws RemoteException { @Override public void onConnectionStatus(DeviceId deviceId, ConnectionStatus connectionStatus) { - if (connectionStatus == ConnectionStatus.ESTABLISHED) { - searchReconnectAttempts = 0; - } else if (connectionStatus == ConnectionStatus.CLOSED && searchReconnectAttempts < MAX_RECONNECT_ATTEMPTS) { - searchReconnectAttempts++; - connectionStatus = ConnectionStatus.CONNECTING; + reconnectAttempts = 0; + } - try { - Timber.d("[%s] Retrying connection, attempt %d...", deviceId, searchReconnectAttempts); - disconnectInternal(); - connect(antManager); - } catch (Exception e) { - Timber.e(e, "Unable to connect"); - disconnectInternal(); - connectionStatus = ConnectionStatus.CLOSED; - } + if (connectionStatus == ConnectionStatus.CLOSED && reconnectAttempts < MAX_RECONNECT_ATTEMPTS) { + reconnectAttempts++; + Timber.d("[%s] Retrying connection to device, attempt %d...", deviceId, reconnectAttempts); + disconnectInternal(); + connect(); + return; } postConnectionStatus(deviceId, connectionStatus); @@ -198,4 +252,5 @@ public void onData(DeviceId deviceId, DataType dataType, Parcelable data) { deviceConnectionListener.onData(deviceId, dataType, data); } } + } diff --git a/app/src/main/java/com/valterc/ki2/services/Ki2Service.java b/app/src/main/java/com/valterc/ki2/services/Ki2Service.java index 135fa3fb..a09a8ff9 100644 --- a/app/src/main/java/com/valterc/ki2/services/Ki2Service.java +++ b/app/src/main/java/com/valterc/ki2/services/Ki2Service.java @@ -432,6 +432,8 @@ public void onCreate() { @Override public void onDestroy() { + antConnectionManager.disconnectAll(); + antManager.dispose(); antManager = null;