Return-Path: MIME-Version: 1.0 Sender: armansito@google.com In-Reply-To: <780A1AF5-538F-4239-A4E2-86B0BB2EDDB0@holtmann.org> References: <1399351563-25991-1-git-send-email-armansito@chromium.org> <780A1AF5-538F-4239-A4E2-86B0BB2EDDB0@holtmann.org> Date: Tue, 6 May 2014 23:53:19 -0700 Message-ID: Subject: Re: [PATCH 00/10] bt_att initial implementation From: Arman Uguray To: Marcel Holtmann Cc: Luiz Augusto von Dentz , "linux-bluetooth@vger.kernel.org" , Marcin Kraglak Content-Type: text/plain; charset=UTF-8 List-ID: Hi Luiz, >> Apparently you have ignored my comments regarding the API, it is >> probably not the best thing to base everything on mgmt API since that >> is a internal protocol which does not need encoding as everything is >> passed in host order, also unlike mgmt you have GATT on top o ATT so >> it is beneficial to use iovec in the API. Sorry, I must have missed those comments in my inbox somewhere, so I didn't mean to ignore them. That said, as Marcel also pointed out, bt_att is only meant for ATT protocol transactions. It doesn't do any state keeping apart from some internal queues for ATT request control-flow, but that's pretty much it. The intended use is to create a bt_att around a file descriptor, use bt_att_send with the opcode and the relevant parameters to send requests/commands, bt_att_reply to send replies to incoming requests/indications, and some other function to subscribe to events. In that regard, the structure of src/shared/mgmt is remarkably suitable for bt_att. The encoding of the PDU, including the necessary endianness conversion, is handled by bt_att. That last bit of course is different from mgmt, where the parameters structure definition itself encodes the data being sent over the socket, whereas bt_att performs encoding/decoding logic. Either way, I think this definition makes for a nice API. >> Regarding unit tests, I think you should take a look at test-av*, and >> there exists a test specification for ATT/GATT so we should start with >> it since is what PTS uses for qualification: >> https://www.bluetooth.org/docman/handlers/downloaddoc.ashx?doc_id=229871 >> >> You can use the name and description of the unit test following the >> test specification: >> >> /TP/GAC/CL/BV-01-C: Verify that a Generic Attribute Profile client can >> generate an Exchange MTU Request command to configure ATT_MTU over >> LE. > > This is still relevant and perhaps it should involve testing gatt-db > as well since the testing spec also involves the server, which leads > to the question if we should keep them apart? I guess this should be > done in a single place since we will be controlling the fd, or the > idea is that bt_att should access gatt_db internally to handle > incoming requests? This is a very good point. Currently the idea is to have a bt_gatt which defines GATT client-side functions for primary service discovery, read/write requests etc, which would then internally use bt_att for ATT protocol transactions, and gatt_db which solely serves as a medium for storage of local/remote attributes. Basically, a complete implementation of a GATT client/server would create and manage individual instances of bt_gatt/bt_att for all requests and gatt_db to explicitly store the results of those requests. In essence, the database implementation and bt_gatt/bt_att won't need to explicitly depend on each other. This is why it might make sense to hold off on writing PTS style tests until we have the complete picture in place. For now, I went with a simple set of unit tests to make sure that basic PDU encoding/decoding is done correctly for bt_att. As we then start putting gatt_db and bt_gatt together, I say we then write tests in the fashion of test-av*. Cheers, Arman