Parameter specified as non-null is null

[ghost]

The request that fails is

fun disconnectWithoutReconnect() {
        disconnect()
                .done { Timber.d("disconnect success") }
                .fail { device, status ->
                    Timber.d("disconnect fail")

                    close()
                }
                .enqueue()
    }

java.lang.IllegalArgumentException: Parameter specified as non-null is null: method kotlin.jvm.internal.Intrinsics.checkParameterIsNotNull, parameter it
        at com.mycompany.MyNordicBleManager$disconnectWithoutReconnect$1.onRequestCompleted(Unknown Source:2)
        at no.nordicsemi.android.ble.Request.notifySuccess(Request.java:829)
        at no.nordicsemi.android.ble.BleManager.internalDisconnect(BleManager.java:665)
        at no.nordicsemi.android.ble.BleManager.access$2100(BleManager.java:113)
        at no.nordicsemi.android.ble.BleManager$BleManagerGattCallback.nextRequest(BleManager.java:2812)
        at no.nordicsemi.android.ble.BleManager$BleManagerGattCallback.access$1300(BleManager.java:1812)
        at no.nordicsemi.android.ble.BleManager.lambda$enqueue$4(BleManager.java:1786)
        at no.nordicsemi.android.ble.-$$Lambda$BleManager$KDSxdgn5nClZRFe_FvyLUfX8mWc.run(Unknown Source:2)
        at android.os.Handler.handleCallback(Handler.java:790)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:164)
        at android.app.ActivityThread.main(ActivityThread.java:6494)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:807)

Scenario

• beta2
• attempt to disconnect from a device I'm not connected to. In this case, I never even attempted to connect to it.

[philips77]

Thanks for testing the lib! I'll push sone updates asap.

[philips77]

What result would you expect in that case? In both success and fail callbacks, the BluetoothDevice is @NonNull. Should I add another callback for invalid state? Remove the annotation in fail or 'success' (set to @Nullable)? Right now it reports success, as you are indeed disconnected.

It's only this one case when the BluetoohtDevice may be null.

[philips77]

The BluetoothDevice could also be set in the BlueManager's constructor, instead of calling it in connect(..). But that would break a lot when upgrading to the next version. What do you think?

[ghost]

From disconnect javadoc

Does nothing if device was not connected.

In this case, I'd expect the call to do nothing, since we are not connected.

Does doing nothing also invoke fail? I'm not sure if that's what I'd expect, but I understand that you'd rather invoke one of the two, tho I find done is more appropriate in this case.

First option would be to remove the @nonnull annotation from BluetoothDevice and set it to @nullable. That'd fix kotlin expectations, tho it's a breaking change that should be added to release notes.

As a second, more complex approach, would you consider an approach similar to RxJava's Maybe? It either emits onSuccess, onError or onComplete

From its javadoc: onSuccess, onError and onComplete are mutually exclusive events

onComplete's name might be misleading and developers might think it's invoked after done, so maybe something like ignored or skipped

I'd maybe go for the 1st approach. Adding a ignored callback means increasing the complexity of the library. Is there any usecase for such a callback?

Regarding the BluetoothDevice as a parameter to BleManager, that'd also break the possibility connect to multiple devices with a single BleManager. I don't know how desirable that is or if people are using it that way.

If it's not desirable to reuse a BleManager for multiple devices, then I'd definitely enforce it as a constructor parameter to avoid unexpected issues. Otherwise, I'd leave it as it.

[philips77]

I've already started implementing invalid callback to each request. It would only be called when connect was never called and BluetoothDevice is null. At least so far. That would also include Sleep request (which doesn't need BluetoothDevice, actually, but for consistency. And I left @NonNull in both done and fail callbacks. That way, I think, is the most "backwards compatible", as you don't need to make any changes, you don't need to handle the new invalid callback and you are guaranteed that success and fail will indeed have non-null parameter.

RxJava approach also seems good, but that would break everything.

Btw, how does the lib work with Kotlin? I tried to make it Kotlin-firendly, but actually never tried it with it.

[ghost]

Thanks for the quick response!

There are no differences when using it from kotlin, It's straightforward.

The thing is, if a parameter is annotated as @nonnull BluetootDevice, kotlin's type is BluetoothDevice, whereas if it's @nullable the type is BluetoothDevice?, which is different from the non-question mark type. If a non-null parameter receives null, it's a runtime error because it's not of the type it expects.

[philips77]

But there are those thing that make a library more Kotlin'y, like @Nullable or @NonNull that, that you already mentioned, that are converted to optional or not optional fields, but also callbacks as a last parameter can be open, etc. I just wonder if anything could be improved to make it better before going to final version.

[ghost]

sorry I misunderstood your question

I haven't found any issues, but I'm not that experienced with Kotlin

I know of two interop guides, but I guess you already checked that

https://kotlinlang.org/docs/reference/java-to-kotlin-interop.html
https://android.github.io/kotlin-guides/interop.html

[philips77]

Ok, this issue should be fixed with the new PR. Would you like to have a look at it before I merge?

[ghost]

I'm taking a look but I don't expect to be of help, it's a complex code

[philips77]

Released in beta3.

[ghost]

It's fixed, congrats and thanks!!