Operation viability check #240

mikolak · 2017-07-03T14:34:04Z

This PR is is connected to #224

I have implemented a mechanism for checking whether the requested characteristic operation can succeed. The check is based upon the properties of the chosen characteristic. End user can opt-out of the 'hard' way of checking (ie. throwing an exception) using new connection setup API.

…le according to characteristic's properties

# Conflicts: # rxandroidble/src/main/java/com/polidea/rxandroidble/internal/RxBleDeviceImpl.java # rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionModule.java # rxandroidble/src/test/groovy/com/polidea/rxandroidble/internal/RxBleDeviceTest.groovy

dariuszseweryn

Added a few thoughts.

dariuszseweryn · 2017-07-04T11:37:15Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionModule.java

    }

    @Provides
    @ConnectionScope
-    static BluetoothGatt provideBluetoothGatt(BluetoothGattProvider bluetoothGattProvider) {
-        return bluetoothGattProvider.getBluetoothGatt();
+    IllegalOperationChecker provideIllegalOperationChecker() {


This one could be a static function if suppressPropertiesCheck would be passed to it. Play around and find a way that will introduce the lease amount of methods / functions to .dex

You can see how many fields and methods there are by adding the following snippet below apply from: in rxandroidble/build.gradle:

buildscript { repositories { mavenCentral() // or jcenter() } dependencies { classpath 'com.getkeepsafe.dexcount:dexcount-gradle-plugin:0.7.0' } } apply plugin: 'com.getkeepsafe.dexcount' dexcount { format = "list" includeClasses = true includeFieldCount = false includeTotalMethodCount = false orderByMethodCount = true verbose = false maxTreeDepth = Integer.MAX_VALUE teamCityIntegration = false }

dariuszseweryn · 2017-07-04T12:30:03Z

...droidble/src/main/java/com/polidea/rxandroidble/internal/connection/RxBleConnectionImpl.java

@@ -195,6 +204,10 @@ public int getMtu() {
    @Override
    public Observable<Observable<byte[]>> setupIndication(@NonNull BluetoothGattCharacteristic characteristic,
                                                          @NonNull NotificationSetupMode setupMode) {
+        illegalOperationChecker.checkAnyPropertyMatches(


I am actually thinking wether or not it would be better to have something like:

if (illegalOperationChecker.checkAnyPropertyMatches()) { return ... // normal stuff } else { return Observable.error(new IllegalOperationException(/* construct */)); }

It would be then consistent no matter how the user would call the function RxBleConnection.setupIndication(UUID) / RxBleConnection.setupIndication(BluetoothGattCharacteristic). Because now calling RxBleConnection.setupIndication(BluetoothGattCharacteristic) will just throw.

It would need a different abstraction though. @uKL any thoughts?

I have modified IllegalOperationChecker.checkAnyPropertyMatches() to return Completable.fromCallable() and will upload it shortly.

I agree that there should be a model separation. The method should check whether preconditions are correct and the result should be described by the caller.

Moreover, the point where the check happens is inconsistent. Sometimes it happens even before the operation is enqueued and sometimes not (when it's required to perform discovery in order to get the characteristic object)

dariuszseweryn · 2017-07-04T12:38:50Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/RxBleDeviceImpl.java

@@ -53,12 +54,20 @@

    @Override
    public Observable<RxBleConnection> establishConnection(final boolean autoConnect) {
+        ConnectionSetup options = (new ConnectionSetup.Builder())
+                .setAutoConnect(autoConnect)
+                .build();


This should also have .setSuppressOperationCheck(false) to match the previous behaviour.

dariuszseweryn · 2017-07-04T12:39:21Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

+            return this;
+        }
+
+        public Builder setSupressOperationCheck(boolean suppressOperationCheck) {


setSuppressIllegalOperationCheck()? Now it is not too informative in my opinion.

I think it would be nice to inform on the nature of the checks. That they are related to permissions.

dariuszseweryn · 2017-07-04T12:41:11Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectorImpl.java

-                final ConnectionComponent connectionComponent = connectionComponentBuilder.build();
+                final ConnectionComponent connectionComponent = connectionComponentBuilder
+                        .connectionModule(new ConnectionModule(options.suppressOperationCheck))
+                        .build();
                RxBleRadioOperationConnect operationConnect = connectionComponent.connectOperationBuilder()


It should be now possible to get rid of the RxBleRadioOperationConnect.Builder as all dependencies could be passed through ConnectionComponent.Builder.

dariuszseweryn · 2017-07-04T12:43:24Z

rxandroidble/src/main/java/com/polidea/rxandroidble/RxBleDevice.java

+     * <p>
+     * During the disconnect process the library automatically handles order and requirement of device disconnect and gatt close operations.
+     * <p>
+     * Autoconnect concept may be misleading at first glance. In cases when the BLE device is available and it is advertising constantly you


This part about autoConnect should go to ConnectionSetup javadoc as it will be the place to set.

…e has ended (was unsubscribed). Enchanced tests.

uKL · 2017-07-05T11:43:32Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

+            return this;
+        }
+
+        public Builder setSupressOperationCheck(boolean suppressOperationCheck) {


I think it would be nice to inform on the nature of the checks. That they are related to permissions.

uKL · 2017-07-05T11:45:12Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

+
+        private boolean autoConnect = false;
+        private boolean supressOperationCheck = false;
+


We need a documentation for public methods. (like in RxBleConnection)

uKL · 2017-07-05T11:46:08Z

...droidble/src/main/java/com/polidea/rxandroidble/exceptions/BleIllegalOperationException.java

+package com.polidea.rxandroidble.exceptions;
+
+/**
+ *


Missing JavaDoc?

uKL · 2017-07-05T11:46:36Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/RxBleDeviceImpl.java

@@ -53,12 +54,20 @@

    @Override
    public Observable<RxBleConnection> establishConnection(final boolean autoConnect) {
+        ConnectionSetup options = (new ConnectionSetup.Builder())
+                .setAutoConnect(autoConnect)
+                .build();


uKL · 2017-07-05T11:47:06Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/RxBleDeviceImpl.java

@@ -53,12 +55,25 @@

    @Override
    public Observable<RxBleConnection> establishConnection(final boolean autoConnect) {
+        ConnectionSetup options = (new ConnectionSetup.Builder())


Redundant backet

uKL · 2017-07-05T12:03:08Z

...droidble/src/main/java/com/polidea/rxandroidble/internal/connection/RxBleConnectionImpl.java

@@ -195,6 +204,10 @@ public int getMtu() {
    @Override
    public Observable<Observable<byte[]>> setupIndication(@NonNull BluetoothGattCharacteristic characteristic,
                                                          @NonNull NotificationSetupMode setupMode) {
+        illegalOperationChecker.checkAnyPropertyMatches(


I agree that there should be a model separation. The method should check whether preconditions are correct and the result should be described by the caller.

uKL · 2017-07-05T12:08:06Z

...droidble/src/main/java/com/polidea/rxandroidble/internal/connection/RxBleConnectionImpl.java

@@ -195,6 +204,10 @@ public int getMtu() {
    @Override
    public Observable<Observable<byte[]>> setupIndication(@NonNull BluetoothGattCharacteristic characteristic,
                                                          @NonNull NotificationSetupMode setupMode) {
+        illegalOperationChecker.checkAnyPropertyMatches(


Moreover, the point where the check happens is inconsistent. Sometimes it happens even before the operation is enqueued and sometimes not (when it's required to perform discovery in order to get the characteristic object)

uKL · 2017-07-05T12:11:44Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/util/IllegalOperationChecker.java

+        final int characteristicProperties = characteristic.getProperties();
+
+        if ((characteristicProperties & neededProperties) == 0) {
+            int[] possibleProperties = getPossibleProperties();


I guess it's more like permited or allowed than possible, right?

or even supported if I'm reading correctly from the context.

uKL · 2017-07-05T12:16:32Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/util/IllegalOperationChecker.java

+    protected abstract void handleMessage(String message);
+
+    @NonNull
+    private String propertiesIntToString(int property, int[] possibleProperties) {


More like an util. What do you think about extracting it?

I'm not sure. If it's supposed to be testable, then it would need to be instantiated. That would lead to increasing method count and I'm not that convinced that it would be useful anywhere else.

uKL · 2017-07-05T12:20:40Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/util/IllegalOperationChecker.java

+     * Method for handling the potential non-match message
+     * @param message message describing the
+     */
+    protected abstract void handleMessage(String message);


I'd prefer the checker to return the model with the report, rather than to pass it to a method that needs to be overridden. @dariuszseweryn what do you think? It's easier to test then.

Currently we have a IllegalOperationChecker which is checking and handling the result. We could have IllegalOperationChecker that would work with IllegalOperationHandler which would either log or throw/emit an IllegalOperationException. The checker part would be always the same (could be put directly in RxBleConnectionImpl or injected as a separate object)

… into operation_viability

# Conflicts: # rxandroidble/src/main/java/com/polidea/rxandroidble/internal/RxBleDeviceImpl.java

…ng for illegal operation check.

dariuszseweryn

And one comment from the previous review was not addressed.

dariuszseweryn · 2017-07-07T13:43:13Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

@@ -34,6 +34,10 @@ private ConnectionSetup(boolean autoConnect, boolean suppressOperationCheck) {


        /**
+         * Autoconnect concept may be misleading at first glance. In cases when the BLE device is available and it is advertising constantly
+         * you won't need to use autoconnect. Use autoconnect for connections where the BLE device is not advertising at
+         * the moment of #establishConnection call.


{@link com.polidea.rxandroidble.RxBleConnection#establishConnection(ConnectionSetup)} instead of #establishConnection

dariuszseweryn · 2017-07-07T13:45:49Z

...droidble/src/main/java/com/polidea/rxandroidble/exceptions/BleIllegalOperationException.java

+    public final int supportedProperties;
+    public final int neededProperties;
+
+    public BleIllegalOperationException(String message,


Add @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) and a javadoc that will tell people not to instantiate this class. Maybe it is worth to expose getters instead of public finals — just a thought.

Well, getters will increase the method count and all of these fields are immutable anyhow. Can be done, though I'm not sure if it should be.

True though here we have fields in the API and we cannot change that later on if we would decide to pass BluetoothGattCharacteristic in the constructor. But it is just an opinion. @uKL thoughts?

Well, the first thought is "Why do I need an int?! It's ugly.". If we'd have an API to read properties more easily.

Maybe we could annotate the ints with @BluetoothGattCharacteristicProperty to make it more precise what is this int related to. BluetoothGattCharacteristicProperty would then be put on the public API.

dariuszseweryn · 2017-07-07T13:47:33Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionModule.java

@@ -34,21 +34,23 @@ public ConnectionModule(ConnectionSetup connectionSetup) {
    @ConnectionScope
    IllegalOperationChecker provideIllegalOperationChecker() {
        if (suppressOperationCheck) {
-            return new LoggingIllegalOperationChecker(BluetoothGattCharacteristic.PROPERTY_BROADCAST,
+            return new IllegalOperationChecker(BluetoothGattCharacteristic.PROPERTY_BROADCAST,


This may be now optimised. if else only picks a MismatchDataHandler

dariuszseweryn · 2017-07-07T13:49:29Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/util/IllegalOperationChecker.java

@@ -79,18 +83,30 @@ public Object call() throws Exception {
                            propertiesIntToString(neededProperties, possibleProperties),
                            neededProperties
                    );
-                    handleMessage(message);
+                    MismatchData mismatchData = new MismatchData(message,


I think that having a MismatchData here is superfluous and it only adds an allocation. The data could be passed directly to the MismatchDataHandler

dariuszseweryn · 2017-07-07T13:53:00Z

rxandroidble/src/main/java/com/polidea/rxandroidble/internal/util/MismatchDataHandler.java

+ * Handler for {@link com.polidea.rxandroidble.internal.util.IllegalOperationChecker.MismatchData} returned by
+ * {@link IllegalOperationChecker}.
+ */
+public interface MismatchDataHandler {


Why not just a IllegalOperationHandler? And it can be put just in internal.connection package. Does not look like an util that is used all over the code.

dariuszseweryn · 2017-07-10T12:20:38Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

+        /**
+         * @param suppressOperationCheck Flag describing the method of operation viability checking. If set to false,
+         *                               a {@link com.polidea.rxandroidble.exceptions.BleIllegalOperationException} will be
+         *                               thrown everytime properties of the characteristic don't match those required by the operation.


thrown -> emitted

dariuszseweryn · 2017-07-10T12:22:03Z

...droidble/src/main/java/com/polidea/rxandroidble/exceptions/BleIllegalOperationException.java

+ * This exception is thrown when a non-supported operation has been requested upon a characteristic, eg. write operation on a
+ * characteristic with only read property.
+ */
+@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)


the annotation should be on the constructor only. the class is meant to be used in the API

dariuszseweryn · 2017-07-10T12:27:53Z

...droidble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionComponent.java

    @Subcomponent.Builder
    interface Builder {

+        Builder connectionModule(ConnectionModule connectionModule);
+
        ConnectionComponent build();
    }

    @ConnectionScope


Maybe it is possible to get rid of the @ConnectionScope here as the operation is used only once and it is not needed to be cached. It is worth checking.

Maybe it is possible to get rid of caching for other items as well. please check method counts / fields.

I think further optimization is out of scope for this pull request. We should have a separate PR for optimizations if you think they are necessary.

dariuszseweryn · 2017-07-10T12:33:10Z

...idble/src/main/java/com/polidea/rxandroidble/internal/connection/ConnectionModuleBinder.java

+@Module
+abstract public class ConnectionModuleBinder {
+
+    static final String GATT_WRITE_MTU_OVERHEAD = "GATT_WRITE_MTU_OVERHEAD";


Maybe this should also be with other names in the ConnectionComponent?

dariuszseweryn · 2017-07-10T12:36:00Z

...droidble/src/main/java/com/polidea/rxandroidble/exceptions/BleIllegalOperationException.java

+    public final int supportedProperties;
+    public final int neededProperties;
+
+    public BleIllegalOperationException(String message,


True though here we have fields in the API and we cannot change that later on if we would decide to pass BluetoothGattCharacteristic in the constructor. But it is just an opinion. @uKL thoughts?

dariuszseweryn · 2017-07-10T12:37:07Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

+         * you won't need to use autoconnect. Use autoconnect for connections where the BLE device is not advertising at
+         * the moment of {@link RxBleDevice#establishConnection(ConnectionSetup)} call.
+         *
+         * @param autoConnect Marker related with


related to or connected with ;) Flag maybe?

My fault for copy-paste without proof reading, not my grammar mistake. ;) Should I correct it elsewhere as well?

dariuszseweryn · 2017-07-10T12:37:43Z

rxandroidble/src/main/java/com/polidea/rxandroidble/ConnectionSetup.java

+         *                    {@link android.bluetooth.BluetoothDevice#connectGatt(Context, boolean, BluetoothGattCallback)} autoConnect
+         *                    flag. In case of auto connect is enabled the observable will wait with the emission of RxBleConnection.
+         *                    Without auto connect flag set to true the connection will fail
+         *                    with {@link com.polidea.rxandroidble.exceptions.BleGattException} if the device is not in range.


after around 30 seconds timeout

Should I correct it elsewhere as well?

uKL · 2017-07-11T07:05:37Z

...est/groovy/com/polidea/rxandroidble/internal/util/ThrowingIllegalOperationHandlerTest.groovy

+
+    def "should throw BleIllegalOperationException if no property matches"() {
+        when:
+        objectUnderTest.handleMismatchData(message, mockUuid, supportedProperties, neededProperties)


the name suggests a mock for mockUuid but it's null.

uKL · 2017-07-11T07:06:13Z

...le/src/test/groovy/com/polidea/rxandroidble/internal/util/IllegalOperationCheckerTest.groovy

+
+    void setup() {
+        mockHandler = Mock IllegalOperationHandler
+        subscriber = new  TestSubscriber()


too many spaces

uKL · 2017-07-11T07:06:52Z

...le/src/test/groovy/com/polidea/rxandroidble/internal/util/IllegalOperationCheckerTest.groovy

+
+    def "should handle message if no property matches"() {
+        given:
+        bluetoothGattCharacteristic.getProperties() >> 0


What stands for 0?

uKL

Some more comments to talk about.

uKL · 2017-07-11T07:07:18Z

...le/src/test/groovy/com/polidea/rxandroidble/internal/util/IllegalOperationCheckerTest.groovy

+        bluetoothGattCharacteristic.getProperties() >> 0
+
+        when:
+        objectUnderTest.checkAnyPropertyMatches(bluetoothGattCharacteristic, BluetoothGattCharacteristic.PROPERTY_READ)


Is subscriber used anyhow?

uKL · 2017-07-11T07:07:24Z

...le/src/test/groovy/com/polidea/rxandroidble/internal/util/IllegalOperationCheckerTest.groovy

+        bluetoothGattCharacteristic.getProperties() >> BluetoothGattCharacteristic.PROPERTY_READ
+
+        when:
+        objectUnderTest.checkAnyPropertyMatches(bluetoothGattCharacteristic, BluetoothGattCharacteristic.PROPERTY_READ)


Is subscriber used anyhow?

uKL · 2017-07-11T07:07:28Z

...le/src/test/groovy/com/polidea/rxandroidble/internal/util/IllegalOperationCheckerTest.groovy

+        bluetoothGattCharacteristic.getProperties() >> BluetoothGattCharacteristic.PROPERTY_WRITE
+
+        when:
+        objectUnderTest.checkAnyPropertyMatches(bluetoothGattCharacteristic,


Is subscriber used anyhow?

uKL · 2017-07-11T07:09:20Z

rxandroidble/src/test/groovy/com/polidea/rxandroidble/internal/RxBleDeviceTest.groovy

+        Observable<RxBleConnection> establishConnection(RxBleDevice d) {
+            return establishConnectionCaller.connectionStartClosure.call(d, connectionSetup)
+        }
+


Remove empty line

uKL · 2017-07-11T07:18:26Z

...dble/src/main/java/com/polidea/rxandroidble/internal/connection/IllegalOperationChecker.java

+ */
+public class IllegalOperationChecker {
+
+    private int propertyBroadcast;


Please make attributes final, just in case.

uKL · 2017-07-11T07:19:37Z

...dble/src/main/java/com/polidea/rxandroidble/internal/connection/IllegalOperationChecker.java

+    }
+
+    @NonNull
+    private String propertiesIntToString(int property, int[] possibleProperties) {


I still think generating a message is not in scope of the checker, it should be a separate util class.

True, I'll extract it.

uKL · 2017-07-11T07:19:47Z

...dble/src/main/java/com/polidea/rxandroidble/internal/connection/IllegalOperationChecker.java

+    @NonNull
+    private String propertiesIntToString(int property, int[] possibleProperties) {
+        StringBuilder builder = new StringBuilder();
+        builder.append(" ");


Why is there a space at the beginning?

For prettyness. [ ] looks better than [] if there are no properties and [ READ WRITE ] looks better than [READ WRITE ]. I decided to avoid tracking whether any property has already been matched or not. I admit that I wrote specifically for the message which puts the string in square brackets, so for the purpose of extracting it to a separate parser, maybe the brackets should be added to this function?

uKL · 2017-07-11T07:20:51Z

...dble/src/main/java/com/polidea/rxandroidble/internal/connection/IllegalOperationChecker.java

+            return "NOTIFY";
+        } else {
+            RxBleLog.e("Unknown property specified");
+            throw new IllegalArgumentException("Unknown property specified");


Is it documented that the method may throw IllegalArgumentException? It will be passed to the public API.

Just removed the exception in a refactor. ;)

…ationChecker. Additional minor changes after review.

dariuszseweryn

Added comment to the exception.

dariuszseweryn · 2017-07-17T10:09:33Z

...droidble/src/main/java/com/polidea/rxandroidble/exceptions/BleIllegalOperationException.java

+    public final int supportedProperties;
+    public final int neededProperties;
+
+    public BleIllegalOperationException(String message,


Maybe we could annotate the ints with @BluetoothGattCharacteristicProperty to make it more precise what is this int related to. BluetoothGattCharacteristicProperty would then be put on the public API.

dariuszseweryn · 2017-07-19T09:13:25Z

rxandroidble/src/main/java/com/polidea/rxandroidble/BluetoothGattCharacteristicProperty.java

@@ -0,0 +1,3 @@
+package com.polidea.rxandroidble;
+
+public @interface BluetoothGattCharacteristicProperty { }


It still needs proper annotations?

...aaaaaand javadoc ;)

dariuszseweryn · 2017-07-19T09:14:26Z

...dble/src/main/java/com/polidea/rxandroidble/internal/connection/IllegalOperationHandler.java

@@ -14,5 +16,5 @@
        this.messageCreator = messageCreator;
    }

-    public abstract void handleMismatchData(BluetoothGattCharacteristic characteristic, int neededProperties);
+    public abstract BleIllegalOperationException handleMismatchData(BluetoothGattCharacteristic characteristic, int neededProperties);


mark it with @Nullable

uKL

Two more comments regarding the documentation. The rest is acceptable.

uKL · 2017-07-28T12:02:16Z

rxandroidble/src/main/java/com/polidea/rxandroidble/RxBleDevice.java

@@ -31,11 +31,12 @@

    /**
     * @param context     Android's context.
-     * @param autoConnect Marker related with
+     * @param autoConnect Flag related to


In the ConnectionSetup it still says "marker"

uKL · 2017-07-28T12:15:28Z

...dble/src/main/java/com/polidea/rxandroidble/internal/connection/IllegalOperationChecker.java

+     * @param characteristic   a {@link BluetoothGattCharacteristic} the operation is done on
+     * @param neededProperties properties required for the operation to be successfully completed
+     * @return {@link Completable} deferring execution of the check till subscription
+     * @throws BleIllegalOperationException if there was no match between supported and necessary properties of characteristic and check has


In fact it does not throw, it emits BleIllegalOperationException

dariuszseweryn

I am good with what was done. 👍

…lishConnection(SetupOptions)` (#240 #224) This new function is a perfect extension point to introduce a new public API #239 which will return the new API without changing the Major version number. Till then all new public classes were moved to the `internal` package and should not be publicly available.

…nal package. (#240 #224) Will become public when new API #239 will be ready.

(#240 #224)

uKL · 2018-03-20T13:39:17Z

Dear @mikolak ,

similar to many open source projects we kindly request to sign our CLA if you'd like to contribute to our project. This license is for your protection as a Contributor as well as the protection of Polidea; it does not change your rights to use your own Contributions for any other purpose.

You can find a link here: https://cla-assistant.io/Polidea/RxAndroidBle

Added a functionality to check whether requested operation is availab…

3038699

…le according to characteristic's properties

mikolak requested review from dariuszseweryn, uKL and mzgreen July 3, 2017 14:34

mikolak self-assigned this Jul 3, 2017

dariuszseweryn requested changes Jul 4, 2017

View reviewed changes

Fixed not being able to establish a new connection after the first on…

60ce167

…e has ended (was unsubscribed). Enchanced tests.

uKL requested changes Jul 5, 2017

View reviewed changes

dariuszseweryn added the enhancement label Jul 5, 2017

mikolak added 4 commits July 5, 2017 15:58

Adjusted implementation according to code review suggestions.

74a8b29

Merge branch 'operation_viability' of github.com:Polidea/RxAndroidBle…

6594944

… into operation_viability

Merge branch 'develop' into operation_viability

5752706

# Conflicts: # rxandroidble/src/main/java/com/polidea/rxandroidble/internal/RxBleDeviceImpl.java

Fixed tests, did adjustments after code review, changed result handli…

143633c

…ng for illegal operation check.

dariuszseweryn reviewed Jul 7, 2017

View reviewed changes

Fixes after review.

1769585

dariuszseweryn requested changes Jul 10, 2017

View reviewed changes

uKL reviewed Jul 11, 2017

View reviewed changes

uKL requested changes Jul 11, 2017

View reviewed changes

mikolak added 2 commits July 12, 2017 15:33

Extracted message creation and parsing of a property from IllegalOper…

e2594fd

…ationChecker. Additional minor changes after review.

Merge branch 'develop' into operation_viability

d088339

dariuszseweryn reviewed Jul 17, 2017

View reviewed changes

mikolak added 2 commits July 18, 2017 17:38

Moved throwing of the exception to IllegalOperationChecker.

b86107e

Merge branch 'develop' into operation_viability

7204475

dariuszseweryn reviewed Jul 19, 2017

View reviewed changes

mikolak added 2 commits July 19, 2017 14:58

Corrected annotations.

83e83cf

Fixed checkstyle, added javadoc.

da85b6c

uKL requested changes Jul 28, 2017

View reviewed changes

Documentation fixes

d2e6b2c

dariuszseweryn approved these changes Jul 31, 2017

View reviewed changes

uKL approved these changes Aug 1, 2017

View reviewed changes

Fixed checkstyle.

9a6d7e3

mikolak force-pushed the operation_viability branch from bea3b36 to 9a6d7e3 Compare August 1, 2017 08:53

dariuszseweryn merged commit 2685130 into develop Aug 3, 2017

dariuszseweryn deleted the operation_viability branch August 3, 2017 08:59

dariuszseweryn added a commit that referenced this pull request Aug 3, 2017

TEMPORARY (until 1.5.0) Moved BleIllegalOperationException to inter…

0343338

…nal package. (#240 #224) Will become public when new API #239 will be ready.

dariuszseweryn added a commit that referenced this pull request Aug 3, 2017

TEMPORARY (until 1.5.0) Removed bogus line.

96176a9

(#240 #224)


		private boolean autoConnect = false;
		private boolean supressOperationCheck = false;

		@@ -0,0 +1,3 @@
		package com.polidea.rxandroidble;

		public @interface BluetoothGattCharacteristicProperty { }

Operation viability check #240

Operation viability check #240

Conversation

mikolak commented Jul 3, 2017

dariuszseweryn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariuszseweryn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uKL left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariuszseweryn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uKL left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dariuszseweryn left a comment

Choose a reason for hiding this comment

uKL left a comment •

edited

Loading