aws / amazon-freertos-ble-android-sdk

Android SDK for FreeRTOS Bluetooth Devices.
Apache License 2.0
49 stars 41 forks source link

CBOR Decodes List Network Value Incorrectly #2

Closed KylePalko closed 5 years ago

KylePalko commented 5 years ago

The Android demo compiles, finds and connects to the device successfully, but crashes when parsing the CBOR notification payload from the list networks characteristic. No error is thrown on decoding, but the object is returning { null: null } in the decoded CBOR payload. This causes the demo app to crash. I am running on the beta branch.

bxpan commented 5 years ago

Hi KylePaiko,

Thanks for using the AmazonFreeRTOS BLE Android SDK. Could you please let us know which commit on beta branch are you using? Could you share the adb log from the phone that includes crash stack trace? How often does this issue occur?

KylePalko commented 5 years ago

Hey @bxpan, thanks for responding. I won't be able to provide the stack trace until later this week, but you should be able to test with the latest commit of the beta branch. It decodes the CBOR without failing, but as soon as the application attempts to get a property from the decoded CBOR payload it returns null, which causes an error to be thrown as it's being casted to a type.

The iOS FREERTOS BLE demo version of the app decodes the CBOR payload from the device successfully with all the properties.

bxpan commented 5 years ago

Hi @KylePalko, what do you mean by "get a property" from decoded CBOR payload? Which "type" is it casting to? Did the crash happen in the decode method in "ListNetworkResp" class?

KylePalko commented 5 years ago

Yes it's the ListNetworkResp class.

This line in particular:

dataItem = map.get(new UnicodeString(BSSID_KEY));

But it's really any of the .get calls because the object in map only contains SSID and Status Key.

So, for instance, if you comment out that line in particular, the next one will throw.

bxpan commented 5 years ago

How often does this occur? Do you have the reproduce steps? We would like to understand why the map only contains SSID and status , but missing other fields.

KylePalko commented 5 years ago

I was able to reproduce this every time on Android. To reproduce it: pull down the beta branch code, run it on an Android phone, connect it to a BLE Module running Amazon's FREERTOS BLE firmware. Once it begins scanning for networks it will throw on the line I detailed above.

rrivadeneira-rsc commented 5 years ago

Hi @bxpan, I'd also like to state that we have narrowed down that the issue is not likely with the module, as the iOS version of this SDK decodes and displays the scan results successfully and is able to go through provisioning.

bxpan commented 5 years ago

From what KylePalko mentioned, the object in the map only contains SSID and Status Key, could you confirm that on iOS, it is also the same case where the data received from the BLE module also only contains SSID and Status key? I would like to understand whether it is the handling of received data is different on Android than on iOS, or is it that the data itself is missing other fields?

Could you share the adb logcat with stack trace of the crash?

rrivadeneira-rsc commented 5 years ago

Wouldn't the iOS version crash if it finds any null parameters in the object like the Android version does or display like so? I'm assuming that if the iOS app gets any null parameters then the next step, which is showing the scan results on the app, would fail to display any correct information. So the received data on the iOS side should be showing more than just the SSID and status key, if I am able to see the networks and select said networks to connect.

We'll work on getting you more logging information here soon. Thank you

KylePalko commented 5 years ago

The iOS app is able to properly decode the CBOR payload from the device and get the SSID, BSSID, index, etc. The Android app fails to do this—this is why I haven't put an issue in on the Amazon FREERTOS iOS BLE SDK repository and originally stated this above:

The iOS FREERTOS BLE demo version of the app decodes the CBOR payload from the device successfully with all the properties.

@bxpan I cannot provide an adb logcat at this time. Please run the app and you will see the issue.

bxpan commented 5 years ago

If the iOS app can decode the payload with all properties, it means that the data received from the device does contain all fields. I'll try to reproduce this issue on Android app to debug why it cannot be decoded correctly on Android app. Could you please let us know the version/branch you used for the BLE module on the device?

rrivadeneira-rsc commented 5 years ago

Sure! We are using a stable version of ble-beta, which I believe is from commit 17ba347f and AFR 1.4.7 (from March 27th)

bxpan commented 5 years ago

Hi KylePalko, do you know what is the MTU between your device and the android phone? Did you set the MTU from the demo app explicitly? One possibility is that the MTU is too small, and the listNetwork response got truncated, which could have caused the decoding to fail.

rrivadeneira-rsc commented 5 years ago

Isn't the MTU set automatically in the demo app or OS? I have tried this without setting anything explicitly and, on the module's side, I get the following: W (641051) BT_GATT: attribute value too long, to be truncated to 20 W (641051) BT_GATT: attribute value too long, to be truncated to 20 W (641051) BT_GATT: attribute value too long, to be truncated to 20 W (641061) BT_GATT: attribute value too long, to be truncated to 20 W (641071) BT_GATT: attribute value too long, to be truncated to 20 W (641071) BT_GATT: attribute value too long, to be truncated to 20 W (641081) BT_GATT: attribute value too long, to be truncated to 20 W (641091) BT_GATT: attribute value too long, to be truncated to 20 W (641091) BT_GATT: attribute value too long, to be truncated to 20 W (641101) BT_GATT: attribute value too long, to be truncated to 20 W (641111) BT_GATT: attribute value too long, to be truncated to 20

And a blank screen on the mobile app. Is this what you are looking to see if the MTU is too small?

bxpan commented 5 years ago

Yes, this is due to mtu too small. On iOS, MTU is set automatically by the OS. But on Android, it must be explicitly set. In the version of demo app you were using, the MTU must be set by the demo app. Once the BLE is connected, click on the "more" menu button, click on "set mtu" to set the MTU explicitly. Could you try this out and see if this resolves the issue?

In the latest commit of the SDK, the SDK will set the MTU automatically, so that the app does not need to explicitly do that. If you pull the latest change on the beta branch, you should not need to set mtu from app anymore. Please let us know if this resolves the issue.

rrivadeneira-rsc commented 5 years ago

Ah ok, I see. That makes sense. That resolved the issue for me, I see scan results and I'm able to provision the module with the Android demo. However, I will let @KylePalko confirm this is what he was looking for and if it resolves the issue on his side.

Thank you for your help!

KylePalko commented 5 years ago

Thank-you @bxpan !

bxpan commented 5 years ago

Glad that it resolved the issue.

Thank you for your interest in AmazonFreeRTOS BLE Android SDK. Please feel free to open a new issue if you have other questions.