-
Notifications
You must be signed in to change notification settings - Fork 106
[Pandora] Implement L2CAP service #320
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
| self.up_queue.put_nowait(pdu) | ||
|
|
||
|
|
||
| class CocChannelProxy(ChannelProxy): |
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.
suggestion: rename to DynamicChannelProxy
|
|
||
| @channel.once('close') | ||
| def on_close() -> None: | ||
| self.disconnection_result.set_result(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.
| self.disconnection_result.set_result(None) | |
| self.channel = None | |
| self.disconnection_result.set_result(None) |
| self.channel.write(data) | ||
|
|
||
| @property | ||
| def closed(self): |
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.
Replace by ?
@property
def closed(self):
return self.channel is None| self, channel: Union[l2cap.ClassicChannel, l2cap.LeCreditBasedChannel] | ||
| ) -> None: | ||
| super().__init__() | ||
| self.channel = channel |
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.
| self.channel = channel | |
| self.channel: Union[l2cap.ClassicChannel, l2cap.LeCreditBasedChannel, None] = channel |
bumble/pandora/l2cap.py
Outdated
|
|
||
| connection = self.device.lookup_connection(connection_handle) | ||
| if connection is None: | ||
| raise RuntimeError('Connection not exist') |
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.
Isnt the connection field optional ?
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 thought you don't want to support this feature?
google/bt-test-interfaces#8 (comment)
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 apologize for this because I can only admit that I was wrong. The reason I forgot about at first was: For timing issue we need to be able listen for incoming L2CAP channel connection before ACL's connect. I'm wondering if we need to change the APIs, WDYT ? @DeltaEvo ? Also, we need to remove the fixed field from the ConnectRquest
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.
You may mark it optional or totally remove it.
bumble/pandora/l2cap.py
Outdated
| channel_proxy = FixedChannelProxy( | ||
| connection_handle=connection_handle, cid=cid, device=self.device | ||
| ) | ||
| self.channels[connection_handle][cid] = channel_proxy |
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.
Missing remove from the map later
bumble/pandora/l2cap.py
Outdated
| handle = channel.connection.handle | ||
| # Filter channels from non-specified connection | ||
| if handle != connection_handle: | ||
| connection.abort_on('disconnect', channel.disconnect()) |
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.
It should be possible to have multiple OnConnection in parallel, we should not disconnect when filtering
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.
Well, that's the problem of this interface. Since Bumble L2CAP server is based on CID instead of connection-cid pair, there must be no more than 1 server for the same cid. The same restriction also exists on Android.
uael
left a comment
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.
First batch done!
Thanks you and well done Josh 💯
|
I did some experiments here! github.com/google/bumble/ |
Avatar: google/avatar#57