Hi,
following up from a previous conversation[1] and some clarifications via
IRC with Vinicius Costa Gomes, I am proposing these three new btd_*
calls which I need in order to keep the playstation-peripheral plugin
(ex sixaxis plugin) external.
The first two should be functionally OK, but maybe the implementation
can be improved.
The third one needs some more care, I added some XXX notes to the pieces
I have still some doubts on.
I used an interface based on "char *" to avoid using bluez types in an
external plugin and avoid adding calls like ba2str() and str2ba() to
src/bluetooth.ver
I checked the patches with checkpatch.pl from linux and there are a
couple of WARNING: braces {} are not necessary for single statement
blocks I can fix these in another iteration if this is required by bluez
coding guidelines too.
Thanks,
Antonio
[1] http://article.gmane.org/gmane.linux.bluez.kernel/16032
Antonio Ospite (3):
Add a btd_manager_get_default_adapter_str() call
Add a btd_store_record_string() call
Add a btd_device_set_trusted() call
src/device.h | 6 +++++
src/manager.h | 1 +
src/storage.h | 1 +
src/device.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/manager.c | 16 +++++++++++++
src/storage.c | 17 +++++++++++++
6 files changed, 111 insertions(+), 0 deletions(-)
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
On Tue, 20 Sep 2011 14:30:42 +0300
Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Antonio,
>
> On Mon, Sep 19, 2011 at 8:42 PM, Antonio Ospite
> <[email protected]> wrote:
> > Add a new btd_* call to get the default adapter address as a string,
> > this is going to be used by the external playstation-peripheral plugin.
> > ---
> > ?src/manager.h | ? ?1 +
> > ?src/manager.c | ? 16 ++++++++++++++++
> > ?2 files changed, 17 insertions(+), 0 deletions(-)
> >
> > diff --git a/src/manager.h b/src/manager.h
> > index 4f92d2f..91d9894 100644
> > --- a/src/manager.h
> > +++ b/src/manager.h
> > @@ -36,6 +36,7 @@ const char *manager_get_base_path(void);
> > ?struct btd_adapter *manager_find_adapter(const bdaddr_t *sba);
> > ?struct btd_adapter *manager_find_adapter_by_id(int id);
> > ?struct btd_adapter *manager_get_default_adapter(void);
> > +char *btd_manager_get_default_adapter_str(void);
> > ?void manager_foreach_adapter(adapter_cb func, gpointer user_data);
> > ?GSList *manager_get_adapters(void);
> > ?struct btd_adapter *btd_manager_register_adapter(int id);
> > diff --git a/src/manager.c b/src/manager.c
> > index 464b0ca..8947f85 100644
> > --- a/src/manager.c
> > +++ b/src/manager.c
> > @@ -270,6 +270,22 @@ struct btd_adapter *manager_get_default_adapter(void)
> > ? ? ? ?return manager_find_adapter_by_id(default_adapter_id);
> > ?}
> >
> > +char *btd_manager_get_default_adapter_str(void)
> > +{
> > + ? ? ? struct btd_adapter *adapter;
> > + ? ? ? bdaddr_t adapter_bdaddr;
> > + ? ? ? char *str = bt_malloc(18);
> > +
> > + ? ? ? adapter = manager_get_default_adapter();
> > + ? ? ? if (adapter == NULL) {
> > + ? ? ? ? ? ? ? return NULL;
> > + ? ? ? }
> > +
> > + ? ? ? adapter_get_address(adapter, &adapter_bdaddr);
> > + ? ? ? ba2str(&adapter_bdaddr, str);
> > + ? ? ? return str;
> > +}
> > +
>
> It doesn't seems that gonna be very useful for the core daemon,
> besides you can always get the bdaddr and convert yourself, if the
> purpose is just to be a helper function then perhaps we should find
> another place. Btw are you sure you need the adapter address and not
> the remote device address?
>
The point of the helper function here is to avoid making
adapter_get_address() and ba2str() public/global symbols for external
plugins to use. That's why it is prefixed with btd_*, unfortunately
I couldn't find a rule which explains the rationale behind the btd_
prefix, and the conditions where it should be used, for now I just
follow the rule "if you want it in an external plugin then it must be
prefixed with btd_".
I need the adapter address because of how the association works on
those PlayStation peripherals: the _adapter_ bt address needs to
be stored into the _device_ (via USB) before it can communicate with
it.
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
Hi Antonio,
On Mon, Sep 19, 2011 at 8:42 PM, Antonio Ospite
<[email protected]> wrote:
> Add a new btd_* call to get the default adapter address as a string,
> this is going to be used by the external playstation-peripheral plugin.
> ---
> ?src/manager.h | ? ?1 +
> ?src/manager.c | ? 16 ++++++++++++++++
> ?2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/src/manager.h b/src/manager.h
> index 4f92d2f..91d9894 100644
> --- a/src/manager.h
> +++ b/src/manager.h
> @@ -36,6 +36,7 @@ const char *manager_get_base_path(void);
> ?struct btd_adapter *manager_find_adapter(const bdaddr_t *sba);
> ?struct btd_adapter *manager_find_adapter_by_id(int id);
> ?struct btd_adapter *manager_get_default_adapter(void);
> +char *btd_manager_get_default_adapter_str(void);
> ?void manager_foreach_adapter(adapter_cb func, gpointer user_data);
> ?GSList *manager_get_adapters(void);
> ?struct btd_adapter *btd_manager_register_adapter(int id);
> diff --git a/src/manager.c b/src/manager.c
> index 464b0ca..8947f85 100644
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -270,6 +270,22 @@ struct btd_adapter *manager_get_default_adapter(void)
> ? ? ? ?return manager_find_adapter_by_id(default_adapter_id);
> ?}
>
> +char *btd_manager_get_default_adapter_str(void)
> +{
> + ? ? ? struct btd_adapter *adapter;
> + ? ? ? bdaddr_t adapter_bdaddr;
> + ? ? ? char *str = bt_malloc(18);
> +
> + ? ? ? adapter = manager_get_default_adapter();
> + ? ? ? if (adapter == NULL) {
> + ? ? ? ? ? ? ? return NULL;
> + ? ? ? }
> +
> + ? ? ? adapter_get_address(adapter, &adapter_bdaddr);
> + ? ? ? ba2str(&adapter_bdaddr, str);
> + ? ? ? return str;
> +}
> +
It doesn't seems that gonna be very useful for the core daemon,
besides you can always get the bdaddr and convert yourself, if the
purpose is just to be a helper function then perhaps we should find
another place. Btw are you sure you need the adapter address and not
the remote device address?
--
Luiz Augusto von Dentz
Add a new btd_* call to store an SDP record passed as a string, this is
going to be used by the external playstation-peripheral plugin.
---
src/storage.h | 1 +
src/storage.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/src/storage.h b/src/storage.h
index bb64727..c94c691 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -56,6 +56,7 @@ GSList *list_trusts(bdaddr_t *local, const char *service);
int write_device_profiles(bdaddr_t *src, bdaddr_t *dst, const char *profiles);
int delete_entry(bdaddr_t *src, const char *storage, const char *key);
int store_record(const gchar *src, const gchar *dst, sdp_record_t *rec);
+int btd_store_record_string(const gchar *src, const gchar *dst, const gchar *record_str);
sdp_record_t *record_from_string(const gchar *str);
sdp_record_t *fetch_record(const gchar *src, const gchar *dst, const uint32_t handle);
int delete_record(const gchar *src, const gchar *dst, const uint32_t handle);
diff --git a/src/storage.c b/src/storage.c
index 1f3da6e..8bf2d8d 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -816,6 +816,23 @@ int store_record(const gchar *src, const gchar *dst, sdp_record_t *rec)
return err;
}
+int btd_store_record_string(const gchar *src,
+ const gchar *dst,
+ const gchar *record_str)
+{
+ sdp_record_t *record;
+ int ret = 0;
+
+ record = record_from_string(record_str);
+ if (record == NULL) {
+ return -ENOMEM;
+ }
+ ret = store_record(src, dst, record);
+ sdp_record_free(record);
+
+ return ret;
+}
+
sdp_record_t *record_from_string(const gchar *str)
{
sdp_record_t *rec;
--
1.7.6.3
Add a new btd_* call to set a device as trusted, this is going to be
used by the external playstation-peripheral plugin.
---
src/device.h | 6 +++++
src/device.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/src/device.h b/src/device.h
index 6efcf63..f1b43b7 100644
--- a/src/device.h
+++ b/src/device.h
@@ -60,6 +60,12 @@ void device_register_services(DBusConnection *conn, struct btd_device *device,
GSList *prim_list, int psm);
GSList *device_services_from_record(struct btd_device *device,
GSList *profiles);
+int btd_device_set_trusted(const char *adapter_address,
+ const char *device_address,
+ char *name,
+ uint16_t vendor_id,
+ uint16_t product_id,
+ const char *uuid);
void btd_device_add_uuid(struct btd_device *device, const char *uuid);
struct btd_adapter *device_get_adapter(struct btd_device *device);
void device_get_address(struct btd_device *device, bdaddr_t *bdaddr);
diff --git a/src/device.c b/src/device.c
index 96e798f..67182c9 100644
--- a/src/device.c
+++ b/src/device.c
@@ -49,6 +49,7 @@
#include "att.h"
#include "hcid.h"
#include "adapter.h"
+#include "manager.h"
#include "gattrib.h"
#include "attio.h"
#include "device.h"
@@ -2481,6 +2482,75 @@ GSList *btd_device_get_primaries(struct btd_device *device)
return device->primaries;
}
+int btd_device_set_trusted(const char *adapter_address,
+ const char *device_address,
+ char *name,
+ uint16_t vendor_id,
+ uint16_t product_id,
+ const char *uuid)
+{
+ struct btd_adapter *adapter;
+ struct btd_device *device;
+ DBusConnection *conn;
+ bdaddr_t src;
+ bdaddr_t dst;
+ int ret = 0;
+
+ str2ba(adapter_address, &src);
+ str2ba(device_address, &dst);
+
+ /* XXX: is this really needed? Association works even without it */
+ write_device_name(&dst, &src, name);
+
+ /* Set the device id */
+ store_device_id(adapter_address, device_address, 0xffff, vendor_id,
+ product_id, 0);
+ /* Don't write a profile here,
+ * it will be updated when the device connects */
+
+ write_trust(adapter_address, device_address, "[all]", TRUE);
+
+ conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
+ if (conn == NULL) {
+ DBG("Failed to get on the bus");
+ ret = -EPERM;
+ goto out;
+ }
+
+ adapter = manager_find_adapter(&src);
+ if (adapter == NULL) {
+ DBG("Failed to get the adapter");
+ ret = -EPERM;
+ goto out_dbus_unref;
+ }
+
+ /* This is needed: adapter_find_device() wouldn't need a Dbus
+ * connection but it would not be enough as it only searches for
+ * already existing devices, while adapter_get_device() will create a
+ * new device if necessary.
+ */
+ device = adapter_get_device(conn, adapter, device_address);
+ if (device == NULL) {
+ DBG("Failed to get the device");
+ ret = -ENODEV;
+ goto out_dbus_unref;
+ }
+
+ btd_device_add_uuid(device, uuid);
+
+ /* XXX: these two should not be needed either, as the values are reset
+ * on the actual BT connection. We are just preparing for it now, we
+ * are still connected via USB. I just wanted confirmation on that.
+ */
+ device_set_temporary(device, FALSE);
+ device_set_name(device, name);
+
+out_dbus_unref:
+ dbus_connection_unref(conn);
+out:
+ return ret;
+}
+
void btd_device_add_uuid(struct btd_device *device, const char *uuid)
{
GSList *uuid_list;
--
1.7.6.3
Add a new btd_* call to get the default adapter address as a string,
this is going to be used by the external playstation-peripheral plugin.
---
src/manager.h | 1 +
src/manager.c | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/src/manager.h b/src/manager.h
index 4f92d2f..91d9894 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -36,6 +36,7 @@ const char *manager_get_base_path(void);
struct btd_adapter *manager_find_adapter(const bdaddr_t *sba);
struct btd_adapter *manager_find_adapter_by_id(int id);
struct btd_adapter *manager_get_default_adapter(void);
+char *btd_manager_get_default_adapter_str(void);
void manager_foreach_adapter(adapter_cb func, gpointer user_data);
GSList *manager_get_adapters(void);
struct btd_adapter *btd_manager_register_adapter(int id);
diff --git a/src/manager.c b/src/manager.c
index 464b0ca..8947f85 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -270,6 +270,22 @@ struct btd_adapter *manager_get_default_adapter(void)
return manager_find_adapter_by_id(default_adapter_id);
}
+char *btd_manager_get_default_adapter_str(void)
+{
+ struct btd_adapter *adapter;
+ bdaddr_t adapter_bdaddr;
+ char *str = bt_malloc(18);
+
+ adapter = manager_get_default_adapter();
+ if (adapter == NULL) {
+ return NULL;
+ }
+
+ adapter_get_address(adapter, &adapter_bdaddr);
+ ba2str(&adapter_bdaddr, str);
+ return str;
+}
+
static void manager_remove_adapter(struct btd_adapter *adapter)
{
uint16_t dev_id = adapter_get_dev_id(adapter);
--
1.7.6.3
On Mon, 10 Oct 2011 19:16:26 +0200
Daniele Forsi <[email protected]> wrote:
> 2011/10/10 Antonio Ospite:
>
> > +char *btd_manager_get_default_adapter_str(void)
>
> > + ? ? ? char *str = bt_malloc(18);
> > +
> > + ? ? ? adapter = manager_get_default_adapter();
> > + ? ? ? if (adapter == NULL) {
> > + ? ? ? ? ? ? ? return NULL;
>
> you are leaking str if you return here
> --
> Daniele Forsi
Thanks Daniele, I'll move the allocation down, just before using str.
Not that it is an excuse, but those patches are more to test the water
than meant for inclusion as they are :)
For the record, I originally intended to use batostr() here,
unfortunately it returns the bdaddr reversed and fixing it would break
the API.
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
2011/10/10 Antonio Ospite:
> +char *btd_manager_get_default_adapter_str(void)
> + ? ? ? char *str = bt_malloc(18);
> +
> + ? ? ? adapter = manager_get_default_adapter();
> + ? ? ? if (adapter == NULL) {
> + ? ? ? ? ? ? ? return NULL;
you are leaking str if you return here
--
Daniele Forsi
Add a new btd_* call to set a device as trusted, this is going to be
used by the external playstation-peripheral plugin.
---
src/device.h | 6 +++++
src/device.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 0 deletions(-)
diff --git a/src/device.h b/src/device.h
index 1ea5ce4..4994d2a 100644
--- a/src/device.h
+++ b/src/device.h
@@ -63,6 +63,12 @@ void device_register_services(DBusConnection *conn, struct btd_device *device,
GSList *prim_list, int psm);
GSList *device_services_from_record(struct btd_device *device,
GSList *profiles);
+int btd_device_set_trusted(const char *adapter_address,
+ const char *device_address,
+ char *name,
+ uint16_t vendor_id,
+ uint16_t product_id,
+ const char *uuid);
void btd_device_add_uuid(struct btd_device *device, const char *uuid);
struct btd_adapter *device_get_adapter(struct btd_device *device);
void device_get_address(struct btd_device *device, bdaddr_t *bdaddr);
diff --git a/src/device.c b/src/device.c
index 68d504d..2d167d3 100644
--- a/src/device.c
+++ b/src/device.c
@@ -49,6 +49,7 @@
#include "att.h"
#include "hcid.h"
#include "adapter.h"
+#include "manager.h"
#include "gattrib.h"
#include "attio.h"
#include "device.h"
@@ -2698,6 +2699,75 @@ GSList *btd_device_get_primaries(struct btd_device *device)
return device->primaries;
}
+int btd_device_set_trusted(const char *adapter_address,
+ const char *device_address,
+ char *name,
+ uint16_t vendor_id,
+ uint16_t product_id,
+ const char *uuid)
+{
+ struct btd_adapter *adapter;
+ struct btd_device *device;
+ DBusConnection *conn;
+ bdaddr_t src;
+ bdaddr_t dst;
+ int ret = 0;
+
+ str2ba(adapter_address, &src);
+ str2ba(device_address, &dst);
+
+ /* XXX: is this really needed? Association works even without it */
+ write_device_name(&dst, &src, name);
+
+ /* Set the device id */
+ store_device_id(adapter_address, device_address, 0xffff, vendor_id,
+ product_id, 0);
+ /* Don't write a profile here,
+ * it will be updated when the device connects */
+
+ write_trust(adapter_address, device_address, "[all]", TRUE);
+
+ conn = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
+ if (conn == NULL) {
+ DBG("Failed to get on the bus");
+ ret = -EPERM;
+ goto out;
+ }
+
+ adapter = manager_find_adapter(&src);
+ if (adapter == NULL) {
+ DBG("Failed to get the adapter");
+ ret = -EPERM;
+ goto out_dbus_unref;
+ }
+
+ /* This is needed: adapter_find_device() wouldn't need a Dbus
+ * connection but it would not be enough as it only searches for
+ * already existing devices, while adapter_get_device() will create a
+ * new device if necessary.
+ */
+ device = adapter_get_device(conn, adapter, device_address);
+ if (device == NULL) {
+ DBG("Failed to get the device");
+ ret = -ENODEV;
+ goto out_dbus_unref;
+ }
+
+ btd_device_add_uuid(device, uuid);
+
+ /* XXX: these two should not be needed either, as the values are reset
+ * on the actual BT connection. We are just preparing for it now, we
+ * are still connected via USB. I just wanted confirmation on that.
+ */
+ device_set_temporary(device, FALSE);
+ device_set_name(device, name);
+
+out_dbus_unref:
+ dbus_connection_unref(conn);
+out:
+ return ret;
+}
+
void btd_device_add_uuid(struct btd_device *device, const char *uuid)
{
GSList *uuid_list;
--
1.7.6.3
Add a new btd_* call to get the default adapter address as a string,
this is going to be used by the external playstation-peripheral plugin.
---
src/manager.h | 1 +
src/manager.c | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/src/manager.h b/src/manager.h
index 4f92d2f..91d9894 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -36,6 +36,7 @@ const char *manager_get_base_path(void);
struct btd_adapter *manager_find_adapter(const bdaddr_t *sba);
struct btd_adapter *manager_find_adapter_by_id(int id);
struct btd_adapter *manager_get_default_adapter(void);
+char *btd_manager_get_default_adapter_str(void);
void manager_foreach_adapter(adapter_cb func, gpointer user_data);
GSList *manager_get_adapters(void);
struct btd_adapter *btd_manager_register_adapter(int id);
diff --git a/src/manager.c b/src/manager.c
index 464b0ca..8947f85 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -270,6 +270,22 @@ struct btd_adapter *manager_get_default_adapter(void)
return manager_find_adapter_by_id(default_adapter_id);
}
+char *btd_manager_get_default_adapter_str(void)
+{
+ struct btd_adapter *adapter;
+ bdaddr_t adapter_bdaddr;
+ char *str = bt_malloc(18);
+
+ adapter = manager_get_default_adapter();
+ if (adapter == NULL) {
+ return NULL;
+ }
+
+ adapter_get_address(adapter, &adapter_bdaddr);
+ ba2str(&adapter_bdaddr, str);
+ return str;
+}
+
static void manager_remove_adapter(struct btd_adapter *adapter)
{
uint16_t dev_id = adapter_get_dev_id(adapter);
--
1.7.6.3
Add a new btd_* call to store an SDP record passed as a string, this is
going to be used by the external playstation-peripheral plugin.
---
src/storage.h | 1 +
src/storage.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/src/storage.h b/src/storage.h
index dbe717f..9b6eb47 100644
--- a/src/storage.h
+++ b/src/storage.h
@@ -55,6 +55,7 @@ int write_trust(const char *src, const char *addr, const char *service, gboolean
int write_device_profiles(bdaddr_t *src, bdaddr_t *dst, const char *profiles);
int delete_entry(bdaddr_t *src, const char *storage, const char *key);
int store_record(const gchar *src, const gchar *dst, sdp_record_t *rec);
+int btd_store_record_string(const gchar *src, const gchar *dst, const gchar *record_str);
sdp_record_t *record_from_string(const gchar *str);
sdp_record_t *fetch_record(const gchar *src, const gchar *dst, const uint32_t handle);
int delete_record(const gchar *src, const gchar *dst, const uint32_t handle);
diff --git a/src/storage.c b/src/storage.c
index c64842c..35c39bf 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -783,6 +783,23 @@ int store_record(const gchar *src, const gchar *dst, sdp_record_t *rec)
return err;
}
+int btd_store_record_string(const gchar *src,
+ const gchar *dst,
+ const gchar *record_str)
+{
+ sdp_record_t *record;
+ int ret = 0;
+
+ record = record_from_string(record_str);
+ if (record == NULL) {
+ return -ENOMEM;
+ }
+ ret = store_record(src, dst, record);
+ sdp_record_free(record);
+
+ return ret;
+}
+
sdp_record_t *record_from_string(const gchar *str)
{
sdp_record_t *rec;
--
1.7.6.3