Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1399666440-14078-1-git-send-email-armansito@chromium.org> <1399666440-14078-2-git-send-email-armansito@chromium.org> <01CD59B6-44C7-425A-8686-82C428F41C87@holtmann.org> Date: Thu, 15 May 2014 16:07:23 +0300 Message-ID: Subject: Re: [PATCH v1 01/10] src/shared/att: Introduce struct bt_att. From: Luiz Augusto von Dentz To: Arman Uguray Cc: Marcel Holtmann , BlueZ development Content-Type: text/plain; charset=UTF-8 List-ID: Hi Arman, On Wed, May 14, 2014 at 5:54 PM, Arman Uguray wrote: > Hi Luiz, > >>> Actually what I have in mind is even simpler than this. For outgoing >>> operations that don't solicit a response (such as commands and >>> outgoing notifications) we simply send them and invoke the callback >>> with a special opcode such as "BT_ATT_PDU_SENT". For those outgoing >>> operations that do solicit a response (e.g. requests and indications), >>> the request remains pending until the response is received. A >>> responses array is not really necessary, since there is already a 1:1 >>> mapping between ATT requests and their responses, so we know exactly >>> which request the received response matches (even the error response >>> contains the request that caused it in its PDU). >> >> I don't think we need an special opcode when the PDU is sent, the >> caller might actually not set any callback and I don't think there is >> any special action that can be triggered by having such a thing, >> specially because it is in no way an acknowledgment it just mean it >> has been dispatched it may still not reach the remote side. In other >> words if ATT does not have acknowledgment we should not invent one >> either. > > Makes sense. The idea I had was that if the caller passes NULL to > bt_att_send for the callback, then no acknowledgement would happen but > if they do, we would confirm with a special PDU, but this will no > longer be necessary if we have a separate function as you suggest > below. > >>> For incoming requests, we treat these as events as you said. To >>> respond to them, the upper layer than just needs to call bt_att_send >>> with the corresponding response opcode and parameters. >> >> I would not reuse the same function for both requests and responses, >> responses should not have any callback and custom data attached for >> the same reasons stated above, so I would have att_send_rsp for >> responses alone. > > Maybe it would make sense to have one function to send operations that > solicit a response (requests/indications), e.g. > bt_att_send_op_with_rsp and one function for those that don't (for > responses/commands/notifications) like bt_att_send_op_no_rsp. Or do > you think we should have separate functions such as bt_att_send_req, > bt_att_send_rsp, bt_att_send_not, bt_att_send_ind, etc? Well personally I would have dedicated function for each opcode, that way there is very little room for confusion i.e using bt_att_send_op_with_rsp for an opcode that does not solicit a response and vice-versa, internally you can still have them using common functions. -- Luiz Augusto von Dentz