Hi all,
This patch is the one that I mentioned that was needed to HDP for
updating the services. When a search services is performed using the
device API, the stored records will be compared with the new found and
if there are any diferences the driver will receive a notification.
I've made this optional in order to keep backwards compatibility, so no extra
changes are needed in the drivers if they don't care about the SDP changes.
Regards
When the remote SDP records that make a driver to be loaded change
(added, remove, or updated) And the driver is not going to be
removed, a notification is sent to the device driver. This
notification is optional annnd does not fail if the driver does
not implemment it.
---
src/device.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
src/device.h | 1 +
2 files changed, 178 insertions(+), 2 deletions(-)
diff --git a/src/device.c b/src/device.c
index 6af76d1..b8d5e26 100644
--- a/src/device.c
+++ b/src/device.c
@@ -99,7 +99,11 @@ struct browse_req {
GSList *match_uuids;
GSList *profiles_added;
GSList *profiles_removed;
+ GSList *profiles_updated;
sdp_list_t *records;
+ sdp_list_t *records_added;
+ sdp_list_t *records_removed;
+ sdp_list_t *records_changed;
int search_uuid;
int reconnect_attempt;
guint listener_id;
@@ -1328,6 +1332,30 @@ static void device_remove_drivers(struct btd_device *device, GSList *uuids)
sdp_list_free(records, (sdp_free_func_t) sdp_record_free);
}
+static void device_update_drivers(struct btd_device *device, GSList *profiles)
+{
+ GSList *l;
+
+ DBG("Update drivers for %s", device->path);
+
+ for (l = device->drivers; l; l = l->next) {
+ struct btd_driver_data *driver_data = l->data;
+ struct btd_device_driver *driver = driver_data->driver;
+ GSList *updated_uuids;
+
+ DBG("Updating device %s", device->path);
+
+ updated_uuids = device_match_driver(device, driver, profiles);
+
+ if (!updated_uuids)
+ continue;
+
+ DBG("No updated uuids");
+ if (driver->update)
+ driver->update(device, updated_uuids);
+ }
+}
+
static void services_changed(struct btd_device *device)
{
DBusConnection *conn = get_dbus_connection();
@@ -1353,11 +1381,50 @@ static int rec_cmp(const void *a, const void *b)
return r1->handle - r2->handle;
}
+static void check_changes(struct browse_req *req, sdp_list_t *stored_recs,
+ sdp_record_t *rec)
+{
+ sdp_list_t *prev_rec, *remove;
+ sdp_buf_t new_buf, prev_buf;
+
+ prev_rec = sdp_list_find(stored_recs, rec, rec_cmp);
+ if (!prev_rec) {
+ req->records_added = sdp_list_append(req->records_added,
+ sdp_copy_record(rec));
+ return;
+ } else if ((remove = sdp_list_find(req->records_removed, rec, rec_cmp))){
+ req->records_removed = sdp_list_remove(req->records_removed,
+ remove->data);
+ sdp_record_free(remove->data);
+ }
+
+ if (sdp_gen_record_pdu(rec, &new_buf) < 0)
+ return;
+ if (sdp_gen_record_pdu(prev_rec->data, &prev_buf) < 0) {
+ free(new_buf.data);
+ return;
+ }
+
+ if (new_buf.data_size != prev_buf.data_size)
+ goto change;
+
+ if (memcmp(new_buf.data, prev_buf.data, new_buf.data_size) == 0)
+ goto end;
+
+change:
+ req->records_changed = sdp_list_append(req->records_changed,
+ sdp_copy_record(rec));
+
+end:
+ free(new_buf.data);
+ free(prev_buf.data);
+}
+
static void update_services(struct browse_req *req, sdp_list_t *recs)
{
struct btd_device *device = req->device;
struct btd_adapter *adapter = device_get_adapter(device);
- sdp_list_t *seq;
+ sdp_list_t *seq, *stored_recs;
char srcaddr[18], dstaddr[18];
bdaddr_t src;
@@ -1365,6 +1432,8 @@ static void update_services(struct browse_req *req, sdp_list_t *recs)
ba2str(&src, srcaddr);
ba2str(&device->bdaddr, dstaddr);
+ stored_recs = read_records(&src, &req->device->bdaddr);
+
for (seq = recs; seq; seq = seq->next) {
sdp_record_t *rec = (sdp_record_t *) seq->data;
sdp_list_t *svcclass = NULL;
@@ -1418,6 +1487,8 @@ static void update_services(struct browse_req *req, sdp_list_t *recs)
continue;
}
+ check_changes(req, stored_recs, rec);
+
store_record(srcaddr, dstaddr, rec);
/* Copy record */
@@ -1439,6 +1510,8 @@ static void update_services(struct browse_req *req, sdp_list_t *recs)
sdp_list_free(svcclass, free);
}
+
+ sdp_list_free(stored_recs, (sdp_free_func_t)sdp_record_free);
}
static void store_profiles(struct btd_device *device)
@@ -1473,6 +1546,97 @@ static void create_device_reply(struct btd_device *device, struct browse_req *re
g_dbus_send_message(req->conn, reply);
}
+static void remove_record(void *a, void *b)
+{
+ sdp_record_t *rec = a;
+ struct btd_device *device = b;
+ char srcaddr[18], dstaddr[18];
+ bdaddr_t src;
+
+ adapter_get_address(device_get_adapter(device), &src);
+ ba2str(&src, srcaddr);
+ ba2str(&device->bdaddr, dstaddr);
+
+ delete_record(srcaddr, dstaddr, rec->handle);
+}
+
+static void extract_updated_uuids(struct browse_req *req, sdp_record_t *rec)
+{
+ sdp_list_t *svcclass = NULL;
+ gchar *profile_uuid;
+ GSList *l;
+
+ if (sdp_get_service_classes(rec, &svcclass) < 0)
+ return;
+
+ /* Check for empty service classes list */
+ if (svcclass == NULL) {
+ DBG("Skipping record with no service classes");
+ return;
+ }
+
+ /* Extract the first element and skip the remainning */
+ profile_uuid = bt_uuid2string(svcclass->data);
+ if (!profile_uuid) {
+ sdp_list_free(svcclass, free);
+ return;
+ }
+
+ l = g_slist_find_custom(req->profiles_added, profile_uuid,
+ (GCompareFunc) strcmp);
+ if (l)
+ goto end;
+
+ l = g_slist_find_custom(req->profiles_removed, profile_uuid,
+ (GCompareFunc) strcmp);
+ if (l)
+ goto end;
+
+ l = g_slist_find_custom(req->profiles_updated, profile_uuid,
+ (GCompareFunc) strcmp);
+ if (l)
+ goto end;
+
+ req->profiles_updated = g_slist_append(req->profiles_updated,
+ profile_uuid);
+
+end:
+ g_free(profile_uuid);
+ sdp_list_free(svcclass, free);
+}
+
+static void get_updated_uuids(struct browse_req *req)
+{
+ sdp_list_t *l;
+
+ for (l = req->records_added; l; l = l->next) {
+ extract_updated_uuids(req, l->data);
+ }
+
+ for (l = req->records_removed; l; l = l->next) {
+ extract_updated_uuids(req, l->data);
+ }
+
+ for (l = req->records_changed; l; l = l->next) {
+ extract_updated_uuids(req, l->data);
+ }
+
+ if (req->records_added)
+ sdp_list_free(req->records_added,
+ (sdp_free_func_t)sdp_record_free);
+
+ if (req->records_changed)
+ sdp_list_free(req->records_changed,
+ (sdp_free_func_t)sdp_record_free);
+
+ if (req->records_removed) {
+ sdp_list_foreach(req->records_removed, remove_record,
+ req->device);
+ sdp_list_free(req->records_removed,
+ (sdp_free_func_t)sdp_record_free);
+ }
+}
+
static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
{
struct browse_req *req = user_data;
@@ -1493,7 +1657,10 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
device->tmp_records = req->records;
req->records = NULL;
- if (!req->profiles_added && !req->profiles_removed) {
+ get_updated_uuids(req);
+
+ if (!req->profiles_added && !req->profiles_removed &&
+ !req->profiles_updated) {
DBG("%s: No service update", device->path);
goto send_reply;
}
@@ -1506,6 +1673,10 @@ static void search_cb(sdp_list_t *recs, int err, gpointer user_data)
if (req->profiles_removed)
device_remove_drivers(device, req->profiles_removed);
+ /* Notify the UUIDS whoose SDP records have changed */
+ if (req->profiles_updated)
+ device_update_drivers(device, req->profiles_updated);
+
/* Propagate services changes */
services_changed(req->device);
@@ -1574,6 +1745,7 @@ done:
static void init_browse(struct browse_req *req, gboolean reverse)
{
GSList *l;
+ bdaddr_t src;
/* If we are doing reverse-SDP don't try to detect removed profiles
* since some devices hide their service records while they are
@@ -1585,6 +1757,9 @@ static void init_browse(struct browse_req *req, gboolean reverse)
for (l = req->device->uuids; l; l = l->next)
req->profiles_removed = g_slist_append(req->profiles_removed,
l->data);
+
+ adapter_get_address(device_get_adapter(req->device), &src);
+ req->records_removed = read_records(&src, &req->device->bdaddr);
}
int device_browse(struct btd_device *device, DBusConnection *conn,
diff --git a/src/device.h b/src/device.h
index 5f75e61..72593c4 100644
--- a/src/device.h
+++ b/src/device.h
@@ -103,6 +103,7 @@ struct btd_device_driver {
const char **uuids;
int (*probe) (struct btd_device *device, GSList *uuids);
void (*remove) (struct btd_device *device);
+ void (*update) (struct btd_device *device, GSList *uuids);
};
int btd_register_device_driver(struct btd_device_driver *driver);
--
1.7.1
El Tuesday 03 August 2010 13:42:15 Luiz Augusto von Dentz escribi?:
> Hi,
>
> On Tue, Aug 3, 2010 at 12:12 PM, Jos? Antonio Santos Cadenas
>
> <[email protected]> wrote:
> > The different with a2dp is that the application will decide the end point
> > to connect to, because there can be more than one matching the source.
> > This end point is represented by a path that is passed to the
> > HealthAgent when is discovered. If the remote record changes it is
> > possible that new end points are available so the application needs
> > something to discover the available end points. That is why I think that
> > we need something like this.
>
> I guess we already suggest you guys to take a look in the
> MediaEndpoint interface, which can be found here:
>
> http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae
> 12be10522c82dea6a701688.patch
>
> It doesn't expose the remote endpoints it basically do the best
> matching, if the application requesting the connection has local
> endpoint they have priority, and then the 'agent' or in case of audio
> the endpoint will be notified with the remote endpoint capabilities.
>
> Also while looking the health API it seems you guys missed a very
> important point, HealthAgent only knows about the remote services
> since there is no signal for announcing a new HealthApplication nor
> the application knows the about HealthService, so the only way make
> this to work is that the HealthApplication and HealthAgent leaves in
> the same process but this defeats the point of having them separated.
Yes, this only works in the same process. It is a way for receiving callbacks
from the daemon.
> So either we have the applications and services being announced to
> everyone and then anyone can do the matching or we don't announce them
> at all and make bluetoothd do the matching itself, I guess the latter
> is better so we don't have to split the logic in many place (with
> potential code duplication + bugs).
At this point, I have the same opinion, Services can be notified using
signals. But Marcel thought that it will be better and will let the
programmers make less mistakes if only the services related to the application
are notified, that is why this information is send through the agent instead
of sending signals. Of course this can be changed if we think that is better.
>
> Another important thing is that dbus introspection does not have any
> way to notify changes, so it really need to be signal to notify
> applications about new services.
Hi,
El Tuesday 03 August 2010 13:42:15 Luiz Augusto von Dentz escribi?:
> Hi,
>
> On Tue, Aug 3, 2010 at 12:12 PM, Jos? Antonio Santos Cadenas
>
> <[email protected]> wrote:
> > The different with a2dp is that the application will decide the end point
> > to connect to, because there can be more than one matching the source.
> > This end point is represented by a path that is passed to the
> > HealthAgent when is discovered. If the remote record changes it is
> > possible that new end points are available so the application needs
> > something to discover the available end points. That is why I think that
> > we need something like this.
>
> I guess we already suggest you guys to take a look in the
> MediaEndpoint interface, which can be found here:
>
> http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae
> 12be10522c82dea6a701688.patch
>
> It doesn't expose the remote endpoints it basically do the best
> matching, if the application requesting the connection has local
> endpoint they have priority, and then the 'agent' or in case of audio
> the endpoint will be notified with the remote endpoint capabilities.
>
> Also while looking the health API it seems you guys missed a very
> important point, HealthAgent only knows about the remote services
> since there is no signal for announcing a new HealthApplication nor
> the application knows the about HealthService, so the only way make
> this to work is that the HealthApplication and HealthAgent leaves in
> the same process but this defeats the point of having them separated.
> So either we have the applications and services being announced to
> everyone and then anyone can do the matching or we don't announce them
> at all and make bluetoothd do the matching itself, I guess the latter
> is better so we don't have to split the logic in many place (with
> potential code duplication + bugs).
>
> Another important thing is that dbus introspection does not have any
> way to notify changes, so it really need to be signal to notify
> applications about new services.
Can we have an IRC session to talk about the API and continuing this
discussion? I think that this thread is changing too much from its original
topic.
Regards.
Hi,
On Tue, Aug 3, 2010 at 12:12 PM, Jos? Antonio Santos Cadenas
<[email protected]> wrote:
> The different with a2dp is that the application will decide the end point to
> connect to, because there can be more than one matching the source. This end
> point is represented by a path that is passed to the HealthAgent when is
> discovered. If the remote record changes it is possible that new end points
> are available so the application needs something to discover the available end
> points. That is why I think that we need something like this.
I guess we already suggest you guys to take a look in the
MediaEndpoint interface, which can be found here:
http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae12be10522c82dea6a701688.patch
It doesn't expose the remote endpoints it basically do the best
matching, if the application requesting the connection has local
endpoint they have priority, and then the 'agent' or in case of audio
the endpoint will be notified with the remote endpoint capabilities.
Also while looking the health API it seems you guys missed a very
important point, HealthAgent only knows about the remote services
since there is no signal for announcing a new HealthApplication nor
the application knows the about HealthService, so the only way make
this to work is that the HealthApplication and HealthAgent leaves in
the same process but this defeats the point of having them separated.
So either we have the applications and services being announced to
everyone and then anyone can do the matching or we don't announce them
at all and make bluetoothd do the matching itself, I guess the latter
is better so we don't have to split the logic in many place (with
potential code duplication + bugs).
Another important thing is that dbus introspection does not have any
way to notify changes, so it really need to be signal to notify
applications about new services.
--
Luiz Augusto von Dentz
Computer Engineer
Hi,
El Tuesday 03 August 2010 11:00:07 Luiz Augusto von Dentz escribi?:
> Hi,
>
> On Tue, Aug 3, 2010 at 10:30 AM, Jos? Antonio Santos Cadenas
>
> <[email protected]> wrote:
> > Hi Luiz,
> >
> > El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribi?:
> >> Hi,
> >>
> >> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
> >>
> >> <[email protected]> wrote:
> >> > When the remote SDP records that make a driver to be loaded change
> >> > (added, remove, or updated) And the driver is not going to be
> >> > removed, a notification is sent to the device driver. This
> >> > notification is optional annnd does not fail if the driver does
> >> > not implemment it.
> >>
> >> How this is supposed to work?
> >
> > This tries to be a generic way to update the remote services. When we
> > planned the HDP API some weeks ago in Recife, we decided that we need a
> > way to update the services that a device offers (this implies to make
> > some calls to the HealtAgent). Marcel and Johan tell me that it will be
> > better to implement it in a generic way using a generic update services.
> > May be I misunderstood something or this is not the best way to do this,
> > but that's the objective. The objective is to provide an API to the user
> > that notifies them about new health services.
> >
> > Can you see better ways to do this? We are open to any new ideas.
> >
> >> If does seems to me that you guys are
> >> planning to keep polling the services records to update the drivers
> >> which IMO is not a good idea, DiscoverServices is not meant to update
> >> the drivers either it is basically a replacement to sdptool to so
> >> application can do it over dbus.
> >
> > Of course I am not trying to poll remote SDP nor having it cached. Just
> > offering a way to update services for the user.
> >
> >> Also if you take a closer look in the
> >> plugins that use rfcomm channel, which can be changed at any point,
> >> they basically fetch the record when attempting to connect, avdtp also
> >> work in a similar way, we have to do the discover + get capabilities
> >> every time we want to connect to a sink/source since there is no way
> >> to keep this information updated locally.
> >
> > Of course, every time that you are going to connect is when you check the
> > remote record. In the HDP implementation is doing the same way.
> >
> >> In the other hand, this can be useful for LE which we can subscribe to
> >> changes, but for sdp I don't see the point.
> >
> > For HDP is also useful in SDP, because the device can update services and
> > the device driver will not receive any notification. This way we are
> > offering a way to update it when the user wants. This is almost the same
> > than making a service discovery and notify a new UUID discovered, but
> > also notifying the changes in the records.
>
> But you must do the discover every time you want to connect, as you
> state there is no notification for SDP, so updating the drivers
> doesn't guarantee anything. Application should not know about this,
> they basically want to connect and bluetoothd has to figure out which
> sink/source do match for them.
>
> So IMO this has to be done similar to a2dp/avdtp, prior to connect
> discover the endpoints and them proceed with the connection attempt,
> the stored records are just a hint of what the remote device supports.
The different with a2dp is that the application will decide the end point to
connect to, because there can be more than one matching the source. This end
point is represented by a path that is passed to the HealthAgent when is
discovered. If the remote record changes it is possible that new end points
are available so the application needs something to discover the available end
points. That is why I think that we need something like this.
Regards.
Hi,
On Tue, Aug 3, 2010 at 10:30 AM, Jos? Antonio Santos Cadenas
<[email protected]> wrote:
> Hi Luiz,
>
> El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribi?:
>> Hi,
>>
>> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
>>
>> <[email protected]> wrote:
>> > When the remote SDP records that make a driver to be loaded change
>> > (added, remove, or updated) And the driver is not going to be
>> > removed, a notification is sent to the device driver. This
>> > notification is optional annnd does not fail ?if the driver does
>> > not implemment it.
>>
>> How this is supposed to work?
>
> This tries to be a generic way to update the remote services. When we planned
> the HDP API some weeks ago in Recife, we decided that we need a way to update
> the services that a device offers (this implies to make some calls to the
> HealtAgent). Marcel and Johan tell me that it will be better to implement it
> in a generic way using a generic update services. May be I misunderstood
> something or this is not the best way to do this, but that's the objective.
> The objective is to provide an API to the user that notifies them about new
> health services.
>
> Can you see better ways to do this? We are open to any new ideas.
>
>
>> If does seems to me that you guys are
>> planning to keep polling the services records to update the drivers
>> which IMO is not a good idea, DiscoverServices is not meant to update
>> the drivers either it is basically a replacement to sdptool to so
>> application can do it over dbus.
>
> Of course I am not trying to poll remote SDP nor having it cached. Just
> offering a way to update services for the user.
>
>
>> Also if you take a closer look in the
>> plugins that use rfcomm channel, which can be changed at any point,
>> they basically fetch the record when attempting to connect, avdtp also
>> work in a similar way, we have to do the discover + get capabilities
>> every time we want to connect to a sink/source since there is no way
>> to keep this information updated locally.
>
> Of course, every time that you are going to connect is when you check the
> remote record. In the HDP implementation is doing the same way.
>
>>
>> In the other hand, this can be useful for LE which we can subscribe to
>> changes, but for sdp I don't see the point.
>
> For HDP is also useful in SDP, because the device can update services and the
> device driver will not receive any notification. This way we are offering a
> way to update it when the user wants. This is almost the same than making a
> service discovery and notify a new UUID discovered, but also notifying the
> changes in the records.
But you must do the discover every time you want to connect, as you
state there is no notification for SDP, so updating the drivers
doesn't guarantee anything. Application should not know about this,
they basically want to connect and bluetoothd has to figure out which
sink/source do match for them.
So IMO this has to be done similar to a2dp/avdtp, prior to connect
discover the endpoints and them proceed with the connection attempt,
the stored records are just a hint of what the remote device supports.
--
Luiz Augusto von Dentz
Computer Engineer
Hi Luiz,
El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribi?:
> Hi,
>
> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
>
> <[email protected]> wrote:
> > When the remote SDP records that make a driver to be loaded change
> > (added, remove, or updated) And the driver is not going to be
> > removed, a notification is sent to the device driver. This
> > notification is optional annnd does not fail if the driver does
> > not implemment it.
>
> How this is supposed to work?
This tries to be a generic way to update the remote services. When we planned
the HDP API some weeks ago in Recife, we decided that we need a way to update
the services that a device offers (this implies to make some calls to the
HealtAgent). Marcel and Johan tell me that it will be better to implement it
in a generic way using a generic update services. May be I misunderstood
something or this is not the best way to do this, but that's the objective.
The objective is to provide an API to the user that notifies them about new
health services.
Can you see better ways to do this? We are open to any new ideas.
> If does seems to me that you guys are
> planning to keep polling the services records to update the drivers
> which IMO is not a good idea, DiscoverServices is not meant to update
> the drivers either it is basically a replacement to sdptool to so
> application can do it over dbus.
Of course I am not trying to poll remote SDP nor having it cached. Just
offering a way to update services for the user.
> Also if you take a closer look in the
> plugins that use rfcomm channel, which can be changed at any point,
> they basically fetch the record when attempting to connect, avdtp also
> work in a similar way, we have to do the discover + get capabilities
> every time we want to connect to a sink/source since there is no way
> to keep this information updated locally.
Of course, every time that you are going to connect is when you check the
remote record. In the HDP implementation is doing the same way.
>
> In the other hand, this can be useful for LE which we can subscribe to
> changes, but for sdp I don't see the point.
For HDP is also useful in SDP, because the device can update services and the
device driver will not receive any notification. This way we are offering a
way to update it when the user wants. This is almost the same than making a
service discovery and notify a new UUID discovered, but also notifying the
changes in the records.
Regards.
Jose.
Hi,
On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
<[email protected]> wrote:
> When the remote SDP records that make a driver to be loaded change
> (added, remove, or updated) And the driver is not going to be
> removed, a notification is sent to the device driver. This
> notification is optional annnd does not fail ?if the driver does
> not implemment it.
How this is supposed to work? If does seems to me that you guys are
planning to keep polling the services records to update the drivers
which IMO is not a good idea, DiscoverServices is not meant to update
the drivers either it is basically a replacement to sdptool to so
application can do it over dbus. Also if you take a closer look in the
plugins that use rfcomm channel, which can be changed at any point,
they basically fetch the record when attempting to connect, avdtp also
work in a similar way, we have to do the discover + get capabilities
every time we want to connect to a sink/source since there is no way
to keep this information updated locally.
In the other hand, this can be useful for LE which we can subscribe to
changes, but for sdp I don't see the point.
--
Luiz Augusto von Dentz
Computer Engineer