Hi Szymon,
On Wed, Feb 12, 2014 at 12:59 PM, Szymon Janc <szymon.janc@xxxxxxxxx> wrote:
> Hi,
>
> On Wednesday 12 of February 2014 11:58:16 Luiz Augusto von Dentz wrote:
>> Hi Anderson,
>>
>> On Tue, Feb 11, 2014 at 8:32 PM, Anderson Lizardo
>>
>> <anderson.lizardo@xxxxxxxxxxxxx> wrote:
>> > These UUIDs are assigned by BT-SIG and therefore there is no need to
>> > use full 128-bit UUIDs. This also avoids unnecessary conversion from
>> > string representation.
>> > ---
>> >
>> > android/handsfree.c | 3 +--
>> > android/hidhost.c | 7 +++----
>> > 2 files changed, 4 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/android/handsfree.c b/android/handsfree.c
>> > index 9482b2e..a869d58 100644
>> > --- a/android/handsfree.c
>> > +++ b/android/handsfree.c
>> > @@ -34,7 +34,6 @@
>> >
>> > #include "lib/bluetooth.h"
>> > #include "lib/sdp.h"
>> > #include "lib/sdp_lib.h"
>> >
>> > -#include "lib/uuid.h"
>> >
>> > #include "src/sdp-client.h"
>> > #include "src/uuid-helper.h"
>> > #include "src/shared/hfp.h"
>> >
>> > @@ -294,7 +293,7 @@ static void handle_connect(const void *buf, uint16_t
>> > len)>
>> > device_init(&bdaddr);
>> >
>> > - bt_string2uuid(&uuid, HFP_HS_UUID);
>> > + sdp_uuid16_create(&uuid, HANDSFREE_SVCLASS_ID);
>> >
>> > if (bt_search_service(&adapter_addr, &device.bdaddr, &uuid,
>> >
>> > sdp_search_cb, NULL, NULL, 0) < 0)
>> > {
>> >
>> > error("handsfree: SDP search failed");
>> >
>> > diff --git a/android/hidhost.c b/android/hidhost.c
>> > index 8bd3455..45fe14f 100644
>> > --- a/android/hidhost.c
>> > +++ b/android/hidhost.c
>> > @@ -37,7 +37,6 @@
>> >
>> > #include "lib/bluetooth.h"
>> > #include "lib/sdp.h"
>> > #include "lib/sdp_lib.h"
>> >
>> > -#include "lib/uuid.h"
>> >
>> > #include "src/shared/mgmt.h"
>> > #include "src/sdp-client.h"
>> > #include "src/uuid-helper.h"
>> >
>> > @@ -751,7 +750,7 @@ static void hid_sdp_did_search_cb(sdp_list_t *recs,
>> > int err, gpointer data)>
>> > dev->version = data->val.uint16;
>> >
>> > }
>> >
>> > - bt_string2uuid(&uuid, HID_UUID);
>> > + sdp_uuid16_create(&uuid, HID_SVCLASS_ID);
>> >
>> > if (bt_search_service(&adapter_addr, &dev->dst, &uuid,
>> >
>> > hid_sdp_search_cb, dev, NULL, 0) < 0) {
>> >
>> > error("failed to search sdp details");
>> >
>> > @@ -792,7 +791,7 @@ static void bt_hid_connect(const void *buf, uint16_t
>> > len)>
>> > ba2str(&dev->dst, addr);
>> > DBG("connecting to %s", addr);
>> >
>> > - bt_string2uuid(&uuid, PNP_UUID);
>> > + sdp_uuid16_create(&uuid, PNP_INFO_SVCLASS_ID);
>> >
>> > if (bt_search_service(&adapter_addr, &dev->dst, &uuid,
>> >
>> > hid_sdp_did_search_cb, dev, NULL,
>> > 0) < 0) {
>> >
>> > error("Failed to search DeviceID SDP details");
>> >
>> > @@ -1293,7 +1292,7 @@ static void connect_cb(GIOChannel *chan, GError
>> > *err, gpointer user_data)>
>> > dev->ctrl_io = g_io_channel_ref(chan);
>> > dev->uhid_fd = -1;
>> >
>> > - bt_string2uuid(&uuid, PNP_UUID);
>> > + sdp_uuid16_create(&uuid, PNP_INFO_SVCLASS_ID);
>> >
>> > if (bt_search_service(&src, &dev->dst, &uuid,
>> >
>> > hid_sdp_did_search_cb, dev, NULL,
>> > 0) < 0) {
>> >
>> > error("failed to search did sdp details");
>> >
>> > --
>> > 1.7.9.5
>>
>> Applied all except 6/7, I think we should probably return an error if
>> there is an attempt to register a service out of range, then the
>> caller can assert so we can have a proper test for it that expect an
>> error in such case.
>
> Well, currently IPC depends on hal-msg.h which defines max allowed service ID
> and using something bigger is a code bug. We could have make IPC independent
> of hal-msg.h and just verify registered ID in runtime but that would add extra
> code for each caller with no profit as IDs are fixed anyway.
Perhaps we should ignore and not register service is initialized with
e.g. -1 just skip ipc_register.
> That test fix is invalid as we actually want to test if IPC handles out-of-
> bound service ID correctly (when receiving register command).
> And I'm not sure why this actually was causing any problems since that test is
> not registering handlers for out-of-bound service ID, just sends ipc message
> with such ID.
Im not sure we have the array of HAL_SERVICE_ID_MAX + 1 I thought that
would be HAL_SERVICE_ID_MAX, nevertheless every test seems to be
calling test_cmd_reg which does call ipc_register which IMO should be
invalid for HAL_SERVICE_ID_MAX + 1.
> (BTW I think we should pass max service id on ipc_init, as currently audio ipc
> is using max service id from bt ipc)
Im did not get the comment above.
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
