NimBLE Server - NIMBLE_PROPERTY::READ leads to crash about nimble-arduino HOT 15 CLOSED

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

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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"

from nimble-arduino.

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.

from nimble-arduino.

@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.

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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.

from nimble-arduino.

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!

from nimble-arduino.

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

from nimble-arduino.