glennrub / ubluepy

Micropython Bluetooth LE API for nrf5x device series
0 stars 0 forks source link

Characteristic UUID not working as expected #1

Open rennard opened 2 years ago

rennard commented 2 years ago

Not sure if this repo is still active but I can't get the Characteristic UUID to work as expected when using wasp-os/micropython, more details here - https://github.com/daniel-thompson/wasp-os/issues/292

glennrub commented 2 years ago

Hi @rennard,

Thanks for trying out the ubluepy module. It's not that active repository due to the fact that micropython is moving towards a common BLE module. However, I have not found the time yet to do a proper implementation of it, and still use the legacy ubluepy myself as well.

Anyway, I did test your code snippet using the micropython submodule in wasp-os master (as of today) on a pca10056 development board, and i cannot see any issue with the UUID. It looks like it is advertising the correct UUID for the service, as well as the characteristic.

Which SoftDevice did you use? And which target device did you build for?

rennard commented 2 years ago

Thanks for the response and trying to reproduce, I'd figured as much but as far, as I know, PineTime has to use ubluepy. Strange that you've not been able to reproduce, I've had someone else verify this result on their PineTime as well, but it may well be something I'm misunderstanding.

I'm using the PineTime, I believe that board is a NRF52832, going off of what this page states http://developer.nordicsemi.com/nRF_Connect_SDK/doc/1.6.0/zephyr/boards/arm/pinetime_devkit0/doc/index.html.

This is the full code for the service and characteristics that are not working correctly:

from ubluepy import Service, Characteristic, UUID, Peripheral, constants

# Battery characteristic
_BATT_CHAR_UUID = UUID("a19585e9-0004-39d0-015f-b3e2b9a0c854")
_BATT_CHAR = Characteristic(
    _BATT_CHAR_UUID,
    props=Characteristic.PROP_NOTIFY | Characteristic.PROP_READ,
    attrs=Characteristic.ATTR_CCCD,
)

# Accelerometer characteristic
_ACCELEROMETER_UUID = UUID("a19585e9-0002-39d0-015f-b3e2b9a0c854")
_ACCELEROMETER_CHAR = Characteristic(
    _ACCELEROMETER_UUID,
    props=Characteristic.PROP_NOTIFY | Characteristic.PROP_READ,
    attrs=Characteristic.ATTR_CCCD,
)

# OSD service
_OSD_SERVICE_UUID = UUID("a19585e9-0001-39d0-015f-b3e2b9a0c854")
_OSD_SERVICE = Service(_OSD_SERVICE_UUID)
_OSD_SERVICE.addCharacteristic(_BATT_CHAR)
_OSD_SERVICE.addCharacteristic(_ACCELEROMETER_CHAR)
...

and here is a corresponding screenshot from nRFconnect showing the UUIDs:

Screenshot (19 Jan 2022 09_54_04)

The service UUID matches the code, but the characteristics seem to be sharing the same UUID, which appears to be random after '85e9'. I'm really not sure what's going on so any help is much appreciated.

Cheers

glennrub commented 2 years ago

Hi @rennard, i might debug this a bit more later. I managed to replicate a similar issue. It's most probably related to something around the count in sd_ble_cfg_set(BLE_COMMON_CFG_VS_UUID) which is not set in the ble_drv.c. You use 3 unique UUID's that not share the base as you change byte 10 and 11, instead of 12 and 13.

You could try to update the UUID's to make use of byte 12 and 13 as the uint16 UUID for the vendor specific UUID.

Updated code:

from ubluepy import Service, Characteristic, UUID, Peripheral, constants

# Battery characteristic
_BATT_CHAR_UUID = UUID("a1950004-85e9-39d0-015f-b3e2b9a0c854")
_BATT_CHAR = Characteristic(
    _BATT_CHAR_UUID,
    props=Characteristic.PROP_NOTIFY | Characteristic.PROP_READ,
    attrs=Characteristic.ATTR_CCCD,
)

# Accelerometer characteristic
_ACCELEROMETER_UUID = UUID("a1950002-85e9-39d0-015f-b3e2b9a0c854")
_ACCELEROMETER_CHAR = Characteristic(
    _ACCELEROMETER_UUID,
    props=Characteristic.PROP_NOTIFY | Characteristic.PROP_READ,
    attrs=Characteristic.ATTR_CCCD,
)

# OSD service
_OSD_SERVICE_UUID = UUID("a1950001-85e9-39d0-015f-b3e2b9a0c854")
_OSD_SERVICE = Service(_OSD_SERVICE_UUID)
_OSD_SERVICE.addCharacteristic(_BATT_CHAR)
_OSD_SERVICE.addCharacteristic(_ACCELEROMETER_CHAR)

periph = Peripheral()
periph.addService(_OSD_SERVICE)
periph.advertise(device_name="OSD", services=[_OSD_SERVICE])
rennard commented 2 years ago

Thanks for your response. I tried your suggested code and I'm still getting strange behaviour, as shown by my screenshot. Screenshot_20220126-163153

The UUIDs for the characteristics are completely wrong, and the name of the characteristics has changed, one of them now being called 'RX characteristic' which is the same as a characteristic on the service at the top. I'm not sure if this is something in ubluepy or wasp-os to be honest, but I've really got no idea where that UUID is coming from.

I've just tested using 16bit UUIDs, e.g. '0x85e9' and that seems to be doing what I want, but is unfortunately using the reserved UUID space, so ideally I wouldn't be doing this

glennrub commented 2 years ago

Yeah, right. I do have a hunch on what this could be. The default number of VS_UUID's in s132 is 10. Even without setting the BLE_COMMON_CFG_VS_UUID config i guess this should be more than enough. However, i'm looking at ubluepy_uuid.c i believe i've spotted the error. I'll have to take a closer look at it to see if i added some more logic into the order to declare. Anyway, ubluepy_uuid_make_new seems to call ble_drv_uuid_add_vs for all128-bit UUID, which is wrong. It should be added once as a base, and referenced for subsequent use by the idx returned when adding the base. I'll see if i can provide a fix.

rennard commented 2 years ago

Thanks for your response, unfortunately your explanation is lost on me as I'm not too familiar with C. It'd be helpful if we could get somewhere with fixing it but if not then I can always use 16bit UUIDs for both the service and the characteristic.

While I have you, may I ask how I can get the handle of a characteristic from python? I figured it'd be something like calling .getHandle() on the characteristic but I can't seem to get it to work

glennrub commented 2 years ago

I believe i'm on track on the issue now. I managed to reproduce the error you see.

Looking at the objects in python it looks correct:

Service UUID: UUID(uuid: 0x0001, VS idx: 02)
Char UUID: UUID(uuid: 0x0003, VS idx: 02)
Char UUID: UUID(uuid: 0x0002, VS idx: 02)
Service UUID: UUID(uuid: 0x0001, VS idx: 03)
Char UUID: UUID(uuid: 0x0004, VS idx: 03)
Char UUID: UUID(uuid: 0x0002, VS idx: 03)

However, looking at the input to the ble_drv.c, it looks a bit wrong:

Adding VS UUID Type: 2
Adding VS UUID Type: 2
Adding VS UUID Type: 2
Adding Char: UUID Type: 2, UUID 0003
Adding Char: UUID Type: 2, UUID 0002
Adding VS UUID Type: 3
Adding VS UUID Type: 3
Adding VS UUID Type: 3
Adding Char: UUID Type: 2, UUID 0004
Adding Char: UUID Type: 2, UUID 0002

The two last characteristics should have used UUID Type 3.

glennrub commented 2 years ago

While I have you, may I ask how I can get the handle of a characteristic from python? I figured it'd be something like calling .getHandle() on the characteristic but I can't seem to get it to work

Did you issue the getHandle() after <service>.addCharacteristic() ? Characteristics needs to be added after a service has been added to the BLE stack (SoftDevice) to get their handle in the attribute table.

rennard commented 2 years ago

Glad to hear you're making progress, sorry I can't be of any help when it comes to C!

Regarding getHandle, I have tried that yeah, no matter what I do I can't seem to find a way to access it. I can see a handle when I print the Characteristic but I can't find how to get it, and when I call getHandle I'm told the method doesn't exist.

An example of the code I'm using to try

_HEART_RATE_CHAR_UUID = UUID("0x2A37")
_HEART_RATE_CHAR = Characteristic(
    _HEART_RATE_CHAR_UUID,
    props=Characteristic.PROP_NOTIFY | Characteristic.PROP_READ,
    attrs=Characteristic.ATTR_CCCD,
)
_HEART_RATE_SERVICE.addCharacteristic(_HEART_RATE_CHAR)
_HEART_RATE_CHAR.getHandle()
glennrub commented 2 years ago

Patch for the issue related to the UUID messup: https://github.com/glennrub/micropython/commit/5b4e8e1baa959903e6db689f8b3b6f9fe597442f

I do have a different variant of the patch in mind, putting it in ubluepy and not in micropython ble_drv.c. I'm not sure what's best right now. Anyway, this solves the problem =)

glennrub commented 2 years ago

@rennard , does this look like the error you get? AttributeError: 'Characteristic' object has no attribute 'getHandle'?

I got this error in wasp-os version of micropython, but not when using micropython master. I have a feeling this might be outside of the ubluepy.

glennrub commented 2 years ago

Hehe =) Now i got it. It's not into microypthon master either. It's in a PR that will not get merge upstream: https://github.com/micropython/micropython/pull/4082

However, i'm planning to bring it into this repo, the PR's outstanding; descriptor support and central bonding. I've also added descriptor read and write yesterday. If everything works, it should be possible to compile in this repo as a USER_C_MODULE to bypass inclusion of the legacy ubluepy in micropython.

rennard commented 2 years ago

@rennard , does this look like the error you get? AttributeError: 'Characteristic' object has no attribute 'getHandle'?

Yep that's the error I was getting, sounds like you've found the issue! Thanks for your work on this, sounds like I should be able to compile this repo into ubluepy and point wasp-os to a new version of micropython?