Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugin crash after reconnect() on Android #705

Open
phatpaul opened this issue Jul 26, 2021 · 9 comments · May be fixed by #707
Open

plugin crash after reconnect() on Android #705

phatpaul opened this issue Jul 26, 2021 · 9 comments · May be fixed by #707

Comments

@phatpaul
Copy link
Contributor

When app goes into the background, "pause" event fires and I call bluetoothle.disconnect().

When app is back in foreground, I call reconnect() and it seems to reconnect OK.

But after that when getting the services the plugin crashes:

2021-07-26 15:59:35.178 11816-11855/com.app D/BluetoothGatt: onClientConnectionState() - status=0 clientIf=7 device=84:CC:A8:0D:5C:F6
2021-07-26 15:59:35.189 11816-11816/com.app I/chromium: [INFO:CONSOLE(215)] "ble connected", source: file:///android_asset/www/shared/app.ble.js (215)
2021-07-26 15:59:35.191 11816-11991/com.app D/BluetoothGatt: configureMTU() - device: 84:CC:A8:0D:5C:F6 mtu: 517
2021-07-26 15:59:35.528 11816-11855/com.app D/BluetoothGatt: onConfigureMTU() - Device=84:CC:A8:0D:5C:F6 mtu=517 status=0
2021-07-26 15:59:35.552 11816-11816/com.app I/chromium: [INFO:CONSOLE(293)] "ble MTU has been set to 517", source: file:///android_asset/www/shared/app.ble.js (293)
2021-07-26 15:59:35.552 11816-11816/com.app I/chromium: [INFO:CONSOLE(224)] "ble Getting device services.", source: file:///android_asset/www/shared/app.ble.js (224)
2021-07-26 15:59:35.553 11816-11991/com.app E/PluginManager: Uncaught exception from plugin
    java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.util.LinkedList.add(java.lang.Object)' on a null object reference
        at com.randdusing.bluetoothle.BluetoothLePlugin.queueAdd(BluetoothLePlugin.java:3187)
        at com.randdusing.bluetoothle.BluetoothLePlugin.execute(BluetoothLePlugin.java:354)
        at org.apache.cordova.CordovaPlugin.execute(CordovaPlugin.java:98)
        at org.apache.cordova.PluginManager.exec(PluginManager.java:139)
        at org.apache.cordova.CordovaBridge.jsExec(CordovaBridge.java:59)
        at org.apache.cordova.engine.SystemExposedJsApi.exec(SystemExposedJsApi.java:41)
        at android.os.MessageQueue.nativePollOnce(Native Method)
        at android.os.MessageQueue.next(MessageQueue.java:342)
        at android.os.Looper.loop(Looper.java:197)
        at android.os.HandlerThread.run(HandlerThread.java:67)
2021-07-26 15:59:35.553 11816-11816/com.app I/chromium: [INFO:CONSOLE(912)] "ble reading DevInfo", source: file:///android_asset/www/shared/app.ble.js (912)

The crash happened in queueAdd() function (Android java code) at queue.add(operation);

  //Helpers for Callbacks
  private void queueAdd(Operation operation) {
    JSONArray args = operation.args;
    CallbackContext callbackContext = operation.callbackContext;

    JSONObject obj = getArgsObject(args);
    if (isNotArgsObject(obj, callbackContext)) {
      return;
    }

    String address = getAddress(obj);
    if (isNotAddress(address, callbackContext)) {
      return;
    }

    HashMap<Object, Object> connection = connections.get(address);

    if (connection != null) {
      LinkedList<Operation> queue = (LinkedList<Operation>) connection.get(keyQueue);
      queue.add(operation);
      queueStart(connection);
    }
  }
@phatpaul
Copy link
Contributor Author

Note that I can workaround the issue by calling close() after the pause event. But then the connect on resume takes a bit longer.

@randdusing
Copy link
Owner

Do you know what is calling queueAdd? Are you calling read or subscribe? I typically didn't use reconnect, so there may be some strange behavior.

@phatpaul
Copy link
Contributor Author

Sorry I know I should debug it further, but I'm not familiar with the Android source and debug tools. The workaround I mentioned above is working for me at the moment.

@VinardoZzZ2000 would you please assist with this issue since you added this code recently?
https://github.com/randdusing/cordova-plugin-bluetoothle/blame/000f6fb989d33891334e6150e046d32869ba1774/src/android/BluetoothLePlugin.java#L3213

UPDATE: I noticed that it didn't fail in my initial minimal test case, so I progressively added more of my actual code until I see the crash.

  • Just reconnect() => discover() : no crash.
  • Added request mtu: reconnect() => mtu() => discover(), : no crash.
  • Added subscribe: reconnect() => mtu() => discover() => subscribe : CRASH!

Also, I tried on an earlier version of this plugin (6.2.1) and it works ok. My guess is this bug is introduced with the new queuing function added recently.

Here's some minimal code to recreate the issue. To test, put the app in the background for >10s, then bring it back.

// @ts-check
"use strict"

const BLE_MTU_REQ = 517; // request server to increase to as high as this.
let ble_mtu = 23;  // default is 23 if not requested higher.

/**@type {BluetoothlePlugin.DeviceInfo} */
let deviceHandle = {
    address: "3C:71:BF:5F:66:4E",
    name: "",
    status: "discovered"
};

/**@type {?*} */
let sleepTimer = null;

/**
 * 
 * @param {?function()=} cb Callback function on done (pass or fail) (optional).
 */
 function requestMtu(cb) {
    if (cordova.platformId == "android") {
        window.bluetoothle.mtu(
            function (mtu) {
                ble_mtu = mtu.mtu;
                console.log('MTU has been set to ' + ble_mtu);
                cb && cb();
            },
            function (error) {
                console.error('MTU set fail' , error);
                cb && cb();
            },
            {
                address: deviceHandle.address,
                mtu: BLE_MTU_REQ,
            }
        );
    } else {
        // set MTU not needed on iOS, it is done automatically by the OS
        cb && cb();
    }
}

/**
 * subscribe to notifications
 * @param {string} service
 * @param {string} characteristic
 * @param {?function(string)=} cb Callback function on recieved notification accepts argument of (message)=>{}
 * @return the promise is only used for the setup of the notification
 */
 function subscribe (service, characteristic, cb) {
    return new Promise((resolve, reject) => {
            console.log('enabling notify');
            window.bluetoothle.subscribe((result) => {
                // This callback is called repeatedly until disableNotification is called.
                if (result.status == "subscribed") {
                    console.log('subscribed');
                    resolve(true);
                } else {
                    cb && cb(result.value);
                }
            }, (error) => {
                if (error.error = "isDisconnected") {
                    // this callback fires when device is disconnected (after setup)
                    //my_disconnectCallback();
                } else {
                    // assume an error in setup subscribe
                    console.error(error);
                    resolve(false); // but continue anyway
                }
            },
                {
                    address: deviceHandle.address,
                    service: service,
                    characteristic: characteristic,
                }
            );
        });

};

/**
 * 
 * @param {?BluetoothlePlugin.DeviceInfo=} result 
 */
function connectCallback(result) {
    if (!result) return;
    console.log(result.status);
    if (result.status === "connected") {
        requestMtu(function () {
            console.log("Getting device services.");
            new Promise(function (resolve, reject) {
                window.bluetoothle.discover(resolve, reject,
                    {
                        address: deviceHandle.address,
                        //clearCache: true,  // clearCache = true / false (default) Force the device to re-discover services, instead of relying on cache from previous discovery (Android only)
                    });
            }).then(() => {
                console.log("connected!");
                return subscribe('b0c0fff0-7c23-476c-9aa9-39dc3e58e0e9', 'b0c0fff4-7c23-476c-9aa9-39dc3e58e0e9', (val) => console.log(val));
            });
        });
    }
    else if (result.status === "disconnected") {
        console.log("Disconnected from: " + result.address);
    }
}

const ble_connect = () => new Promise((resolve, reject) => {
    console.log('try connect');
    window.bluetoothle.connect((result) => {
        connectCallback(result);
        if (result.status == "connected") {
            resolve(result);
        }
    }, (error) => {
        reject(error);
    }, { address: deviceHandle.address, autoConnect: true });
});

// Disconnect b/c pause application for quick reconnect
function ble_pause() {
    console.log('try disconnect');
    return new Promise(function (resolve, reject) {

        if (!(deviceHandle)) reject();
        else {
            window.bluetoothle.disconnect(resolve, resolve,
                { address: deviceHandle.address });
        }
    });

};
// Resume a paused connection
function ble_resume() {

    return new Promise((resolve, reject) => {
        console.log('try reconnect');
        if (!(deviceHandle)) reject();
        else window.bluetoothle.reconnect((result) => {
            connectCallback(result);
            if (result.status == "connected") {
                resolve(result);
            }
        }, (error) => {
            reject(error);
        }, { address: deviceHandle.address });
    });
};


/**
 * The pause event fires when the native platform puts the application into the background, typically when the user switches to a different application.
 * @param {Event} e 
 */
function onPause(e) {
    console.log("onPause() starting 10s timer");
    sleepTimer = setTimeout(() => {  // if app stays in background for 10s, then sleep
        ble_pause();
        sleepTimer = null;
    }, 10000);
}

/**
 * The resume event fires when the native platform pulls the application out from the background.
 * @param {Event} e 
 */
function onResume(e) {
    console.log("onResume()");
    if (sleepTimer != null) {
        clearInterval(sleepTimer);
        sleepTimer = null;
        // sleepTimer was still in progress, so not asleep, no need to resume
    } else {
        // sleepTimer must have expired, so asleep, resume
        ble_resume();
    }
}

document.addEventListener('deviceready', function () {

    // wrap in 0 timeout per https://cordova.apache.org/docs/en/latest/cordova/events/events.html#ios-quirks
    document.addEventListener("resume", () => setTimeout(onResume, 0), false);
    document.addEventListener("pause", () => setTimeout(onPause, 0), false);

    new Promise(function (resolve) {

        bluetoothle.initialize(resolve, { request: true, statusReceiver: false });

    }).then(ble_connect);

});

@ttongsul
Copy link
Contributor

ttongsul commented Aug 1, 2021

Sorry for the late reply. I was busy preparing my thesis.

Seems like I overlooked this, as I've never used disconnect() and reconnect().
It seems like the queue needs to be reinitialized on reconnect().

I'll prepare a fix ASAP. Sorry for the inconvenience.

@ttongsul ttongsul linked a pull request Aug 1, 2021 that will close this issue
@ttongsul
Copy link
Contributor

ttongsul commented Aug 1, 2021

@phatpaul

I've made a PR: #707

But unfortunately, I cannot fully test your code. connect() with autoConnect: true failed on my end. reconnect() also fails with autoConnect: false. Bonding first also didn't work. My guess is because I'm using a simulated peripheral (Android peripheral mode), which has a non-static address.

@max-scopp
Copy link

Waiting for merge.

I'm testing using a real device, however, I haven't implemented autoConnect: true for my app, yet.

Apart from that, the PR works well for me. Will respond if autoConnect: true works just as well or not.

@phatpaul
Copy link
Contributor Author

phatpaul commented Aug 5, 2021

@max-scopp what did the PR #707 fix for you? It didn't help my case.
Seems to me there's more logic fixes needed in the native code to keep the callbacks for future (re)connect events.

@phatpaul
Copy link
Contributor Author

phatpaul commented Aug 5, 2021

It seems the autoconnect / reconnect feature was first discussed here: #333

And seems it was left in the state where it could only be used like this:
Connect(autoconnect=false) =triggers=> connectcallback("connected")
... then say device disappears ...
=triggers=> connectcallback("disconnected") => close() => connect(autoconnect=true)

That last call to connect with autoreconnect if disconnected is just to tell the plugin to keep trying to reconnect. It also re-registers the callback (since the callbacks were erased when the disconnect happened).

I think the logic could be more useful. If my BLE peripheral device disappears (out of range, or reboots), this plugin should just try to reconnect to it. It does reconnect, but my JS code never gets a callback that it reconnected.

I suspect it is due to the plugin clearing the callbacks when a disconnect happens.

connection = new HashMap<Object, Object>();

which clears the connection object when a disconnect happens.
Why does it get cleared it here? That means that a (automatic) reconnect will not trigger a callback to the JS code.
Instead, the callbacks should only be cleared after the close(). @randdusing Do you agree?

Should I bother trying to fix it? Or just use the workaround?
If workaround, I think it should be better documented.

@phatpaul
Copy link
Contributor Author

phatpaul commented Aug 5, 2021

Another observation: When I initially connect with (autoconnect:true), the Android (and iOS?) takes anywhere from 1~20 seconds to even begin the connection attempt. I verified that the delay is from the central device with a BLE sniffer.

Setting (autoconnect:false) on initial connect fixes this issue.

I was going to open a separate issue for this, but it seems it's not a problem if I use the pattern I outlined above, since the initial connection will always be with autoconnect:false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants