-
Notifications
You must be signed in to change notification settings - Fork 93
add passkey delegate support for pairing #708
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
Conversation
df90a5a
to
d631156
Compare
bumble/smp.py
Outdated
try: | ||
utils.cancel_on_event( | ||
self.connection, | ||
'disconnection', |
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.
'disconnection', | |
self.connection.EVENT_DISCONNECTION, |
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 ended up creating a help function for the common call: utils.cancel_on_event(self.connection, 'disconnection', foo()
--> self.connection.cancel_on_disconnection(foo())
bumble/smp.py
Outdated
|
||
# Send our public key back to the initiator | ||
self.send_public_key_command() | ||
def next_steps(): |
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.
To enforce type checking
def next_steps(): | |
def next_steps() -> None: |
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.
done
) | ||
except Exception as error: | ||
logger.warning(f'exception while displaying number: {error}') | ||
await self.pairing_config.delegate.display_number(self.passkey, digits=6) |
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: This will become blocking, but it looks intended. Maybe we should have a better documentation about which methods are and should be blocking.
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.
all the delegate methods are made async so that they can block if needed. I'm planning on doing a complete doc of the pairing delegate, where this will be explained in details.
""" | ||
|
||
# By default, generate a random passkey. | ||
return secrets.randbelow(1000000) |
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.
Unrelated: Currently, both PairingConfig and PairingDelegate don't have information about the pairing session. I think we may need to add such information like a connection attribute in the future.
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.
Actually, the pairing configs can use the connection context if they need it. The PairingConfig
and PairingDelegate
base classes don't store it, because they don't need it, but pairing_config_factory
takes a connection
arg, so if your config or delegate needs the context, it can keep it.
Ex:
device.pairing_config_factory = lambda connection: MyPairingConfig(connection)
As discussed in #705
Passkey generation has a default implementation in
PairingDelegate
which can be overridden by subclasses.