-
Notifications
You must be signed in to change notification settings - Fork 79
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
Add advertiser classes and handle adv set terminated events #366
Conversation
zxzxwu
commented
Dec 6, 2023
- Convert hci.OwnAddressType to enum
- Add LegacyAdvertiser and ExtendedAdvertiser classes
- Rename start/stop_advertising() => start/stop_legacy_advertising()
- Handle HCI_Advertising_Set_Terminated
- Properly restart advertisement on disconnection
Maybe we should merge this request after we have a way to test extended advertising? |
bumble/device.py
Outdated
|
||
adv_handle = -1 | ||
# Find a free handle | ||
for i in range( | ||
DEVICE_MIN_EXTENDED_ADVERTISING_SET_HANDLE, | ||
DEVICE_MAX_EXTENDED_ADVERTISING_SET_HANDLE + 1, | ||
): | ||
if i not in self.extended_advertising_handles: | ||
if i not in self.extended_advertisers.keys(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if i not in self.extended_advertisers
should be sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this could provide a better readability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Either way is fine, but in general I think it is more readable to use the standard/idiomatic way of using collections.
For looking up a key in a dict, the recommended way in most style guides is to just use key in dict
(see https://google.github.io/styleguide/pyguide.html#284-decision for example).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair. Modified.
* Convert hci.OwnAddressType to enum * Add LegacyAdvertiser and ExtendedAdvertiser classes * Rename start/stop_advertising() => start/stop_legacy_advertising() * Handle HCI_Advertising_Set_Terminated * Properly restart advertisement on disconnection