Skip to content

Fix properties check to use Bleak Write value#40

Open
philrittenhouse wants to merge 1 commit intoadafruit:mainfrom
philrittenhouse:fix_write
Open

Fix properties check to use Bleak Write value#40
philrittenhouse wants to merge 1 commit intoadafruit:mainfrom
philrittenhouse:fix_write

Conversation

@philrittenhouse
Copy link
Copy Markdown

Re: Issue #39 Unable to Write Characteristic

I have tested this change on Windows 10

Let me know if there is anything I missed.

Thanks,
Phil

@dhalbert dhalbert self-requested a review October 8, 2021 02:38
@dhalbert
Copy link
Copy Markdown
Contributor

dhalbert commented Oct 8, 2021

After looking at this in more detail, I think the problem may be instead that this code is wrong:

for prop in _bleak_characteristic.properties:
properties |= GattCharacteristicsFlags[prop.replace("-", "_")].value

It is assuming that the bleak property bits are the same as the ones I defined. Instead, we need a mapping from each bleak bit to each other bit, or we need to change our bits to match bleak's. The bits chosen here were the same ones that nRF uses, so I think it might be better to do the mapping, rather than just change the bit values. But I need to think about that. I Your change works for that particular bit, but it might not work for the others. I will look at this again tomorrow; it's late at the moment. I'm sorry I led you astray in your initial proposed change.

@philrittenhouse
Copy link
Copy Markdown
Author

I have been doing some digging and I have found a few sources that seem to suggest the characterstic properties are defined in the spec. I don't have access to the spec to confirm but this table from Nordic suggests the values are defined in the BLE Core Spec v4.2

https://devzone.nordicsemi.com/nordic/short-range-guides/b/bluetooth-low-energy/posts/ble-characteristics-a-beginners-tutorial
image

@ukBaz
Copy link
Copy Markdown

ukBaz commented Oct 8, 2021

The Core Specification is open to anyone and is available at:

https://www.bluetooth.com/specifications/specs/core-specification/

The version has moved on to 5.3 now but this data shouldn't be changing.

I think this is the table you are referring to:

image

@markjbianchi
Copy link
Copy Markdown

I came across this same issue (and just now found this pull request), and traced it down to the Characteristic.READ/WRITE/etc bit masks not matching the BLE standard, as shown above. I think the correct fix would be to simply change the property bit mask values in Characteristic class in common.py to match that standard. I have done that locally and it works perfectly now.

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

Successfully merging this pull request may close these issues.

5 participants