Fixed default payload size when a non-standard MTU was negotiated.

Summary: #161 Reviewers: pawel.urban Reviewed By: pawel.urban Differential Revision: https://phabricator.polidea.com/D2286
dariuszseweryn · Apr 6, 2017 · 88ee8c5 · 88ee8c5
1 parent 30e8258
commit 88ee8c5
Show file tree

Hide file tree

Showing 10 changed files with 113 additions and 38 deletions.
diff --git a/rxandroidble/src/main/java/com/polidea/rxandroidble/RxBleConnection.java b/rxandroidble/src/main/java/com/polidea/rxandroidble/RxBleConnection.java
@@ -27,6 +27,20 @@
  */
 public interface RxBleConnection {
 
+    /**
+     * The overhead value that is subtracted from the amount of bytes available when writing to a characteristic.
+     * The default MTU value on Android is 23 bytes which gives effectively 23 - GATT_WRITE_MTU_OVERHEAD = 20 bytes
+     * available for payload.
+     */
+    int GATT_WRITE_MTU_OVERHEAD = 3;
+
+    /**
+     * The overhead value that is subtracted from the amount of bytes available when reading from a characteristic.
+     * The default MTU value on Android is 23 bytes which gives effectively 23 - GATT_READ_MTU_OVERHEAD = 22 bytes
+     * available for payload.
+     */
+    int GATT_READ_MTU_OVERHEAD = 1;
+
     interface Connector {
 
         Observable<RxBleConnection> prepareConnection(boolean autoConnect);

diff --git a/...droidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionModule.java b/...droidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionModule.java
@@ -6,23 +6,25 @@
 import com.polidea.rxandroidble.internal.operations.OperationsProviderImpl;
 import dagger.Module;
 import dagger.Provides;
-import java.util.concurrent.Callable;
 import javax.inject.Named;
 
 @Module
 public class ConnectionModule {
 
-    public static final String CURRENT_MTU = "current-mtu";
+    private static final String CURRENT_MTU_PROVIDER = "current-mtu-provider";
+    static final String CURRENT_MAX_WRITE_PAYLOAD_SIZE_PROVIDER = "current-max-write-batch-size-provider";
 
     @Provides
-    @Named(CURRENT_MTU)
-    Callable<Integer> provideCurrentMtuProvider(final RxBleConnectionImpl rxBleConnection) {
-        return new Callable<Integer>() {
-            @Override
-            public Integer call() throws Exception {
-                return rxBleConnection.currentMtu;
-            }
-        };
+    @Named(CURRENT_MTU_PROVIDER)
+    IntProvider provideMtuProvider(RxBleConnectionImpl rxBleConnectionImpl) {
+        return rxBleConnectionImpl;
+    }
+
+    @Provides
+    @ConnectionScope
+    @Named(CURRENT_MAX_WRITE_PAYLOAD_SIZE_PROVIDER)
+    IntProvider provideWriteBatchSizeProvider(@Named(CURRENT_MTU_PROVIDER) IntProvider mtuProvider) {
+        return new MaxWritePayloadSizeProvider(mtuProvider, RxBleConnection.GATT_WRITE_MTU_OVERHEAD);
     }
 
     @Provides

diff --git a/rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/IntProvider.java b/rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/IntProvider.java
@@ -0,0 +1,10 @@
+package com.polidea.rxandroidble.internal.connection;
+
+
+import android.support.annotation.RestrictTo;
+
+@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
+public interface IntProvider {
+
+    int getValue();
+}
diff --git a/...main/java/com/polidea/rxandroidble/internal/connection/LongWriteOperationBuilderImpl.java b/...main/java/com/polidea/rxandroidble/internal/connection/LongWriteOperationBuilderImpl.java
@@ -8,7 +8,6 @@
 import com.polidea.rxandroidble.internal.operations.OperationsProvider;
 
 import java.util.UUID;
-import java.util.concurrent.Callable;
 
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -23,20 +22,20 @@ public final class LongWriteOperationBuilderImpl implements RxBleConnection.Long
     private final OperationsProvider operationsProvider;
 
     private Observable<BluetoothGattCharacteristic> writtenCharacteristicObservable;
-    private Callable<Integer> maxBatchSizeCallable;
+    private IntProvider maxBatchSizeProvider;
     private RxBleConnection.WriteOperationAckStrategy writeOperationAckStrategy = new ImmediateSerializedBatchAckStrategy();
 
     private byte[] bytes;
 
     @Inject
     LongWriteOperationBuilderImpl(
             RxBleRadio rxBleRadio,
-            @Named(ConnectionModule.CURRENT_MTU) Callable<Integer> defaultMaxBatchSizeCallable,
+            @Named(ConnectionModule.CURRENT_MAX_WRITE_PAYLOAD_SIZE_PROVIDER) IntProvider defaultMaxBatchSizeCallable,
             RxBleConnection rxBleConnection,
             OperationsProvider operationsProvider
     ) {
         this.rxBleRadio = rxBleRadio;
-        this.maxBatchSizeCallable = defaultMaxBatchSizeCallable;
+        this.maxBatchSizeProvider = defaultMaxBatchSizeCallable;
         this.rxBleConnection = rxBleConnection;
         this.operationsProvider = operationsProvider;
     }
@@ -61,9 +60,9 @@ public RxBleConnection.LongWriteOperationBuilder setCharacteristic(@NonNull Blue
 
     @Override
     public RxBleConnection.LongWriteOperationBuilder setMaxBatchSize(final int maxBatchSize) {
-        this.maxBatchSizeCallable = new Callable<Integer>() {
+        this.maxBatchSizeProvider = new IntProvider() {
             @Override
-            public Integer call() throws Exception {
+            public int getValue() {
                 return maxBatchSize;
             }
         };
@@ -92,7 +91,7 @@ public Observable<byte[]> build() {
             public Observable<byte[]> call(BluetoothGattCharacteristic bluetoothGattCharacteristic) {
                 return rxBleRadio.queue(
                         operationsProvider.provideLongWriteOperation(bluetoothGattCharacteristic,
-                                writeOperationAckStrategy, maxBatchSizeCallable, bytes)
+                                writeOperationAckStrategy, maxBatchSizeProvider, bytes)
                 );
             }
         });

diff --git a/...c/main/java/com/polidea/rxandroidble/internal/connection/MaxWritePayloadSizeProvider.java b/...c/main/java/com/polidea/rxandroidble/internal/connection/MaxWritePayloadSizeProvider.java
@@ -0,0 +1,21 @@
+package com.polidea.rxandroidble.internal.connection;
+
+
+import android.support.annotation.RestrictTo;
+
+@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
+class MaxWritePayloadSizeProvider implements IntProvider {
+
+    private final IntProvider mtuProvider;
+    private final int gattWriteMtuOverhead;
+
+    MaxWritePayloadSizeProvider(IntProvider mtuProvider, int gattWriteMtuOverhead) {
+        this.mtuProvider = mtuProvider;
+        this.gattWriteMtuOverhead = gattWriteMtuOverhead;
+    }
+
+    @Override
+    public int getValue() {
+        return mtuProvider.getValue() - gattWriteMtuOverhead;
+    }
+}
diff --git a/...idble/src/main/java/com/polidea/rxandroidble/internal/connection/RxBleConnectionImpl.java b/...idble/src/main/java/com/polidea/rxandroidble/internal/connection/RxBleConnectionImpl.java
@@ -49,7 +49,7 @@
 import static rx.Observable.just;
 
 @ConnectionScope
-public class RxBleConnectionImpl implements RxBleConnection {
+public class RxBleConnectionImpl implements RxBleConnection, IntProvider {
 
     static final UUID CLIENT_CHARACTERISTIC_CONFIG_UUID = UUID.fromString("00002902-0000-1000-8000-00805f9b34fb");
     private final RxBleRadio rxBleRadio;
@@ -62,7 +62,7 @@ public class RxBleConnectionImpl implements RxBleConnection {
 
     private final HashMap<CharacteristicNotificationId, Observable<Observable<byte[]>>> notificationObservableMap = new HashMap<>();
     private final HashMap<CharacteristicNotificationId, Observable<Observable<byte[]>>> indicationObservableMap = new HashMap<>();
-    Integer currentMtu = 20; // Default value at the beginning
+    private int currentMtu = 23; // Default value at the beginning
 
     @Inject
     public RxBleConnectionImpl(
@@ -491,4 +491,11 @@ protected BleException provideException(DeadObjectException deadObjectException)
             }
         });
     }
+
+    // IntProvider
+
+    @Override
+    public int getValue() {
+        return currentMtu;
+    }
 }
diff --git a/...oidble/src/main/java/com/polidea/rxandroidble/internal/operations/OperationsProvider.java b/...oidble/src/main/java/com/polidea/rxandroidble/internal/operations/OperationsProvider.java
@@ -6,16 +6,16 @@
 import android.support.annotation.RequiresApi;
 
 import com.polidea.rxandroidble.RxBleConnection;
+import com.polidea.rxandroidble.internal.connection.IntProvider;
 
-import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
 public interface OperationsProvider {
 
     RxBleRadioOperationCharacteristicLongWrite provideLongWriteOperation(
             BluetoothGattCharacteristic bluetoothGattCharacteristic,
             RxBleConnection.WriteOperationAckStrategy writeOperationAckStrategy,
-            Callable<Integer> maxBatchSizeCallable,
+            IntProvider maxBatchSizeProvider,
             byte[] bytes);
 
     @RequiresApi(api = Build.VERSION_CODES.LOLLIPOP)

diff --git a/...le/src/main/java/com/polidea/rxandroidble/internal/operations/OperationsProviderImpl.java b/...le/src/main/java/com/polidea/rxandroidble/internal/operations/OperationsProviderImpl.java
@@ -9,9 +9,9 @@
 import com.polidea.rxandroidble.ClientComponent;
 import com.polidea.rxandroidble.RxBleConnection;
 import com.polidea.rxandroidble.internal.DeviceModule;
+import com.polidea.rxandroidble.internal.connection.IntProvider;
 import com.polidea.rxandroidble.internal.connection.RxBleGattCallback;
 
-import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
 import javax.inject.Inject;
@@ -49,15 +49,15 @@ public class OperationsProviderImpl implements OperationsProvider {
     public RxBleRadioOperationCharacteristicLongWrite provideLongWriteOperation(
             BluetoothGattCharacteristic bluetoothGattCharacteristic,
             RxBleConnection.WriteOperationAckStrategy writeOperationAckStrategy,
-            Callable<Integer> maxBatchSizeCallable,
+            IntProvider maxBatchSizeProvider,
             byte[] bytes) {
 
         return new RxBleRadioOperationCharacteristicLongWrite(bluetoothGatt,
                 rxBleGattCallback,
                 mainThreadScheduler,
                 timeoutConfiguration,
                 bluetoothGattCharacteristic,
-                maxBatchSizeCallable,
+                maxBatchSizeProvider,
                 writeOperationAckStrategy,
                 bytes);
     }

diff --git a/.../polidea/rxandroidble/internal/operations/RxBleRadioOperationCharacteristicLongWrite.java b/.../polidea/rxandroidble/internal/operations/RxBleRadioOperationCharacteristicLongWrite.java
@@ -14,14 +14,13 @@
 import com.polidea.rxandroidble.exceptions.BleGattCannotStartException;
 import com.polidea.rxandroidble.exceptions.BleGattOperationType;
 import com.polidea.rxandroidble.internal.DeviceModule;
-import com.polidea.rxandroidble.internal.RxBleLog;
 import com.polidea.rxandroidble.internal.RxBleRadioOperation;
+import com.polidea.rxandroidble.internal.connection.IntProvider;
 import com.polidea.rxandroidble.internal.connection.RxBleGattCallback;
 import com.polidea.rxandroidble.internal.util.ByteAssociation;
 
 import java.nio.ByteBuffer;
 import java.util.UUID;
-import java.util.concurrent.Callable;
 
 import javax.inject.Named;
 
@@ -41,7 +40,7 @@ public class RxBleRadioOperationCharacteristicLongWrite extends RxBleRadioOperat
     private final Scheduler mainThreadScheduler;
     private final TimeoutConfiguration timeoutConfiguration;
     private final BluetoothGattCharacteristic bluetoothGattCharacteristic;
-    private final Callable<Integer> batchSizeProvider;
+    private final IntProvider batchSizeProvider;
     private final WriteOperationAckStrategy writeOperationAckStrategy;
     private final byte[] bytesToWrite;
     private byte[] tempBatchArray;
@@ -52,7 +51,7 @@ public class RxBleRadioOperationCharacteristicLongWrite extends RxBleRadioOperat
             @Named(ClientComponent.NamedSchedulers.MAIN_THREAD) Scheduler mainThreadScheduler,
             @Named(DeviceModule.OPERATION_TIMEOUT) TimeoutConfiguration timeoutConfiguration,
             BluetoothGattCharacteristic bluetoothGattCharacteristic,
-            Callable<Integer> batchSizeProvider,
+            IntProvider batchSizeProvider,
             WriteOperationAckStrategy writeOperationAckStrategy,
             byte[] bytesToWrite) {
         this.bluetoothGatt = bluetoothGatt;
@@ -67,7 +66,7 @@ public class RxBleRadioOperationCharacteristicLongWrite extends RxBleRadioOperat
 
     @Override
     protected void protectedRun() throws Throwable {
-        int batchSize = getBatchSize();
+        int batchSize = batchSizeProvider.getValue();
 
         if (batchSize <= 0) {
             throw new IllegalArgumentException("batchSizeProvider value must be greater than zero (now: " + batchSize + ")");
@@ -111,15 +110,6 @@ protected BleException provideException(DeadObjectException deadObjectException)
         return new BleDisconnectedException(deadObjectException, bluetoothGatt.getDevice().getAddress());
     }
 
-    private int getBatchSize() {
-        try {
-            return batchSizeProvider.call();
-        } catch (Exception e) {
-            RxBleLog.w(e, "Failed to get batch size.");
-            throw new RuntimeException("Failed to get batch size from the batchSizeProvider.", e);
-        }
-    }
-
     @NonNull
     private Observable<ByteAssociation<UUID>> writeBatchAndObserve(final int batchSize, final ByteBuffer byteBuffer) {
         final Observable<ByteAssociation<UUID>> onCharacteristicWrite = rxBleGattCallback.getOnCharacteristicWrite();

diff --git a/...roovy/com/polidea/rxandroidble/internal/connection/MaxWritePayloadSizeProviderTest.groovy b/...roovy/com/polidea/rxandroidble/internal/connection/MaxWritePayloadSizeProviderTest.groovy
@@ -0,0 +1,32 @@
+package com.polidea.rxandroidble.internal.connection
+
+import spock.lang.Specification
+import spock.lang.Unroll
+
+
+class MaxWritePayloadSizeProviderTest extends Specification {
+
+    def mockMtuProvider = Mock IntProvider
+
+    MaxWritePayloadSizeProvider objectUnderTest
+
+    private void prepareObjectUnderTest(int gattWriteMtuOverhead) {
+        objectUnderTest = new MaxWritePayloadSizeProvider(mockMtuProvider, gattWriteMtuOverhead)
+    }
+
+    @Unroll
+    def "should return current MTU decreased by the write MTU overhead"() {
+
+        given:
+        prepareObjectUnderTest(gattWriteMtuOverhead)
+        mockMtuProvider.getValue() >> currentMtu
+
+        expect:
+        objectUnderTest.getValue() == expectedValue
+
+        where:
+        currentMtu | gattWriteMtuOverhead | expectedValue
+        10         | 2                    | 8
+        2000       | 32                   | 1968
+    }
+}