Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1412375951-13940-1-git-send-email-armansito@chromium.org> <1412375951-13940-3-git-send-email-armansito@chromium.org> Date: Mon, 6 Oct 2014 19:01:34 +0300 Message-ID: Subject: Re: [PATCH BlueZ 2/3] shared/gatt-server: Introduce shared/gatt-server. From: Luiz Augusto von Dentz To: Arman Uguray Cc: "linux-bluetooth@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Arman, On Mon, Oct 6, 2014 at 4:15 PM, Arman Uguray wrote: > Hi Luiz, > >> I think it is better to be able to attach/detach and existing db >> object because otherwise we will need to reconstruct it on every >> connection, furthermore the db might exist anyway since we need to be >> able to register services even when we are not connected. >> > > Yeah, I did it this way for the reasons I explained in the cover > letter: I basically want to keep the layering related to ATT protocol > within bt_gatt_server, however the way gatt-db is currently structured > makes it difficult. This particularly arises from the generic > read/write handlers, where an attribute value is stored outside of the > database and fetched via a callback. > > When bt_gatt_server receives a request, say Read By Group Type, it > calls gatt_db_read_by_group_type and then calls gatt_db_read for each > attribute handle returned. If there's no value stored in the database > and the value should be obtained elsewhere, then gatt-db invokes the > registered read callback. That read callback needs to eventually make > the value known to bt_gatt_server. Yep, I realize this is a bit messed up because even though gatt_db_read can be asynchronous it does not take a callback, worse it takes a bdaddr (most likely inspired by Android HAL)? > So my initial thought was to add an extra layer of callbacks between > bt_gatt_server and make bt_gatt_server the global server database that > can handle multiple bt_att instances that can be attached/detached on > demand. I think I could go back to the way I had it before, have > bt_gatt_server accept a gatt-db in its constructor, and perhaps handle > a special function for the read handling, such as a > bt_gatt_server_set_read_value_for_handle of sorts that can get called > from the read handler and that helps bt_gatt_server construct the ATT > protocol response, but still there are problems such as how the > generic gatt-db read handler will know which bt_gatt_server instance > to call this function on (as there can be several bt_gatt_server > instances for each device that acts simultaneously as a client and a > server). The way things currently are, you can't reliably pass this > information as user_data in the read callback, i.e. there is currently > no way to say which bt_gatt_server the request originated from. If > gatt-db is internal to bt_gatt_server and there are wrappers for > functions such as gatt_db_add_characteristic in the bt_gatt_server_* > namespace, then we can hook callbacks up in a way that handles this. Id say we need to pass a callback along with a userdata, gatt_db_read_t can probably take a the callback and another user_data associated with it so when it is completed it should be called. > So maybe it's better to have a single bt_gatt_server that exists for > the lifetime of the application and for each new connection we create > a bt_att and attach it to bt_gatt_server. Or, we go back to what we > originally had in mind, i.e. one gatt-db and multiple per-connection > bt_gatt_server instances but we will have to rework gatt-db a little > for this. Note that this hasn't been a problem for the android code, > since everything is implemented in one android/gatt layer, hence > gatt-db is inherently internal to the transport handling. Not sure how having bt_gatt_server not per connection would help here, the problem seems much more related to asynchronous calls, be it gatt_db or bt_gatt_server it does not seem to change anything you would still have to create a context that tracks the originating bt_att to send the response to, Android code seem to be building the response itself which I think it is to be in someplace else. -- Luiz Augusto von Dentz