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

Bluetooth onWrite callback uses the wrong characteristic #3153

Closed
JasXSL opened this issue Aug 29, 2019 · 8 comments
Closed

Bluetooth onWrite callback uses the wrong characteristic #3153

JasXSL opened this issue Aug 29, 2019 · 8 comments
Labels
Status: Stale Issue is stale stage (outdated/stuck)

Comments

@JasXSL
Copy link

JasXSL commented Aug 29, 2019

Hardware:

Board: ESP32 Dev Module
Core Installation/update date: 29/aug/2019
IDE name: Arduino IDE
Flash Frequency: 80Mhz
PSRAM enabled: no
Upload Speed: 921600
Computer OS: Windows 10

Description:

Not sure if this issue was created recently, but for some reason the BLECharacteristic in the onWrite event is always the first one added to the service.

For an instance. Writing a value to "ba5edca7-face-d00d-c0de-000000000001" using nrf connect causes onWrite to fire with pCharacteristic as "ba5edca7-face-d00d-c0de-0000000000".

The value does get set though, because if I disconnect in nrf connect and reconnect, the updated value is set. But if I need to be able to get which characteristic was changed in the application.

Edit: Looking closer at what I pasted. It looks like pCharacteristic->getUUID() is shifting off the last 2 characters of the UUID. Is this a bug or by design?

Header

class MySecurity : public BLESecurityCallbacks {
	private:
		uint32_t onPassKeyRequest();
		void onPassKeyNotify(uint32_t pass_key);
		bool onConfirmPIN(uint32_t pass_key);
		bool onSecurityRequest();
		void onAuthenticationComplete(esp_ble_auth_cmpl_t cmpl);
};

class MyServer : public BLEServerCallbacks {
	private:
		void onConnect(BLEServer* pServer);
		//void onConnect(BLEServer* pServer, esp_ble_gatts_cb_param_t *param);
		void onDisconnect(BLEServer* pServer);
};

class bleCharacteristicCallback: public BLECharacteristicCallbacks{
	public:
		static constexpr char SERVICE_MAIN[37] = "ba5edca7-face-d00d-c0de-000000001337";
		
		static constexpr char C_PHOTO_THRESH_VAL[37] = "ba5edca7-face-d00d-c0de-000000000000";
		static constexpr char C_PHOTO_TOGGLE_ENABLED[37] = "ba5edca7-face-d00d-c0de-000000000001";

	private:
		void onWrite(BLECharacteristic* pCharacteristic);
		void onRead(BLECharacteristic* pCharacteristic);
};

class MainBLEServer: public Task{

	public:
		bool initialized = false;
		bool pairing_enabled = false;
		long last_disconnect = 1;				// 0 = we're actively connected
		void toggleBonding(bool on = false);
	private:
		void run(void *data);
};


extern MainBLEServer* bBluetooth;
extern BLESecurity* bSecurity;

CPP File

MainBLEServer* bBluetooth;
BLESecurity *bSecurity;
BLEScan* pBLEScan;
BLEService* pService;

uint32_t MySecurity::onPassKeyRequest(){
	Serial.println("PassKeyRequest");
	return 123456;
}
void MySecurity::onPassKeyNotify(uint32_t pass_key){
	Serial.printf("The passkey Notify number:%d\n", pass_key);
}
bool MySecurity::onConfirmPIN(uint32_t pass_key){
	Serial.printf("The passkey YES/NO number:%d\n", pass_key);
	vTaskDelay(5000);
	return true;
}
bool MySecurity::onSecurityRequest(){
	Serial.println("SecurityRequest");
	return true;
}

void MySecurity::onAuthenticationComplete(esp_ble_auth_cmpl_t cmpl){

	// A device connected here
	Serial.println("onAuthenticationComplete");
	if(cmpl.success){

		uint16_t length;
		esp_ble_gap_get_whitelist_size(&length);
		Serial.printf("size: %d\n", length);
		bConfig.bonded = true;
		bConfig.save();
		bBluetooth->toggleBonding(false);

	}
	
}

// only one device can be connected at a time
void MyServer::onConnect(BLEServer* pServer){
	Serial.println("Client connected");
	bBluetooth->last_disconnect = 0;
}

void MyServer::onDisconnect(BLEServer* pServer){
	Serial.println("Client disconnected");
	bBluetooth->last_disconnect = 1;		// Turn it off immediately or within 60 sec of boot
}

constexpr char bleCharacteristicCallback::SERVICE_MAIN[37];
constexpr char bleCharacteristicCallback::C_PHOTO_THRESH_VAL[37];
constexpr char bleCharacteristicCallback::C_PHOTO_TOGGLE_ENABLED[37];

void bleCharacteristicCallback::onWrite(BLECharacteristic* pCharacteristic) {

	String uuid = pCharacteristic->getUUID().toString().c_str();
	
	// Front light colors, expecting a 3x byte array
	Serial.printf("BT UUID write received %s\n", uuid.c_str());
	std::string value = pCharacteristic->getValue();
	if( uuid == bleCharacteristicCallback::C_PHOTO_THRESH_VAL ){ 
		unsigned int val = (value[0]<<8)|value[1];
		bPhotoresistor.thresh_val = val;
		bConfig.save();
	}

	if( uuid == bleCharacteristicCallback::C_PHOTO_TOGGLE_ENABLED ){ 
		bPhotoresistor.photo_toggle_enabled = value[0];
		bConfig.save();
	}

}

void bleCharacteristicCallback::onRead(BLECharacteristic* pCharacteristic) {
	std::string msg = pCharacteristic->getValue();
	Serial.printf("BLE received: %s, %i\n", msg.c_str(), msg.length());
}

void MainBLEServer::toggleBonding( bool on ){

	pairing_enabled = on;
	if( on ){
		bSecurity->setCapability(ESP_IO_CAP_NONE);
		//statusLED.setBluetoothPairable( true );
		Serial.println("Enabling bluetooth bonding");
	}
	else{
		Serial.println("Turning off bonding");
		bSecurity->setCapability(ESP_IO_CAP_IN);
		//statusLED.setBluetoothPairable( false );
	}
}


void MainBLEServer::run(void *data) {
			
	Serial.println("Starting BLE work!");
	BLEDevice::init("Bike");
	BLEServer* pServer = BLEDevice::createServer();
	pServer->setCallbacks(new MyServer());

	BLEDevice::setEncryptionLevel(ESP_BLE_SEC_ENCRYPT);
	BLEDevice::setSecurityCallbacks(new MySecurity());

	pService = pServer->createService(bleCharacteristicCallback::SERVICE_MAIN);

	
	Serial.println("Creating callback");
	bleCharacteristicCallback *callback = new bleCharacteristicCallback();

	Serial.println("Setting up");
	byte out[] = {0};
	

	// RW Characteristics
	Serial.println();
	Serial.printf("Adding PHOTO_THRESH_VAL\n");
	BLECharacteristic* pCharacteristic = pService->createCharacteristic(
		BLEUUID(bleCharacteristicCallback::C_PHOTO_THRESH_VAL),
		BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_WRITE
	);
	pCharacteristic->setAccessPermissions(ESP_GATT_PERM_READ_ENCRYPTED | ESP_GATT_PERM_WRITE_ENCRYPTED);
	pCharacteristic->setCallbacks(callback);
	out[1] = bPhotoresistor.thresh_val&0xFF;
	out[0] = (bPhotoresistor.thresh_val>>8)&0xFF;
	pCharacteristic->setValue(out, 2);


	Serial.printf("Adding C_PHOTO_TOGGLE_ENABLED\n");
	pCharacteristic = pService->createCharacteristic(
		BLEUUID(bleCharacteristicCallback::C_PHOTO_TOGGLE_ENABLED),
		BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_WRITE
	);
	pCharacteristic->setAccessPermissions(ESP_GATT_PERM_READ_ENCRYPTED | ESP_GATT_PERM_WRITE_ENCRYPTED);
	pCharacteristic->setCallbacks(callback);
	out[0] = bPhotoresistor.photo_toggle_enabled;
	pCharacteristic->setValue(out, 1);


	Serial.println("Starting");

	pService->start();
	BLEAdvertising* pAdvertising = pServer->getAdvertising();
	pAdvertising->addServiceUUID(BLEUUID(pService->getUUID()));
	pAdvertising->setScanResponse(true);

	bSecurity = new BLESecurity();
	bSecurity->setAuthenticationMode(ESP_LE_AUTH_BOND);
	//bSecurity->setCapability(ESP_IO_CAP_OUT);
	//bSecurity->setInitEncryptionKey(ESP_BLE_ENC_KEY_MASK | ESP_BLE_ID_KEY_MASK);
	bSecurity->setCapability(ESP_IO_CAP_IN);
	pAdvertising->start();
	Serial.println("Advertising started!");
	delay(portMAX_DELAY);
}

ino setup

   bBluetooth = new MainBLEServer();
    bBluetooth->setStackSize(20000);
    bBluetooth->start();
@william-ferguson-au
Copy link
Contributor

I can confirm that every BLECharacteristic received by the BleCharacteristicCallback has its rightmost 2 omitted. This used to work in v1.0.2, so I would say that it is a bug introduced in 1.0.3

I (41863) wt_bluetooth: StringCompareResult=-57 '6e99a24f-e50c-425d-acce-b92dbad49c' '6e99a24f-e50c-425d-acce-b92dbad49c90'

Characteristic given to the server and visible to other clients is ''6e99a24f-e50c-425d-acce-b92dbad49c90'', but the callback receives that UUID with the rightmost 2 characters missing.

@william-ferguson-au
Copy link
Contributor

Suggest you update the issue title to reflect the actual error.

@william-ferguson-au
Copy link
Contributor

Actually, this bug is worse than just omitting the last 2 chars. I am seeing BLECharacteristic UUID and their values come back partially overwritten and truncated.

Something within the BLE library is referencing memory incorrectly.

@william-ferguson-au
Copy link
Contributor

OK, retrieved using 1.0.3 with the BLE sources from 1.0.2 and I'm still getting the memory corruption showing up BLECharacteristic UUIDs. So it's a memory issue somewhere else in 1.0.3.

This is now beyond the budget I have, so I'm going to have to revert to 1.0.2

@me-no-dev this is a big worry for 1.0.3 (I tested on master too).

@william-ferguson-au
Copy link
Contributor

OK, looks like I managed to convince myself that I had was working on 1.0.2 when that wasn't true.. Definitely failing for both 1.0.2 and 1.0.3 and master.

@william-ferguson-au
Copy link
Contributor

I have created a very simple project https://github.com/william-ferguson-au/arduino-esp32-issue-3153 It shows that the UUID is corrupt when given to the BLECharcateristic callback.
It also shows that if the length of the BLECharacteristic value exceeds 15 chars then it too becomes corrupted.

I suspect this is an issue in the underlying esp-idf BT library.

@stale
Copy link

stale bot commented Nov 28, 2019

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Stale Issue is stale stage (outdated/stuck) label Nov 28, 2019
@stale
Copy link

stale bot commented Dec 12, 2019

[STALE_DEL] This stale issue has been automatically closed. Thank you for your contributions.

@stale stale bot closed this as completed Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Stale Issue is stale stage (outdated/stuck)
Projects
None yet
Development

No branches or pull requests

2 participants