Skip to content

feat: update default descriptor to default char permissions #810

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.

Sign up for GitHub

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

Closed
klow68 wants to merge 1 commit into from
Closed

feat: update default descriptor to default char permissions #810

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bumble/gatt_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ def add_service(self, service: Service) -> None:
# pylint: disable=line-too-long
Descriptor(
GATT_CLIENT_CHARACTERISTIC_CONFIGURATION_DESCRIPTOR,
att.Attribute.READABLE | att.Attribute.WRITEABLE,
characteristic.permissions,

@zxzxwu zxzxwu Oct 31, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many characteristics support only READ | NOTIFY but not WRITE, so if they have the permissions, CCCD could be unwritable?

Though I agree with the concern, I didn't come up with a good solution here. Maybe we can let users to determine this?

@klow68 klow68 Oct 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, maybe it could be better to do:
characteristic.permissions | att.Attribute.READABLE | att.Attribute.WRITEABLE

That will ensure cccd to be writable even if the char is only read|notify

@zxzxwu zxzxwu Nov 2, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am perservative on this change because:

  1. CCCD values are not shared between clients so they are not sensitive. The access control might be introduced for notify and indicate, instead of CCCD themselves.
  2. Core Spec doesn't regulate the behavior of descriptors and reserve it for higher profiles, but usually they are not regulated in profile specs as well.

If necessary, I tend to add a default CCCD constructor function and let callers control permissions there.

@klow68 klow68 Nov 4, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for your quick response!
Maybe I should also have explained, why I needed that for my project.
In my case, I use this permission on the cccd to trigger the pairing on the mobile app side.
At the moment I just override the permissions to get the same permissions like the ones I proposed and everything works fine. I just thought this would have been the default behavior. But I guess there is a lot of use cases I don't know.

I'm not to much into Bluetooth, so I don't understand what you mean by: "The access control might be introduced for notify and indicate, instead of CCCD themselves."

For the constructor, how the users could handle the permissions ? By passing them when adding the service ? monkey patching ? or something else ?

@zxzxwu zxzxwu Nov 4, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current GATT server only adds CCCD if notification is enabled and CCCD doesn't exist. You can check the conditions above around line #250.

So if users want to pass their own CCCD, just add them when constructing characteristics, like

Characteristic(
    'D901B45B-4916-412E-ACCA-376ECB603B2C',
    Characteristic.Properties.READ | Characteristic.Properties.NOTIFY,
    Characteristic.READABLE | Characteristic.WRITEABLE,
    descriptors=[
        Descriptor(
            GATT_CLIENT_CHARACTERISTIC_CONFIGURATION_DESCRIPTOR, ...
        )
    ],
),

I'm not to much into Bluetooth, so I don't understand what you mean by: "The access control might be introduced for notify and indicate, instead of CCCD themselves."

You can check notify_subscriber() to see how notification is implemented.

bumble/bumble/gatt_server.py

Line 333 in d112901

async def notify_subscriber(

In short, CCCD is a flag to enable and disable subscriptions, and servers can decide whether to send notifications based on it, and other factors like permissions. The current implementation won't send notifications if value is not present by callers, and we can add a checker to block notifications if they don't have enough permissions to Characteristics themselves, but CCCD permission is another thing.

@klow68 klow68 Nov 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we can't add the descriptor with the lambda function to update the characteristic value when we define the characteristic, because the gatt server is created on runtime.
But yeah that's what I do (adding the descriptor, with permissions), and just before adding the service, I override the characteristicto have the callbacks on the characteristic value for the CCCD.

Adding the permissions by default from the characteristic sounded easier but if it doesn't work for all cases, you are right it's fine the way it is.

Thx a lot for the explanation and you're time!
I think we can close then this topic :)

@zxzxwu zxzxwu Nov 6, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's really not easy to compose a CCCD value callback, later we may add a helper to construct it.

CharacteristicValue(
read=lambda connection, characteristic=characteristic: self.read_cccd(
connection, characteristic
Expand Down
Loading