Hi Łukasz, It sounds good to me. I just have a few questions:
On Thu, Jan 12, 2017 at 09:41:49PM +0100, Łukasz Rymanowski wrote: > Outgoing connection: > 1. *chan = ble_l2cap_connect(conn_handle, psm, mtu, > ble_l2cap_chan_ops *ops, > struct os_mbuf *sdu_rx); Currently, the struct ble_l2cap_chan type is hidden from the user. Does the application need to do anything with the channel object? Also, what is the purpose of the sdu_rx parameter? I am just wondering why the user passes it to this function rather than calling ble_l2cap_recv_ready() immediately afterwards. > 2. ops->connected(*chan) callback is called when channel has been > established. If connection has > been rejected, ops->disconnected(*chan, coc_err) is called with coc_err which > contains reason of reject If I understand correctly, there is a separate callback for each of the three events (connect, disconnect, recv). I don't think there is a problem with this, but it is not totally consistent with how host executes the GAP callback. In GAP, there is a single callback per connection. The function distinguishes the event type by inspecting the supplied struct ble_gap_event argument. I don't think GAP is necessarily done correctly; this is just what we ended up with. For connection-oriented channels, I wonder if it would better to follow the GAP convention. I'm even wondering if it would be good to indicate connection-oriented channel events in the GAP callback itself. I'm not sure what the right answer is, so I'd be interested in what others think as well. Finally, there is one more minor inconsistency. When a GAP connect attempt fails, a "connect" event is indicated with a nonzero error code. If I understand correctly, a failed CoC-connect results in the "disconnect" callback getting executed. I don't want to push consistency above all else, but I thought I should at least mention it. > Open points: > 1. As for now we assume that application is responsible for security on the > link. Works for me! Thanks, Chris
