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

NimBLE Server - NIMBLE_PROPERTY::READ leads to crash #8

Closed
beegee-tokyo opened this issue Apr 5, 2020 · 15 comments
Closed

NimBLE Server - NIMBLE_PROPERTY::READ leads to crash #8

beegee-tokyo opened this issue Apr 5, 2020 · 15 comments

Comments

@beegee-tokyo
Copy link
Contributor

HI @h2zero I know you are busy with fixing the Client stuff, but something on the Server side broke along the way.
Simple custom characteristic creation using NIMBLE_PROPERTY::READ makes the app crash on pService->start().

Example code:

// Includes for BLE Arduino-NimBLE
#include <NimBLEUtils.h>
#include <NimBLEServer.h>
#include <NimBLEDevice.h>
#include <NimBLEAdvertising.h>

/** Unique device name */
char apName[] = "ESP32-xxxxxxxxxxxx";

// List of Service and Characteristic UUIDs
#define SERVICE_UUID "0000AAAA-EAD2-11E7-80C1-9A214CF093AE"
#define WIFI_UUID "00005555-EAD2-11E7-80C1-9A214CF093AE"

... // skipping the callbacks
/**
 * initBLE
 * Initialize BLE service and characteristic
 * Start BLE server and service advertising
 */
void initBLE()
{
	// Initialize BLE and set output power
	BLEDevice::init(apName);
	BLEDevice::setPower(ESP_PWR_LVL_P7);
	BLEDevice::setMTU(256);

	Serial.printf("BLE advertising using %s\n", apName);

	// Create BLE Server
	pServer = BLEDevice::createServer();

	// Set server callbacks
	pServer->setCallbacks(new MyServerCallbacks());

	// Create BLE Service
	pService = pServer->createService(SERVICE_UUID);

	// Create BLE Characteristic for WiFi settings
	pCharacteristicWiFi = pService->createCharacteristic(
		// BLEUUID(WIFI_UUID),
		WIFI_UUID,
		NIMBLE_PROPERTY::READ |
			NIMBLE_PROPERTY::WRITE);

	/** Add properties the same way as characteristics now **/
	pCharacteristicWiFi->createDescriptor("2902");

	pCharacteristicWiFi->setCallbacks(new MyCallbackHandler());

	// Start the service
	pService->start();

	// Start advertising
	pAdvertising = pServer->getAdvertising();
	pAdvertising->addServiceUUID(WIFI_UUID);
	pAdvertising->start();
}

Serial debug output:

Build: Apr  5 2020 10:15:02
Read from preferences:
primary SSID: XYZ2 password: secret
secondary SSID: XYZ3 password: secret
I NimBLEDevice: "BLE Host Task Started"
I NimBLEDevice: "NimBle host synced."
D NimBLEDevice: ">> setPower: 7"
D NimBLEDevice: "<< setPower"
BLE advertising using ESP32-3C71BF6CE284
Guru Meditation Error: Core  1 panic'ed (LoadProhibited). Exception was unhandled.
Core 1 register dump:
PC      : 0x400e43e3  PS      : 0x00060530  A0      : 0x800d65d8  A1      : 0x3ffcb5c0  
A2      : 0x00000004  A3      : 0x3ffcb600  A4      : 0x3ffc3600  A5      : 0x00000002  
A6      : 0x3ffcb610  A7      : 0x3ffce201  A8      : 0x00002902  A9      : 0x00000000  
A10     : 0x00000000  A11     : 0x40085110  A12     : 0x3ffce444  A13     : 0x3ffcb538  
A14     : 0x3ffcb5a0  A15     : 0xfefefe01  SAR     : 0x00000018  EXCCAUSE: 0x0000001c  
EXCVADDR: 0x00000004  LBEG    : 0x4000c2e0  LEND    : 0x4000c2f6  LCOUNT  : 0x00000000  

Backtrace: 0x400e43e3:0x3ffcb5c0 0x400d65d5:0x3ffcb5e0 0x400d5f2d:0x3ffcb600 0x400d1de1:0x3ffcb690 0x400d233f:0x3ffcb6d0 0x400eb93f:0x3ffcb790 0x4008dadd:0x3ffcb7b0

Decoded backtrace:

PC: 0x400e43e3: ble_uuid_cmp at .pio\libdeps\esp32devmaxapp\NimBLE-Arduino\src\nimble\host\src\ble_uuid.c line 71
EXCVADDR: 0x00000004

Decoding stack results
0x400e43e3: ble_uuid_cmp at .pio\libdeps\esp32devmaxapp\NimBLE-Arduino\src\nimble\host\src\ble_uuid.c line 71
0x400d65d5: NimBLEUUID::equals(NimBLEUUID) at .pio\libdeps\esp32devmaxapp\NimBLE-Arduino\src\NimBLEUUID.cpp line 181
0x400d5f2d: NimBLEService::start() at .pio\libdeps\esp32devmaxapp\NimBLE-Arduino\src\NimBLEService.cpp line 129
0x400d1de1: initBLE() at src\main.cpp line 355
0x400d233f: setup() at src\main.cpp line 531
0x400eb93f: loopTask(void*) at C:\users\beegee\.platformio\packages\framework-arduinoespressif32\cores\esp32\main.cpp line 14
0x4008dadd: vPortTaskWrapper at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/port.c line 143

I tried as well with the BLE Uart example and it crashes at the same place. Btw. there is an error in the BLE Uart example.

	// Create a BLE Characteristic
	pTxCharacteristic = pService->createCharacteristic(
		CHARACTERISTIC_UUID_TX,
		/******* Enum Type NIMBLE_PROPERTY now *******      
                                        BLECharacteristic::PROPERTY_WRITE
                                        );
                                    **********************************************/
		NIMBLE_PROPERTY::WRITE);

should be

	// Create a BLE Characteristic
	pTxCharacteristic = pService->createCharacteristic(
		CHARACTERISTIC_UUID_TX,
		/******* Enum Type NIMBLE_PROPERTY now *******      
                                        BLECharacteristic::PROPERTY_WRITE
                                        );
                                    **********************************************/
		NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::NOTIFY);
@h2zero
Copy link
Owner

h2zero commented Apr 5, 2020

Thanks for reporting, strange behavior indeed. I'll look into this tomorrow.

@beegee-tokyo
Copy link
Contributor Author

I think I found it, at least how to avoid the crash.
Removing pCharacteristicWiFi->createDescriptor("2902") fixes the crash.

	// Create BLE Characteristic for WiFi settings
	pCharacteristicWiFi = pService->createCharacteristic(
		BLEUUID(WIFI_UUID),
		// WIFI_UUID,
		NIMBLE_PROPERTY::READ | // NIMBLE_PROPERTY::READNOTIFY |
			NIMBLE_PROPERTY::WRITE);  // | NIMBLE_PROPERTY::READ_ENC | NIMBLE_PROPERTY::WRITE_ENC);

	/** Add properties the same way as characteristics now **/
	// pCharacteristicWiFi->createDescriptor("2902" /** , NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::WRITE **/);

But now I got a different problem.
In BLECharacteristicCallbacks in void onWrite(BLECharacteristic *pCharacteristic) I always get only 18 bytes, while the connected client sends something between 70 and 80 bytes.
The buffer content of 18 bytes is the first 18 bytes of the data sent from the Android app.

This application was working with the server_dev branch before. I just retried yesterday with the final library for confirmation.

@h2zero
Copy link
Owner

h2zero commented Apr 5, 2020

I have located the source of the error and it's actually in the application code. If you create a 2902 descriptor without NIMBLE_PROPERTY::NOTIFY or NIMBLE_PROPERTY::INDICATE properties it causes this error.

As for the fix, I would suggest adding one of those properties to the characteristic you are creating the descriptor with as without those set it probably won't function the way you expect.

I'm considering adding those properties automatically if a 2902 is created without them but that would be encouraging bad practice. The other option is to ignore it completely but then there would be complaints that it's not working lol. The final option is to allow it to be created anyway but I'm unsure if notifications would even be sent, which may be even more confusing.

As for the number of bytes you're receiving I am unsure why that would be other than perhaps your android app has not exchanged MTU info with the esp32. Nothing in that code has changed since merging the ServerDev branch. I'll see what I can find.

@chegewara
Copy link

I'm considering adding those properties automatically if a 2902 is created without them but that would be encouraging bad practice.

Maybe just add assert, which will crash or at least display error/warning about missing property, because with descriptor you can have either notification or indication.

I always get only 18 bytes, while the connected client sends something between 70 and 80 bytes.

Indeed it looks like MTU = 23.

@beegee-tokyo
Copy link
Contributor Author

I understand the problem with the 2902 descriptor and will adjust the code.

For the 18 bytes, when I compile the same code with the ESP32-Arduino BLE library it works without problems. It happens only with NimBLE. On Android side I use the same app in both cases.
I tried to set MTU on ESP32 side to 256 (see above code example) but that doesn't change anything.
When using ESP32-Arduino BLE I do not set any MTU size and it works. What is the default MTU size in the original BLE library.?
I will check in the Android app how I can request a larger MTU.

@chegewara, just a comment, 18+3 would indicate a MTU of 21, not 25.

@h2zero
Copy link
Owner

h2zero commented Apr 5, 2020

Just tested it and if a 2902 is created without the properties set on the characteristic for notfiy or indicate the client app will not provide a means to subscribe.

@chegewara I agree an assert/message is probably best.

@beegee-tokyo
Copy link
Contributor Author

Just thinking....

If I don't use Notify or Indicate, I don't need the 2902 descriptor.

Will test tomorrow together with the MTU problem.

@h2zero
Copy link
Owner

h2zero commented Apr 5, 2020

That's correct, if you aren't using notification/indication there is no need for a 2902.

If you set log level to debug you will see if/when the MTU is exchanged and what the value is.

With my phone I get this:

D NimBLEServer: ">> handleGapEvent: BLE_GAP_EVENT_MTU"
I NimBLEServer: "mtu update event; conn_handle=0 mtu=185"

@chegewara
Copy link

chegewara commented Apr 5, 2020

just a comment, 18+3 would indicate a MTU of 21, not 25.

You are correct, write request OP code takes 3 bytes, so it should be 20 bytes of data, unless NimBLE is bugged.

@beegee-tokyo
Copy link
Contributor Author

@h2zero side problem
I have a problem with the debug output in PlatformIO.
If I include the library with lib_deps it doesn't recognize the CORE_LEVEL_DEBUG (or ARDUHAL_DEBUG_LEVEL) setting in platformio.ini. Only if I copy the library into the lib folder of the ithe project I get some debug output, but only until the BLE server started, then all debug output stops. I have to manipulate NimBLE_Log.h to enable debug output. I didn't try on ArduinoIDE, because that's not my preferred IDE.
Temporary problem, not sure why that happens.

@h2zero
Copy link
Owner

h2zero commented Apr 5, 2020

Interesting finding on the MTU, seems NimBLE uses 5 bytes and not 3 for the operation command header.

Or maybe not, I think I was looking at the log wrong.

@beegee-tokyo
Copy link
Contributor Author

Well, forget the problem for now, it might be related to the Android BLE library I use.

I made another test with a customers Android app, which uses nRF52 BLE library and it works just fine there. It uses the same custom characteristic with both READ and WRITE.

I will have to dig into the Android BLE library I use for the app causing the problem.

@h2zero
Copy link
Owner

h2zero commented Apr 6, 2020

Look for an api call to begin MTU exchange in your library and call it after the connection opens. Also I'd like to say thanks for this issue as I now know I need to implement long reads and writes. I already started on the code for that and will push a new branch up shortly with some of that and the latest bugfixes.

@beegee-tokyo
Copy link
Contributor Author

gatt.requestMtu(256); in the Android app solved the problem with the cut off data.

Still wondering why I didn't need it in the ESP32-Arduino BLE library.

But I am good now. Thanks for the support!

@chegewara
Copy link

Also I'd like to say thanks for this issue as I now know I need to implement long reads and writes.

@beegee-tokyo Because this is not implemented yet and its working in old library

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

No branches or pull requests

3 participants