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

Improper characteristic list implementation of LBLEClient class #90

Closed
sfyang opened this issue Feb 7, 2018 · 1 comment
Closed

Improper characteristic list implementation of LBLEClient class #90

sfyang opened this issue Feb 7, 2018 · 1 comment
Assignees

Comments

@sfyang
Copy link

sfyang commented Feb 7, 2018

There are several issues concerning the LBLEClient class declared in LBLECentral.h and implemented in LBLECentral.cpp, and this is one of them.

With current implementation of the LBLEClient class, a flat characteristic list m_characteristics implemented with std::map container using the UUID of the characteristic as the key and the value handle as the value is used to store the characteristic found by the protected LBLEClient::discoverCharacteristicsOfService() method. This way of the implementation could have the following problems:

To demonstrate this issue, consider the following slightly modified version of the 'ConnectPeripheral.ino' example code from the LBLE library, which is extended to add methods and code segment for dumping the content of the characteristic list:

/*
  This example scans nearby BLE peripherals and prints the peripherals found.

  created Mar 2017 by MediaTek Labs
*/

#include <LBLE.h>
#include <LBLECentral.h>
#include <iterator>

class ABLEClient : public LBLEClient
{
public:
  int getCharacteristicCount()
  {
    return m_characteristics.size();
  }

  LBLEUuid getCharacteristicUuid(int index)
  {
    if (index < 0 || index >= getCharacteristicCount()) {
      return LBLEUuid();
    }

    auto c = m_characteristics.begin();
    std::advance(c, index);

    return c->first;
  }
};

ABLEClient client;

void setup() {
  //Initialize serial
  Serial.begin(9600);

  // Initialize BLE subsystem
  Serial.println("BLE begin");
  LBLE.begin();
  while (!LBLE.ready()) {
    delay(10);
  }
  Serial.println("BLE ready");

  // start scanning nearby advertisements
  LBLECentral.scan();
}

void printDeviceInfo(int i) {
  Serial.print(i);
  Serial.print("\t");
  Serial.print(LBLECentral.getAddress(i));
  Serial.print("\t");
  Serial.print(LBLECentral.getAdvertisementFlag(i), HEX);
  Serial.print("\t");
  Serial.print(LBLECentral.getRSSI(i));
  Serial.print("\t");
  const String name = LBLECentral.getName(i);
  Serial.print(name);
  if(name.length() == 0)
  {
    Serial.print("(Unknown)");
  }
  Serial.print(" by ");
  const String manu = LBLECentral.getManufacturer(i);
  Serial.print(manu);
  Serial.print(", service: ");
  if (!LBLECentral.getServiceUuid(i).isEmpty()) {
    Serial.print(LBLECentral.getServiceUuid(i));
  } else {
    Serial.print("(no service info)");
  }

  if (LBLECentral.isIBeacon(i)) {
    LBLEUuid uuid;
    uint16_t major = 0, minor = 0;
    int8_t txPower = 0;
    LBLECentral.getIBeaconInfo(i, uuid, major, minor, txPower);

    Serial.print(" ");
    Serial.print("iBeacon->");
    Serial.print("  UUID: ");
    Serial.print(uuid);
    Serial.print("\tMajor:");
    Serial.print(major);
    Serial.print("\tMinor:");
    Serial.print(minor);
    Serial.print("\ttxPower:");
    Serial.print(txPower);
  }

  Serial.println();
}

int searching = 1;

enum AppState
{
  SEARCHING,    // We scan nearby devices and provide a list for user to choose from
  CONNECTING,   // User has choose the device to connect to
  CONNECTED     // We have connected to the device
};

void loop() {
  static AppState state = SEARCHING;
  static LBLEAddress serverAddress;

  // check if we're forcefully disconnected.
  if(state == CONNECTED)
  {
    if(!client.connected())
    {
      Serial.println("disconnected from remote device");
      state = SEARCHING;
    }
  }
  
  switch(state)
  {
  case SEARCHING:
    {
      // wait for a while
      Serial.println("state=SEARCHING");
      for(int i = 0; i < 10; ++i)
      {
        delay(1000);
        Serial.print(".");
      }
      // enumerate advertisements found.
      Serial.print("Peripherals found = ");
      Serial.println(LBLECentral.getPeripheralCount());
      Serial.println("idx\taddress\t\t\tflag\tRSSI");
      for (int i = 0; i < LBLECentral.getPeripheralCount(); ++i) {
        printDeviceInfo(i);
      }

      // waiting for user input
      Serial.println("Select the index of device to connect to: ");
      while(!Serial.available())
      {
        delay(100);
      }

      const int idx = Serial.parseInt();

      if(idx < 0 || idx >= LBLECentral.getPeripheralCount())
      {
        Serial.println("wrong index, keep scanning devices.");
      }
      else
      {
        serverAddress = LBLECentral.getBLEAddress(idx);
        Serial.print("Connect to device with address ");
        Serial.println(serverAddress);
        // we must stop scan before connecting to devices
        LBLECentral.stopScan();
        state = CONNECTING;
      }
    }
    break;
  case CONNECTING:
  {
    Serial.println("state=CONNECTING");
    client.connect(serverAddress);
    if(client.connected())
    {
      state = CONNECTED;
    }
    else
    {
      Serial.println("can't connect");
    }
  }
  break;
  case CONNECTED:
  {
    Serial.println("state=CONNECTED");

    // display all services of the remote device
    const int serviceCount = client.getServiceCount();
    Serial.println("available services = ");
    for(int i = 0; i < serviceCount; ++i)
    {
      Serial.print("\t - ");
      const String serviceName = client.getServiceName(i);
      if(serviceName.length())
      {
        Serial.print("[");
        Serial.print(serviceName);
        Serial.print("] ");
      }
      Serial.println(client.getServiceUuid(i));
      
    }
    const int characteristicCount = client.getCharacteristicCount();
    Serial.println("available characteristics = ");
    for (int i = 0; i < characteristicCount; i++)
    {
      Serial.print("\t - ");
      Serial.println(client.getCharacteristicUuid(i));
    }

    // read the device manufacture - 
    // first we check if "Device Information"(0x180A) service is available:
    if(client.hasService(0x180A))
    {
      const String name = client.readCharacteristicString(LBLEUuid(0x2A29));
      if(name.length() > 0)
      {
        Serial.print("manufacturer=");
        Serial.println(name);
      }
    }
    else
    {
      Serial.println("No device information found");
    }

    // check if there is "Link Loss (0x1803)" available.
    // This service is usually used by the Proximity profile
    // https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.service.link_loss.xml
    if(client.hasService(0x1803))
    {
      Serial.println("Link Loss service found");
      
      // 0x2A06 (https://www.bluetooth.com/specifications/gatt/viewer?attributeXmlFile=org.bluetooth.characteristic.alert_level.xml)
      // is a uint_8 characteristic that we can read and write
      Serial.print("Alert level = ");
      char c = client.readCharacteristicChar(0x2A06);
      Serial.println((int)c);

      Serial.println("set alert level to HIGH(2)");
      client.writeCharacteristicChar(LBLEUuid(0x2A06), 2);
    }
    
    // enter idle state.
    while(true)
    {
      delay(100);
    }
  }
  break;
  }
}

Now run the code on the LinkIt 7697 HDK and connect to a peripheral having several custom services which have some common characteristic with UUID 'E604E95D-A759-4817-87D3-AA005083A0D1' (yes, this is a nRF52-dk running Apple HomeKit server, and the peripheral list of the scanning result is shortened.):

BLE begin
BLE ready
state=SEARCHING
..........Peripherals found = 27
idx	address			flag	RSSI
4	FC:86:7A:4E:E5:F1(RAN)	6	-48	Nordic  by Apple, Inc., service: (no service info)
Select the index of device to connect to: 
Connect to device with address FC:86:7A:4E:E5:F1(RAN)
state=CONNECTING
state=CONNECTED
available services = 
	 - [generic_access] 0x1800
	 - [generic_attribute] 0x1801
	 - 0000003E-0000-1000-8000-0026BB765291
	 - 000000A2-0000-1000-8000-0026BB765291
	 - 00000055-0000-1000-8000-0026BB765291
	 - 00001530-1212-EFDE-1523-785FEABCD123
	 - 00000045-0000-1000-8000-0026BB765291
	 - 00000044-0000-1000-8000-0026BB765291
available characteristics = 
	 - 0x2a00
	 - 0x2a01
	 - 0x2a04
	 - 0x2aa6
	 - 00000005-0000-1000-8000-0026BB765291
	 - 00000014-0000-1000-8000-0026BB765291
	 - 00000037-0000-1000-8000-0026BB765291
	 - 0000004C-0000-1000-8000-0026BB765291
	 - 000000A5-0000-1000-8000-0026BB765291
	 - E604E95D-A759-4817-87D3-AA005083A0D1
No device information found

Here is a Node.js code main.js using the noble library to perform similar tasks as the ConnectPeripheral.ino does:

/*
 *
 * Copyright (C) 2018 Frank Yang
 *
 * This is free software, licensed under the GNU General Public License v3.
 *
 * This code connect to the specified peripheral and perform service
 * and characteristic discovery, and then dump the result in a similar
 * way to the ConnectPeripheral.ino example from the LBLE library of
 * the LinkIt 7697 Arduino BSP package.
*/
const noble = require('noble');

var target;

if (process.argv[2]) {
	target = process.argv[2].toLowerCase();
} else {
	console.log('Usage: ' + process.argv[1] + ' peripheral_address');
	process.exit(1);
}

function connect_target_device(peripheral, target)
{
	if (peripheral.id == target || peripheral.address == target) {
		noble.stopScanning();
		explore_peripheral(peripheral);
	}
}

function explore_peripheral(peripheral)
{
	console.log('Connect to device with address: ' + peripheral.address + '(' + peripheral.addressType + ')');

	peripheral.on('disconnect', function() {
		console.log('Device ' + peripheral.address + '(' + peripheral.addressType + ') disconnected.');
		process.exit(0);
	});

	peripheral.on('exploreCompleted', function(peripheral) {
		dump_peripheral(peripheral);
	});

	peripheral.connect(function(error) {
		if (error) {
			console.log('ERROR: unable to connect to ' + peripheral.address + '(' + peripheral.addressType + '): ' + error.toString());
			process.exit(1);
		}

		discover_services(peripheral);
	});
}

function discover_services(peripheral)
{
	peripheral.flat_service_list = [];
	peripheral.flat_characteristic_list = [];
	
	peripheral.discoverServices([], function(error, services) {
		var svc;
		var i;

		for (i in services) {
			svc = services[i];

			peripheral.flat_service_list.push({name: svc.name, uuid: svc.uuid});

			discover_characteristics(svc, peripheral, (i == (services.length - 1)));
		}
	});
}

function discover_characteristics(service, peripheral, last)
{
	var characteristic_list = peripheral.flat_characteristic_list;

	service.discoverCharacteristics([], function(error, characteristics) {
		var chr;
		var i;

		for (i in characteristics) {
			chr = characteristics[i];

			characteristic_list.push(chr.uuid);

			if (last && (i == (characteristics.length - 1))) {
				peripheral.emit('exploreCompleted', peripheral);
			}
		}
	});
}

function dump_peripheral(peripheral)
{
	var i;
	var s;

	console.log('available services =');
	for (i = 0; i < peripheral.flat_service_list.length; i++) {
		s = peripheral.flat_service_list[i];
		if (s.name) {
			console.log('\t - [' + s.name + '] ' + s.uuid);
		} else {
			console.log('\t - ' + s.uuid);
		}
	}
	console.log('available chacteristics =');
	peripheral.flat_characteristic_list.sort();
	for (i = 0; i < peripheral.flat_characteristic_list.length; i++) {
		console.log('\t - ' + peripheral.flat_characteristic_list[i]);
	}
}

noble.on('stateChange', function(state) {
	if (state == 'poweredOn') {
		noble.startScanning();
	} else {
		noble.stopScanning();
	}
});

noble.on('discover', function(peripheral) {
	connect_target_device(peripheral, target);
});

And the output of the above-mentioned code connecting to the same device:

$ node main.js FC:86:7A:4E:E5:F1
Connect to device with address: fc:86:7a:4e:e5:f1(random)
available services =
	 - [Generic Access] 1800
	 - [Generic Attribute] 1801
	 - 0000003e0000100080000026bb765291
	 - 000000a20000100080000026bb765291
	 - 000000550000100080000026bb765291
	 - 000015301212efde1523785feabcd123
	 - 000000450000100080000026bb765291
	 - 000000440000100080000026bb765291
available chacteristics =
	 - 000000010000100080000026bb765291
	 - 000000050000100080000026bb765291
	 - 0000000e0000100080000026bb765291
	 - 000000140000100080000026bb765291
	 - 000000190000100080000026bb765291
	 - 0000001a0000100080000026bb765291
	 - 0000001c0000100080000026bb765291
	 - 0000001d0000100080000026bb765291
	 - 0000001e0000100080000026bb765291
	 - 0000001f0000100080000026bb765291
	 - 000000200000100080000026bb765291
	 - 000000210000100080000026bb765291
	 - 000000220000100080000026bb765291
	 - 000000230000100080000026bb765291
	 - 000000230000100080000026bb765291
	 - 000000300000100080000026bb765291
	 - 000000370000100080000026bb765291
	 - 000000370000100080000026bb765291
	 - 0000004c0000100080000026bb765291
	 - 0000004e0000100080000026bb765291
	 - 0000004f0000100080000026bb765291
	 - 000000500000100080000026bb765291
	 - 000000520000100080000026bb765291
	 - 000000530000100080000026bb765291
	 - 000000a50000100080000026bb765291
	 - 000000a50000100080000026bb765291
	 - 000015311212efde1523785feabcd123
	 - 2a00
	 - 2a01
	 - 2a04
	 - 2aa6
	 - e604e95da759481787d3aa005083a0d1
	 - e604e95da759481787d3aa005083a0d1
	 - e604e95da759481787d3aa005083a0d1
	 - e604e95da759481787d3aa005083a0d1
	 - e604e95da759481787d3aa005083a0d1
	 - e604e95da759481787d3aa005083a0d1
Device fc:86:7a:4e:e5:f1(random) disconnected.

Other than quite some obviously missing characteristics which itself is another serious issue to the LBLE library, the characteristic list from the ABLEClient class, a extended version of the LBLEClient class, does not show the duplicated characteristics with UUID 'E604E95D-A759-4817-87D3-AA005083A0D1'. This also means that those duplicated characteristics missing from the list can never be written using the LBLEClient::writeCharacteristic() method under current implementation if they are writable characteristics.

By the way, since the implementation of the LBLEClient::readCharacteristic() does not even use the LBLEClient::m_characteristics list during its operation, it, in theory, does not be affected directly by this issue. However, due to the way the read request is constructed by specifying the handle range to be the full range from 0x0001 to 0xFFFF, if the LBLEClient::readCharacteristic() method is used to read characteristics affected by this issue (that is, characteristics with identical UUID but belonging to different services on the same peripheral), according to "Read Using Characteristic UUID" from Bluetooth Core Specification v4.2 Vol.3 Part G, the GATT server returns a list of Attribute Handle and Attribute Value pairs, which is not really what the LBLEClient::readCharacteristic() is designed for, and this could break the LBLEClient::readCharacteristic() wrapper methods like LBLEClient::readCharacteristicInt, etc.

@pablosun
Copy link
Contributor

pablosun commented Feb 7, 2018

We really appreciate such helpful issue report. If I understood correctly, this involves 2 major issues in the LBLE library:

  1. LBLEClient fails to enumerate many attributes, e.g. 000000190000100080000026bb765291 in your case.

  2. The current interface assumes that characteristic UUIDs and characteristic instances are 1-to-1 mapped. This assumption is convenient but wrong. Therefore the interface and implementation cannot properly handle the case that the multiple characteristics using the same UUID within a device.

For the 1st issue, we need to look further into that bug.

For the 2nd issue, you have described the problem and solution clearly. The LBLEClient has to be redesigned. It should allow enumerations of characteristics from either services or UUIDs. The result should be a collection of characteristic objects, which are in fact attribute handles in the underlying implementation.

For example, instead of calling client.readCharacteristicChar(0x2A06), one would call client.readCharacteristicChar(c) where c is an element in a collection returned from client.discoverCharacteristics(some_service).

@pablosun pablosun self-assigned this Feb 7, 2018
pablosun added a commit that referenced this issue Feb 14, 2018
 * #89: use `equal_range` when searching for elements in STL multimap, instead of using `find`. `find` is not guaranteed to return the first element in the equal range.

 * #90: Add a new set of interfaces to `LBLEClient` that allows user to identify a characteristic by using service index and characteristic index, instead of using UUID. Note that it is possible for a device to have multiple characteristics with the same UUID.

 * #90: Add a new example `EnumerateCharacteristic.ino` that uses the new indices-based interface of `LBLEClient` to list all the services and characteristics in the connected BLE device.

 * #90: Use `bt_gattc_read_charc` instead of `bt_gattc_read_using_charc_uuid` to read characteristics.

 * #91: when calling `bt_gattc_discover_charc`, we need to wait for multiple `BT_GATTC_DISCOVER_CHARC` event. We added new helper function `waitAndProcessEventMultiple` that supports such event waiting behavior.

 * Refactored `LBLEValueBuffer` to support re-interpreting its raw buffer content into String, float, int, and char types.
pablosun added a commit that referenced this issue Feb 26, 2018
## Bug Fixes

* #89: use `equal_range` when searching for elements in STL multimap, instead of using `find`. `find` is not guaranteed to return the first element in the equal range.

 * #90: Add a new set of interfaces to `LBLEClient` that allows user to identify a characteristic by using service index and characteristic index, instead of using UUID. Note that it is possible for a device to have multiple characteristics with the same UUID.

 * #90: Add a new example `EnumerateCharacteristic.ino` that uses the new indices-based interface of `LBLEClient` to list all the services and characteristics in the connected BLE device.

 * #90: Use `bt_gattc_read_charc` instead of `bt_gattc_read_using_charc_uuid` to read characteristics.

 * #91: when calling `bt_gattc_discover_charc`, we need to wait for multiple `BT_GATTC_DISCOVER_CHARC` event. We added new helper function `waitAndProcessEventMultiple` that supports such event waiting behavior.

 * Refactored `LBLEValueBuffer` to support re-interpreting its raw buffer content into String, float, int, and char types.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants