* RE: [RFC] Bluetooth: Add firmware load infrastructure for BT devices
From: Shanmugamkamatchi Balashanmugam @ 2010-07-12 6:31 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Perelet, Oleg, linux-bluetooth@vger.kernel.org
In-Reply-To: <1278596572.10421.25.camel@localhost.localdomain>
Hi Marcel,
-----Original Message-----
From: Marcel Holtmann [mailto:marcel@holtmann.org]=20
Sent: Thursday, July 08, 2010 7:13 PM
To: Shanmugamkamatchi Balashanmugam
Cc: Perelet, Oleg; linux-bluetooth@vger.kernel.org
Subject: RE: [RFC] Bluetooth: Add firmware load infrastructure for BT devic=
es
Hi Bala,
> Firmware loading to target RAM needs to be done once when the device is i=
nserted. Firmware loading will not be required every time=20
> the device goes from DOWN to UP. I think each HCI driver requires
> a separate firmware loading code as firmware loading is=20
> different for different interfaces. Please advice if my understanding is=
wrong.
>=20
> I initially thought of registration=20
> mechanism with btusb transport driver to load firmware, but before
> the device is inserted btusb will not be loaded and registering the=20
> firmware load function with btusb was not possible.
>=20
> Please advice alternate solution to load firmware from transport driver.
>my advise would be to just build devices that change their USB VID/PID
>after the firmware got loaded. That way it is easy to have a firmware
>loading driver and just btusb for real operation. Why is it so hard to
>build just simple hardware that would just work.
>Regards
>Marcel
Thanks for the suggestion.
This is what is done for some of the devices.
For performance reasons few other devices comes with=20
small firmware in flash and the device gets detected as=20
generic bluetooth device when plugged in. So control reaches btusb
once the device is plugged in. In this case actual firmware
needs to be downloaded to target from btusb transport driver.
Please suggest if there is other way to handle these devices in linux.
Regards,
Bala.
^ permalink raw reply
* [PATCH 1/2] Fix signal strength for HFP in maemo6 telephony
From: Dmitriy Paliy @ 2010-07-11 21:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1278686657-6997-1-git-send-email-dmitriy.paliy@nokia.com>
Signal strength in terms of bars units is updated by emited
SignalBarsChanged instead of SignalStrengthChanged from libscnet library.
It contains required number of bars to display in scale 0..5. Conversion
from percent or decibel units to bars is not needed and therefore removed.
Same holds when initializing by request of SignalBars, where bars are in
0..5 range. Conversion from percents is removed. Sanity check is done to
prevent -1 that may be returned when the property is unavailabe (modem
state off).
RSSI percents and dBs are neither present among SignalBarsChanged
properties nor requested during initialization and, therefore, removed.
---
audio/telephony-maemo6.c | 20 +++++++++-----------
1 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index 31704c3..d5499ed 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -1233,33 +1233,31 @@ static void update_signal_strength(int32_t signals_bar)
if (signals_bar < 0)
signals_bar = 0;
- else if (signals_bar > 100) {
+ else if (signals_bar > 5) {
DBG("signals_bar greater than expected: %u", signals_bar);
- signals_bar = 100;
+ signals_bar = 5;
}
if (net.signals_bar == signals_bar)
return;
- /* A simple conversion from 0-100 to 0-5 (used by HFP) */
- signal = (signals_bar + 20) / 21;
+ /* no need in any conversion */
+ signal = signals_bar;
telephony_update_indicator(maemo_indicators, "signal", signal);
net.signals_bar = signals_bar;
-
- DBG("telephony-maemo6: signal strength updated: %u/100, %d/5", signals_bar, signal);
+ DBG("telephony-maemo6: signal strength updated: %d/5", signal);
}
static void handle_signal_strength_changed(DBusMessage *msg)
{
- int32_t signals_bar, rssi_in_dbm;
+ int32_t signals_bar;
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_INT32, &signals_bar,
- DBUS_TYPE_INT32, &rssi_in_dbm,
DBUS_TYPE_INVALID)) {
- error("Unexpected parameters in SignalStrengthChanged");
+ error("Unexpected parameters in SignalBarsChanged");
return;
}
@@ -1530,7 +1528,7 @@ static void get_property_reply(DBusPendingCall *call, void *user_data)
dbus_message_iter_get_basic(&sub, &name);
update_operator_name(name);
} else if (g_strcmp0(prop, "SignalBars") == 0) {
- uint32_t signal_bars;
+ int32_t signal_bars; /* signal bars can be -1 */
dbus_message_iter_get_basic(&sub, &signal_bars);
update_signal_strength(signal_bars);
@@ -1899,7 +1897,7 @@ static DBusHandlerResult signal_filter(DBusConnection *conn,
"OperatorNameChanged"))
handle_operator_name_changed(msg);
else if (dbus_message_is_signal(msg, CSD_CSNET_SIGNAL,
- "SignalStrengthChanged"))
+ "SignalBarsChanged"))
handle_signal_strength_changed(msg);
else if (dbus_message_is_signal(msg, "org.freedesktop.Hal.Device",
"PropertyModified"))
--
1.7.0.4
>From 03a4cdc5c55fb0a49c01bde6a59e990ec67fdfe0 Mon Sep 17 00:00:00 2001
From: Dmitriy Paliy <dmitriy.paliy@nokia.com>
Date: Sun, 11 Jul 2010 23:29:39 +0300
Subject: [PATCH 2/2] Code consitency for signal strength in HFP maemo6
This patch simplifies and makes maemo6 telephony driver code consistent
with libscnet API regarding SignalBarsChanged. RSSI percents and RSSI
dBs are not among the parameters when SignalBarsChanged emited.
Therefore, these parameters are removed. Names are changed to reflect
libscnet d-bus interface notations. Comments and debug information are
added in places where units or operations are unclear.
---
audio/telephony-maemo6.c | 46 +++++++++++++++++++++++-----------------------
1 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index d5499ed..0e2a3d4 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -134,11 +134,14 @@ struct csd_call {
static struct {
char *operator_name;
uint8_t status;
- int32_t signals_bar;
+ int32_t signal_bars;
} net = {
.operator_name = NULL,
.status = NETWORK_REG_STATUS_UNKOWN,
- .signals_bar = 0,
+ /* Init as 0 meaning inactive mode. In modem power off state */
+ /* can be be -1, but we treat all values as 0s regardless */
+ /* inactive or power off. */
+ .signal_bars = 0,
};
static int get_property(const char *iface, const char *prop);
@@ -181,6 +184,7 @@ static guint create_request_timer = 0;
static struct indicator maemo_indicators[] =
{
{ "battchg", "0-5", 5, TRUE },
+ /* signal strength in terms of bars */
{ "signal", "0-5", 0, TRUE },
{ "service", "0,1", 0, TRUE },
{ "call", "0,1", 0, TRUE },
@@ -1227,41 +1231,37 @@ static void handle_registration_changed(DBusMessage *msg)
update_registration_status(status);
}
-static void update_signal_strength(int32_t signals_bar)
+static void update_signal_strength(int32_t signal_strength_bars)
{
- int signal;
-
- if (signals_bar < 0)
- signals_bar = 0;
- else if (signals_bar > 5) {
- DBG("signals_bar greater than expected: %u", signals_bar);
- signals_bar = 5;
+ if (signal_strength_bars < 0) {
+ DBG("signal strength smaller than expected: %d<0", signal_strength_bars);
+ signal_strength_bars = 0;
+ } else if (signal_strength_bars > 5) {
+ DBG("signal strength greater than expected: %d>5", signal_strength_bars);
+ signal_strength_bars = 5;
}
- if (net.signals_bar == signals_bar)
+ if (net.signal_bars == signal_strength_bars)
return;
- /* no need in any conversion */
- signal = signals_bar;
-
- telephony_update_indicator(maemo_indicators, "signal", signal);
+ telephony_update_indicator(maemo_indicators, "signal", signal_strength_bars);
- net.signals_bar = signals_bar;
- DBG("telephony-maemo6: signal strength updated: %d/5", signal);
+ net.signal_bars = signal_strength_bars;
+ DBG("telephony-maemo6: signal strength updated: %d/5", signal_strength_bars);
}
-static void handle_signal_strength_changed(DBusMessage *msg)
+static void handle_signal_bars_changed(DBusMessage *msg)
{
- int32_t signals_bar;
+ int32_t signal_bars;
if (!dbus_message_get_args(msg, NULL,
- DBUS_TYPE_INT32, &signals_bar,
+ DBUS_TYPE_INT32, &signal_bars,
DBUS_TYPE_INVALID)) {
error("Unexpected parameters in SignalBarsChanged");
return;
}
- update_signal_strength(signals_bar);
+ update_signal_strength(signal_bars);
}
static gboolean iter_get_basic_args(DBusMessageIter *iter,
@@ -1528,7 +1528,7 @@ static void get_property_reply(DBusPendingCall *call, void *user_data)
dbus_message_iter_get_basic(&sub, &name);
update_operator_name(name);
} else if (g_strcmp0(prop, "SignalBars") == 0) {
- int32_t signal_bars; /* signal bars can be -1 */
+ int32_t signal_bars;
dbus_message_iter_get_basic(&sub, &signal_bars);
update_signal_strength(signal_bars);
@@ -1898,7 +1898,7 @@ static DBusHandlerResult signal_filter(DBusConnection *conn,
handle_operator_name_changed(msg);
else if (dbus_message_is_signal(msg, CSD_CSNET_SIGNAL,
"SignalBarsChanged"))
- handle_signal_strength_changed(msg);
+ handle_signal_bars_changed(msg);
else if (dbus_message_is_signal(msg, "org.freedesktop.Hal.Device",
"PropertyModified"))
handle_hal_property_modified(msg);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH] Set mode options for l2cap sockets in BTIO API
From: Johan Hedberg @ 2010-07-11 15:22 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1278713330-11140-1-git-send-email-sancane@gmail.com>
Hi,
On Sat, Jul 10, 2010, Santiago Carot-Nemesio wrote:
> Some profiles like MCAP or HDP need set an specific l2cap configuraton
> such as Streming, ERTM or another else. This patch enables BTIO API to
> set the mode for l2cap socket options.
> ---
> src/btio.c | 19 +++++++++++++------
> src/btio.h | 1 +
> 2 files changed, 14 insertions(+), 6 deletions(-)
Thanks for the patch! It has now been pushed upstream.
johan
^ permalink raw reply
* Re: [PATCH] Fixed empty 'N:' parameter handling for VCARDs
From: Johan Hedberg @ 2010-07-11 15:14 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Radoslaw Jablonski, linux-bluetooth
In-Reply-To: <1278698276.10421.145.camel@localhost.localdomain>
Hi Marcel,
On Fri, Jul 09, 2010, Marcel Holtmann wrote:
> > Bluetooth PBAP specification expects for call history listing,
> > that parameter N: shall be empty when we cannot retrieve personal data
> > from PSE phone book.
> > Some devices(by example BH-903) are having problems when after N: parameter
> > unnecessary characters occurs. List of dialed/incoming calls on carkit
> > then is useless - carkit shows only blank lines and it's impossible to
> > determine who made call.
>
> please be a bit consistent with your spaces here.
>
> > In previous version unnecessary semicolons were added after N:("N:;;;;")
> > to represent empty name.
> > Now if none of the contact fields is available, then adding real empty "N:"
> > parameter (without semicolons).
> > ---
> > plugins/vcard.c | 35 +++++++++++++++++++++++++++++++++++
> > 1 files changed, 35 insertions(+), 0 deletions(-)
> >
> > diff --git a/plugins/vcard.c b/plugins/vcard.c
> > index 5948a4a..108f6bd 100644
> > --- a/plugins/vcard.c
> > +++ b/plugins/vcard.c
> > @@ -133,9 +133,44 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
> > vcard_printf(vcards, "VERSION:2.1");
> > }
> >
> > +/* checks if there is at least one present contact field with personal data */
>
> "check if there is at least one contact field with personal data
> present"
>
> > +static gboolean contact_fields_present(struct phonebook_contact * contact)
> > +{
> > + if (contact->family && strlen(contact->family) > 0)
> > + return TRUE;
> > +
> > + if (contact->given && strlen(contact->given) > 0)
> > + return TRUE;
> > +
> > + if (contact->additional && strlen(contact->additional) > 0)
> > + return TRUE;
> > +
> > + if (contact->prefix && strlen(contact->prefix) > 0)
> > + return TRUE;
> > +
> > + if (contact->suffix && strlen(contact->suffix) > 0)
> > + return TRUE;
> > +
> > + /* none of the personal data fields is present*/
>
> "none of the personal data fields are present"
>
> > + return FALSE;
> > +}
> > +
> > static void vcard_printf_name(GString *vcards,
> > struct phonebook_contact *contact)
> > {
> > + if (contact_fields_present(contact) == FALSE) {
> > + /*
> > + * If fields are empty, then add only 'N:' as parameter.
> > + * This is crucial for some devices ( Nokia BH-903) which
>
> Please be consistent with spaces.
>
> > + * have problems with history listings - can't determine
> > + * if parameter is really empty if there are unnecessary
> > + * characters after'N:' (like 'N:;;;;').
> > + * We need to add only'N:' param - without semicolons.
>
> Same here.
I went ahead and fixed these issues myself so we can get another obexd
release out. The fixed patch is now upstream.
Johan
^ permalink raw reply
* [PATCH] Set mode options for l2cap sockets in BTIO API
From: Santiago Carot-Nemesio @ 2010-07-09 22:08 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
Some profiles like MCAP or HDP need set an specific l2cap configuraton
such as Streming, ERTM or another else. This patch enables BTIO API to
set the mode for l2cap socket options.
---
src/btio.c | 19 +++++++++++++------
src/btio.h | 1 +
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/src/btio.c b/src/btio.c
index cf91de7..93079ae 100644
--- a/src/btio.c
+++ b/src/btio.c
@@ -57,6 +57,7 @@ struct set_opts {
uint16_t imtu;
uint16_t omtu;
int master;
+ uint8_t mode;
};
struct connect {
@@ -474,10 +475,10 @@ static gboolean get_sec_level(int sock, BtIOType type, int *level,
return TRUE;
}
-static gboolean l2cap_set(int sock, int sec_level, uint16_t imtu,
- uint16_t omtu, int master, GError **err)
+static gboolean l2cap_set(int sock, int sec_level, uint16_t imtu, uint16_t omtu,
+ uint8_t mode, int master, GError **err)
{
- if (imtu || omtu) {
+ if (imtu || omtu || mode) {
struct l2cap_options l2o;
socklen_t len;
@@ -493,6 +494,8 @@ static gboolean l2cap_set(int sock, int sec_level, uint16_t imtu,
l2o.imtu = imtu;
if (omtu)
l2o.omtu = omtu;
+ if (mode)
+ l2o.mode = mode;
if (setsockopt(sock, SOL_L2CAP, L2CAP_OPTIONS, &l2o,
sizeof(l2o)) < 0) {
@@ -629,6 +632,7 @@ static gboolean parse_set_opts(struct set_opts *opts, GError **err,
opts->defer = DEFAULT_DEFER_TIMEOUT;
opts->master = -1;
opts->sec_level = BT_IO_SEC_MEDIUM;
+ opts->mode = L2CAP_MODE_BASIC;
while (opt != BT_IO_OPT_INVALID) {
switch (opt) {
@@ -678,6 +682,9 @@ static gboolean parse_set_opts(struct set_opts *opts, GError **err,
case BT_IO_OPT_MASTER:
opts->master = va_arg(args, gboolean);
break;
+ case BT_IO_OPT_MODE:
+ opts->mode = va_arg(args, int);
+ break;
default:
g_set_error(err, BT_IO_ERROR, BT_IO_ERROR_INVALID_ARGS,
"Unknown option %d", opt);
@@ -1088,7 +1095,7 @@ gboolean bt_io_set(GIOChannel *io, BtIOType type, GError **err,
case BT_IO_L2RAW:
case BT_IO_L2CAP:
return l2cap_set(sock, opts.sec_level, opts.imtu, opts.omtu,
- opts.master, err);
+ opts.mode, opts.master, err);
case BT_IO_RFCOMM:
return rfcomm_set(sock, opts.sec_level, opts.master, err);
case BT_IO_SCO:
@@ -1129,7 +1136,7 @@ static GIOChannel *create_io(BtIOType type, gboolean server,
if (l2cap_bind(sock, &opts->src,
server ? opts->psm : 0, err) < 0)
goto failed;
- if (!l2cap_set(sock, opts->sec_level, 0, 0, -1, err))
+ if (!l2cap_set(sock, opts->sec_level, 0, 0, 0, -1, err))
goto failed;
break;
case BT_IO_L2CAP:
@@ -1142,7 +1149,7 @@ static GIOChannel *create_io(BtIOType type, gboolean server,
server ? opts->psm : 0, err) < 0)
goto failed;
if (!l2cap_set(sock, opts->sec_level, opts->imtu, opts->omtu,
- opts->master, err))
+ opts->mode, opts->master, err))
goto failed;
break;
case BT_IO_RFCOMM:
diff --git a/src/btio.h b/src/btio.h
index fa6ff69..81fda8e 100644
--- a/src/btio.h
+++ b/src/btio.h
@@ -60,6 +60,7 @@ typedef enum {
BT_IO_OPT_MASTER,
BT_IO_OPT_HANDLE,
BT_IO_OPT_CLASS,
+ BT_IO_OPT_MODE,
} BtIOOption;
typedef enum {
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Remove 'sport' field from l2cap_pinfo
From: Marcel Holtmann @ 2010-07-09 20:46 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth, Gustavo F. Padovan
In-Reply-To: <1278701753-25682-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> 'sport' is pointless, is always has the value of l2cap_pinfo.psm, so we
> can just use the psm field instead of this one.
>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
> include/net/bluetooth/l2cap.h | 2 --
> net/bluetooth/l2cap.c | 8 +++-----
> 2 files changed, 3 insertions(+), 7 deletions(-)
NAK here since this part is needed to differentiate server and client
connections.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] Bluetooth: Keep code under column 80
From: Marcel Holtmann @ 2010-07-09 20:46 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth, Gustavo F. Padovan
In-Reply-To: <1278704315-28192-2-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> Purely a cosmetic change, it doesn't change the code flow.
>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
> net/bluetooth/l2cap.c | 48 ++++++++++++++++++++++++++----------------------
> 1 files changed, 26 insertions(+), 22 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: Fix bug in kzalloc allocation size
From: Marcel Holtmann @ 2010-07-09 20:46 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth, Gustavo F. Padovan
In-Reply-To: <1278704315-28192-1-git-send-email-gustavo@padovan.org>
Hi Gustavo,
> Probably a typo error. We were using the wrong struct to get size.
>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
> net/bluetooth/l2cap.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* [PATCH 2/2] Bluetooth: Keep code under column 80
From: Gustavo F. Padovan @ 2010-07-09 19:38 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo F. Padovan
In-Reply-To: <1278704315-28192-1-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
Purely a cosmetic change, it doesn't change the code flow.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 48 ++++++++++++++++++++++++++----------------------
1 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 419e2c3..449cbdd 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -522,31 +522,35 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
}
if (sk->sk_state == BT_CONNECT) {
- if (l2cap_check_security(sk) &&
- __l2cap_no_conn_pending(sk)) {
- struct l2cap_conn_req req;
+ struct l2cap_conn_req req;
- if (!l2cap_mode_supported(l2cap_pi(sk)->mode,
- conn->feat_mask)
- && l2cap_pi(sk)->conf_state &
- L2CAP_CONF_STATE2_DEVICE) {
- tmp1 = kzalloc(sizeof(struct sock_del_list),
- GFP_ATOMIC);
- tmp1->sk = sk;
- list_add_tail(&tmp1->list, &del.list);
- bh_unlock_sock(sk);
- continue;
- }
+ if (!l2cap_check_security(sk) ||
+ !__l2cap_no_conn_pending(sk)) {
+ bh_unlock_sock(sk);
+ continue;
+ }
- req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
- req.psm = l2cap_pi(sk)->psm;
+ if (!l2cap_mode_supported(l2cap_pi(sk)->mode,
+ conn->feat_mask)
+ && l2cap_pi(sk)->conf_state &
+ L2CAP_CONF_STATE2_DEVICE) {
+ tmp1 = kzalloc(sizeof(struct sock_del_list),
+ GFP_ATOMIC);
+ tmp1->sk = sk;
+ list_add_tail(&tmp1->list, &del.list);
+ bh_unlock_sock(sk);
+ continue;
+ }
- l2cap_pi(sk)->ident = l2cap_get_ident(conn);
- l2cap_pi(sk)->conf_state |= L2CAP_CONF_CONNECT_PEND;
+ req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
+ req.psm = l2cap_pi(sk)->psm;
+
+ l2cap_pi(sk)->ident = l2cap_get_ident(conn);
+ l2cap_pi(sk)->conf_state |= L2CAP_CONF_CONNECT_PEND;
+
+ l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
+ L2CAP_CONN_REQ, sizeof(req), &req);
- l2cap_send_cmd(conn, l2cap_pi(sk)->ident,
- L2CAP_CONN_REQ, sizeof(req), &req);
- }
} else if (sk->sk_state == BT_CONNECT2) {
struct l2cap_conn_rsp rsp;
char buf[128];
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] Bluetooth: Fix bug in kzalloc allocation size
From: Gustavo F. Padovan @ 2010-07-09 19:38 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo F. Padovan
From: Gustavo F. Padovan <padovan@profusion.mobi>
Probably a typo error. We were using the wrong struct to get size.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e366be0..419e2c3 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -530,7 +530,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
conn->feat_mask)
&& l2cap_pi(sk)->conf_state &
L2CAP_CONF_STATE2_DEVICE) {
- tmp1 = kzalloc(sizeof(struct srej_list),
+ tmp1 = kzalloc(sizeof(struct sock_del_list),
GFP_ATOMIC);
tmp1->sk = sk;
list_add_tail(&tmp1->list, &del.list);
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Add missing HCIUARTGETDEVICE ioctl to compat_ioctl.c
From: Marcel Holtmann @ 2010-07-09 19:31 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1278703716-9674-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
> fs/compat_ioctl.c | 5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* [PATCH] Bluetooth: Add missing HCIUARTGETDEVICE ioctl to compat_ioctl.c
From: johan.hedberg @ 2010-07-09 19:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Johan Hedberg
From: Johan Hedberg <johan.hedberg@nokia.com>
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
fs/compat_ioctl.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/compat_ioctl.c b/fs/compat_ioctl.c
index 1863896..547452d 100644
--- a/fs/compat_ioctl.c
+++ b/fs/compat_ioctl.c
@@ -601,8 +601,9 @@ static int ioc_settimeout(unsigned int fd, unsigned int cmd,
}
/* Bluetooth ioctls */
-#define HCIUARTSETPROTO _IOW('U', 200, int)
-#define HCIUARTGETPROTO _IOR('U', 201, int)
+#define HCIUARTSETPROTO _IOW('U', 200, int)
+#define HCIUARTGETPROTO _IOR('U', 201, int)
+#define HCIUARTGETDEVICE _IOR('U', 202, int)
#define BNEPCONNADD _IOW('B', 200, int)
#define BNEPCONNDEL _IOW('B', 201, int)
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Bluetooth: Remove 'sport' field from l2cap_pinfo
From: Gustavo F. Padovan @ 2010-07-09 18:55 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
From: Gustavo F. Padovan <padovan@profusion.mobi>
'sport' is pointless, is always has the value of l2cap_pinfo.psm, so we
can just use the psm field instead of this one.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
include/net/bluetooth/l2cap.h | 2 --
net/bluetooth/l2cap.c | 8 +++-----
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 7df70e4..6e4a3a6 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -356,8 +356,6 @@ struct l2cap_pinfo {
__u16 remote_mps;
__u16 mps;
- __le16 sport;
-
struct timer_list retrans_timer;
struct timer_list monitor_timer;
struct timer_list ack_timer;
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e366be0..9ca3042 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -727,7 +727,7 @@ static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
struct sock *sk;
struct hlist_node *node;
sk_for_each(sk, node, &l2cap_sk_list.head)
- if (l2cap_pi(sk)->sport == psm && !bacmp(&bt_sk(sk)->src, src))
+ if (l2cap_pi(sk)->psm == psm && !bacmp(&bt_sk(sk)->src, src))
goto found;
sk = NULL;
found:
@@ -1016,7 +1016,6 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
/* Save source address */
bacpy(&bt_sk(sk)->src, &la.l2_bdaddr);
l2cap_pi(sk)->psm = la.l2_psm;
- l2cap_pi(sk)->sport = la.l2_psm;
sk->sk_state = BT_BOUND;
if (__le16_to_cpu(la.l2_psm) == 0x0001 ||
@@ -1239,7 +1238,6 @@ static int l2cap_sock_listen(struct socket *sock, int backlog)
for (psm = 0x1001; psm < 0x1100; psm += 2)
if (!__l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
l2cap_pi(sk)->psm = cpu_to_le16(psm);
- l2cap_pi(sk)->sport = cpu_to_le16(psm);
err = 0;
break;
}
@@ -1325,12 +1323,12 @@ static int l2cap_sock_getname(struct socket *sock, struct sockaddr *addr, int *l
addr->sa_family = AF_BLUETOOTH;
*len = sizeof(struct sockaddr_l2);
+ la->l2_psm = l2cap_pi(sk)->psm;
+
if (peer) {
- la->l2_psm = l2cap_pi(sk)->psm;
bacpy(&la->l2_bdaddr, &bt_sk(sk)->dst);
la->l2_cid = cpu_to_le16(l2cap_pi(sk)->dcid);
} else {
- la->l2_psm = l2cap_pi(sk)->sport;
bacpy(&la->l2_bdaddr, &bt_sk(sk)->src);
la->l2_cid = cpu_to_le16(l2cap_pi(sk)->scid);
}
--
1.7.1
^ permalink raw reply related
* Re: Proposed API for HDP (v4)
From: José Antonio Santos Cadenas @ 2010-07-09 18:39 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <201007081912.31407.santoscadenas@gmail.com>
BlueZ D-Bus Health API description
**********************************
Santiago Carot-Nemesio <sancane@gmail.com>
José Antonio Santos-Cadenas <santoscadenas@gmail.com>
Elvis Pfützenreuter <epx@signove.com>
Health Device Profile hierarchy
===============================
Service org.bluez
Interface org.bluez.HealthManager
Object path [variable prefix]/
Methods:
path RegisterApplication(object agent, dict config)
Returns the path of the new registered application. The agent
parameter is the path of the object with the callbacks to
notify events (see org.bluez.HealthAgent at the end of this
document)
This petition starts an mcap instance on every adapter and also
register a proper record in the SDP if is needed.
Dict is defined as bellow:
{
"end_points" : [{ (optional)
"role" : ("source" or "sink"), (mandatory)
"specs" :[{ (mandatory)
"data_type" : uint16, (mandatory)
"description" : string, (optional)
}]
}]
}
Application will be closed by the call or implicitly when the
programs leaves the bus.
Possible errors: org.bluez.Error.InvalidArguments
void UnregisterApplication(object application)
Closes the HDP application identified by the object path. Also
application will be closed if the process that started it leaves
the bus. If there is a SDP record associated to this application
it will also be removed.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
void UpdateServices()
This method searches for HDP applications on the all remote
devices and notifies them to the appropriate agents.
Signals:
void ServiceDiscovered(object service)
This signal is sent when a device containing an HDP
application is paired or when the method Update of the
HealthManager is called and new HealthServices are discovered.
The object path is the HealthService path. The signal will be
launched once for each HealthService.
void ServiceRemoved(object service)
This signal is sent if during an Update some HealthServices
have disappeared. The signal is launched once for each removed
HealthService.
--------------------------------------------------------------------------------
Service org.bluez
Interface org.bluez.HealthService
Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
Methods:
array GetProperties()
Returns all properties for the interface. See the properties
section for available properties.
boolean Echo(array{byte})
Sends an echo petition to the remote service. Returns True if
response matches with the buffer sent. If some error is detected
False value is returned and the associated MCL is closed.
string OpenDataChannel(byte mdepid, string conf)
Creates a new data channel with the indicated config to the
remote MCAP Data End Point (MDEP).
The configuration should indicate the channel quality of
service using one of this values "Reliable", "Streaming", "Any".
Returns an string that identifies the data channel.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
void ReconnectDataChannel(string data_channel)
Reconnects a previously created data channel.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
org.bluez.Error.NotFound
void DeleteDataChannel(string data_channel)
Deletes a data channel so it will not be available.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
org.bluez.Error.HealthError
void DeleteAllDataChannels()
Deletes all data channels so they will not be available for
future use.
Possible errors: org.bluez.Error.HealthError
Properties:
array EndPoints
An array with all the end points in this Service. Each one of
then has this format.
{
"mdepid": uint8,
"role" : "source" or "sink" ,
"specs" : [{
"dtype" : uint16,
"description" : string, (optional)
}]
}
HealthAgent hierarchy
=====================
(this object is implemented by the HDP user in order to receive notifications)
Service unique name
Interface org.bluez.HealthAgent
Object path freely definable
Methods:
void Release()
This method gets called when the service daemon unregisters the
agent. An agent can use it to do cleanup tasks. There is no need
to unregister the agent, because when this method gets called it
has already been unregistered.
void DataChannelCreated(object service, string data_channel,
string conf, fd file_descriptor, boolean reconnection)
This method is called when a new data channel is created.
The service parameter contains the object path of the
HealthService that created the connection, data_channel is
the string that identifies the data channel, conf is the quality
of service of the data channel ("reliable" or "streaming"),
file_descriptor the file descriptor for reading and writing
data and reconnection indicates if it is a reconnection or a
data channel creation.
void DataChannelRemoved(object service, string data_channel)
This method is called when a data channel is deleted.
After this call the data channel path will not be valid and can
be reused for future creation of data channels.
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Alan Ott @ 2010-07-09 18:24 UTC (permalink / raw)
To: Marcel Holtmann
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <1278696815.10421.137.camel@localhost.localdomain>
On 07/09/2010 01:33 PM, Marcel Holtmann wrote:
>>>>>>> I looked at this and I am bit worried that this should not be done in
>>>>>>> this detail in the HIDP driver. Essentially HIDP is a pure transport
>>>>>>> driver. It should not handle all these details. Can we make this a bit
>>>>>>> easier for the transport drivers to support such features?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> I put these changes (most notably the addition of hidp_get_raw_report())
>>>>>> in hidp because that's where the parallel function
>>>>>> hidp_output_raw_report() was already located. I figured the input should
>>>>>> go with the output. That said, if there's a better place for both of
>>>>>> them (input and output) to go, let me know where you think it should be,
>>>>>> and I'll get them moved into the proper spot.
>>>>>>
>>>>>> I'm not sure what you mean about HIDP being a pure transport driver.
>>>>>>
>>>>>>
>>>>>>
>>>>> what is usb-hid.ko doing here? I would expect a bunch of code
>>>>> duplication with minor difference between USB and Bluetooth.
>>>>>
>>>>>
>>>> usbhid doesn't have a lot of code for hidraw. Two functions are involved:
>>>> usbhid_output_raw_report()
>>>> - calls usb_control_msg() with Get_Report
>>>> usbhid_get_raw_report()
>>>> - calls usb_control_msg() with Set_Report
>>>> OR
>>>> - calls usb_interrupt_msg() on the Ouput pipe.
>>>>
>>>> This is of course easier than bluetooth because usb_control_msg() is
>>>> synchronous, even when requesting reports, mostly because of the nature
>>>> of USB, where the request and response are part of the same transfer.
>>>>
>>>> For Bluetooth, it's a bit more complicated since the kernel treats it
>>>> more like a networking interface (and indeed it is). My understanding is
>>>> that to make a synchronous transfer in bluetooth, one must:
>>>> - send the request packet
>>>> - block (wait_event_*())
>>>> - when the response is received in the input handler, wake_up_*().
>>>>
>>>> There's not really any code duplication, mostly because initiating
>>>> synchronous USB transfers (input and output) is easy (because of the
>>>> usb_*_msg() functions), while making synchronous Bluetooth transfers
>>>> must be done manually. If there's a nice, convenient, synchronous
>>>> function in Bluetooth similar to usb_control_msg() that I've missed,
>>>> then let me know, as it would simplify this whole thing.
>>>>
>>>>
>>> there is not and I don't think we ever get one. My question here was
>>> more in the direction why HID core is doing these synchronously in the
>>> first place. Especially since USB can do everything async as well.
>>>
>> I'm open to suggestions. The way I see it is from a user space
>> perspective. With Get_Feature being on an ioctl(), I don't see any clean
>> way to do it other than synchronously. Other operating systems (I can
>> say for sure Windows, Mac OS X, and FreeBSD) handle Get/Set Feature the
>> same way (synchronously) from user space.
>>
>> You seem to be proposing an asynchronous interface. What would that look
>> like from user space?
>>
> not necessarily from user space, but at least from HID core to HIDP and
> usb-hid transports. At least that is what I would expect, Jiri?
>
> Regards
>
> Marcel
>
Hi Marcel,
So it sounds like you're mostly concerned about the sleeping (blocking),
and where is the _right_ place for it to occur. It seems like it could
either occur in hid/hidraw.c or in hidp/core.c. If it were to occur in
hid/hidraw.c, what then would get passed back and forth between the
bluetooth/hidp and hidraw?
Maybe something like the following:
hidraw:
- get_report() (hypothetical)
- calls a hypothetical hidp_initiate_get_report(), which:
- sends the report request and returns immediately.
- wait for response
hidp:
- whenever a report is returned, it calls back to hidraw,
which wakes up the get_report() thread if
the data matches the report being waited on.
For this to work, we'd need 2 more function pointers in struct hid_device:
1. a way for hidp to call back into hidraw.
2. a pointer for hidp_initiate_get_report().
These of course would be in addition to the ones that USB already uses
(like hid_get_raw_report()), and would cause USB and Bluetooth to use
different APIs to each transport.
Of course, there could be commonality if we used the asynchronous USB
APIs like you suggested, although, I'm not sure I see the benefit of
making the USB part more complicated. The USB part (hid/usb/hid-core.c)
is currently _very_ simple.
It seems like we have two options:
1. Move to asynchronous APIs in USB and Bluetooth. This involves:
a. Move to asynchronous APIs in hid/usbhid/hid-core.c
b. Adding support into hid/hidraw.c to do the waiting.
c. Changing bluetooth/hidp to be asynchronous in nature.
2. Keep using synchronous USB APIs.
a. hid/usbhid/hid-core remains really simple
b. hid/hidraw.c remains really simple
c. bluetooth/hidp has some complexity
I'd argue that the complexity of bluetooth/hidp isn't really that
complex, and further, it's mostly isolated to one (new) function (that's
where the wait_event_*() is).
Further, if we did option #2, some piece of code has to determine
whether to wake up the blocking thread (which would then be in
hid/hidraw.c). This piece of code would be notified for every packet
received from Bluetooth to decide whether it should wake up the sleeping
thread, and would have to have bluetooth-specific code in it (something
like the block which calls wake_up_interruptible() in my patch). It
seems like this code would _have_ to be in hidp.
From a design standpoint, I can't see how it makes sense to push this
code into hid/hidraw.c when it is bluetooth-specific. Further, I can't
see how it makes sense to do the USB portion the hard way, when the
current implementation is so compact and non-error-prone.
Clients to hidraw provide two functions with very simple interfaces, one
for outputting reports, and one for getting (requesting and receiving)
reports. I think having clean interfaces between modules has a lot of value.
All that said, I'm always open to better ideas. Maybe you have a better
design idea that you can enlighten me with.
Alan.
^ permalink raw reply
* Re: Proposed API for HDP (v3)
From: José Antonio Santos Cadenas @ 2010-07-09 18:13 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20100709173618.GA21080@vigoh>
Again a new version with some modifications
BlueZ D-Bus Health API description
**********************************
Santiago Carot-Nemesio <sancane@gmail.com>
José Antonio Santos-Cadenas <santoscadenas@gmail.com>
Elvis Pfützenreuter <epx@signove.com>
Health Device Profile hierarchy
===============================
Service org.bluez
Interface org.bluez.HealthManager
Object path [variable prefix]/
Methods:
path RegisterApplication(object path, dict config)
Returns the path of the new registered application. The path
parameter is the path of the object with the callbacks to
notify events (see org.bluez.HealthAgent at the end of this
document)
This petition starts an mcap instance on every adapter and also
register a proper record in the SDP if is needed.
Dict is defined as bellow:
{
"end_points" : [{ (optional)
"role" : ("Source" or "Sink"), (mandatory)
"specs" :[{ (mandatory)
"data_type" : uint16, (mandatory)
"description" : string, (optional)
}]
}]
}
Application will be closed by the call or implicitly when the
programs leaves the bus.
Possible errors: org.bluez.Error.InvalidArguments
void UnregisterApplication(path application)
Closes the HDP application identified by the object path. Also
application will be closed if the process that started it leaves
the bus. If there is a SDP record associated to this application
it will also be removed.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
void UpdateServices()
This method searches for HDP applications on the all remote
devices and notifies them to the appropriate agents.
Signals:
void ServiceDiscovered(object path)
This signal is sent when a device containing an HDP
application is paired or when the method Update of the
HealthManager is called and new HealthServices are discovered.
The object path is the HealthService path. The signal will be
launched once for each HealthService.
void ServiceRemoved(object path)
This signal is sent if during an Update some HealthServices
have disappeared. The signal is launched once for each removed
HealthService.
--------------------------------------------------------------------------------
Service org.bluez
Interface org.bluez.HealthService
Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
Methods:
array GetProperties()
Returns all properties for the interface. See the properties
section for available properties.
boolean Echo(array{byte})
Sends an echo petition to the remote service. Returns True if
response matches with the buffer sent. If some error is detected
False value is returned and the associated MCL is closed.
path OpenDataChannel(byte mdepid, string conf)
Creates a new data channel with the indicated config to the
remote MCAP Data End Point (MDEP).
The configuration should indicate the channel quality of
service using one of this values "Reliable", "Streaming", "Any".
Returns the data channel path.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
void ReconnectDataChannel(path data_channel)
Reconnects a previously created data channel indicated by its
path.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.HealthError
org.bluez.Error.NotFound
void DeleteDataChannel(path data_channel)
Deletes a data channel so it will not be available.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
org.bluez.Error.HealthError
void DeleteAllDataChannels()
Deletes all data channels so they will not be available for
future use.
Possible errors: org.bluez.Error.HealthError
Properties:
array EndPoints
An array with all the end points in this Service. Each one of
then has this format.
{
"mdepid": uint8,
"role" : "Source" or "Sink" ,
"specs" : [{
"dtype" : uint16,
"description" : string, (optional)
}]
}
HealthAgent hierarchy
=====================
(this object is implemented by the HDP user in order to receive notifications)
Service unique name
Interface org.bluez.HealthAgent
Object path freely definable
Methods:
void DataChannelCreated(object path, path data_channel, string conf,
fd file_descriptor)
This method is called when a new data channel is created.
The path contains the object path of the HealthService that
created the connection, the data_channel is the path that
identifies the data channel, conf is the quality of service of
the data channel ("reliable" or "streaming") and file_descriptor
the file descriptor for reading and writing data.
void DataChannelReconnected(object path, path data_channel, string conf,
fd file_descriptor))
This method is called when a closed data channel is reconnected
by the remote device.
The path contains the object path of the HealthService that
reconnected the channel. data_channel is the path that
identifies the data channel, conf is the quality of service of
the data channel ("reliable" or "streaming") and file_descriptor
is the file descriptor for reading and writing data.
void DataChannelRemoved(object path, path data_channel)
This method is called when a data channel is deleted.
After this call the data channel path will not be valid and can
be reused for future creation of data channels. The path is the
path of the HealthService that removes the data channel.
^ permalink raw reply
* Re: [PATCH] Fixed empty 'N:' parameter handling for VCARDs
From: Marcel Holtmann @ 2010-07-09 17:57 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1278686070-7647-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi Radek,
> Bluetooth PBAP specification expects for call history listing,
> that parameter N: shall be empty when we cannot retrieve personal data
> from PSE phone book.
> Some devices(by example BH-903) are having problems when after N: parameter
> unnecessary characters occurs. List of dialed/incoming calls on carkit
> then is useless - carkit shows only blank lines and it's impossible to
> determine who made call.
please be a bit consistent with your spaces here.
> In previous version unnecessary semicolons were added after N:("N:;;;;")
> to represent empty name.
> Now if none of the contact fields is available, then adding real empty "N:"
> parameter (without semicolons).
> ---
> plugins/vcard.c | 35 +++++++++++++++++++++++++++++++++++
> 1 files changed, 35 insertions(+), 0 deletions(-)
>
> diff --git a/plugins/vcard.c b/plugins/vcard.c
> index 5948a4a..108f6bd 100644
> --- a/plugins/vcard.c
> +++ b/plugins/vcard.c
> @@ -133,9 +133,44 @@ static void vcard_printf_begin(GString *vcards, uint8_t format)
> vcard_printf(vcards, "VERSION:2.1");
> }
>
> +/* checks if there is at least one present contact field with personal data */
"check if there is at least one contact field with personal data
present"
> +static gboolean contact_fields_present(struct phonebook_contact * contact)
> +{
> + if (contact->family && strlen(contact->family) > 0)
> + return TRUE;
> +
> + if (contact->given && strlen(contact->given) > 0)
> + return TRUE;
> +
> + if (contact->additional && strlen(contact->additional) > 0)
> + return TRUE;
> +
> + if (contact->prefix && strlen(contact->prefix) > 0)
> + return TRUE;
> +
> + if (contact->suffix && strlen(contact->suffix) > 0)
> + return TRUE;
> +
> + /* none of the personal data fields is present*/
"none of the personal data fields are present"
> + return FALSE;
> +}
> +
> static void vcard_printf_name(GString *vcards,
> struct phonebook_contact *contact)
> {
> + if (contact_fields_present(contact) == FALSE) {
> + /*
> + * If fields are empty, then add only 'N:' as parameter.
> + * This is crucial for some devices ( Nokia BH-903) which
Please be consistent with spaces.
> + * have problems with history listings - can't determine
> + * if parameter is really empty if there are unnecessary
> + * characters after'N:' (like 'N:;;;;').
> + * We need to add only'N:' param - without semicolons.
Same here.
> + */
> + vcard_printf(vcards, "N:");
> + return;
> + }
> +
> vcard_printf(vcards, "N:%s;%s;%s;%s;%s", contact->family,
> contact->given, contact->additional,
> contact->prefix, contact->suffix);
Regards
Marcel
^ permalink raw reply
* Re: Proposed API for HDP
From: Gustavo F. Padovan @ 2010-07-09 17:36 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201007091912.44559.santoscadenas@gmail.com>
* José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-09 19:12:44 +0200]:
> El Friday 09 July 2010 18:55:08 Gustavo F. Padovan escribió:
> > Hi José,
> >
> > * José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-09 15:49:42
> +0200]:
> > > I wrote a new API based on the changes suggested in this thread.
> > >
> > > Regards.
> > >
> > > Jose.
> > >
> > >
> > > BlueZ D-Bus Health API description
> > > **********************************
> > >
> > > Santiago Carot-Nemesio <sancane@gmail.com>
> > > José Antonio Santos-Cadenas <santoscadenas@gmail.com>
> > > Elvis Pfützenreuter <epx@signove.com>
> > >
> > > Health Device Profile hierarchy
> > > ===============================
> > >
> > > Service org.bluez
> > > Interface org.bluez.HealthManager
> > > Object path [variable prefix]/{hci0,hci1,...}
> > >
> > > Methods:
> > > path RegisterApplication(object path, dict config)
> > >
> > > Returns the path of the new registered application. The path
> > > parameter is the path of the object with the callbacks to
> > > notify events (see org.bluez.HealthAgent at the end of this
> > > document)
> > > This petition starts an mcap instance on every adapter and also
> > > register a proper record in the SDP if is needed.
> > >
> > > Dict is defined as bellow:
> > > {
> > >
> > > "end_points" : [{ (optional)
> > >
> > > "role" : ("source" or "sink"), (mandatory)
> > > "specs" :[{ (mandatory)
> > >
> > > "data_type" : uint16, (mandatory)
> > > "description" : string, (optional)
> > >
> > > }]
> > >
> > > }]
> > >
> > > }
> > >
> > > Application will be closed by the call or implicitly when the
> > > programs leaves the bus.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > void UnregisterApplication(path application)
> > >
> > > Closes the HDP application identified by the object path. Also
> > > application will be closed if the process that started it leaves
> > > the bus. If there is a SDP record associated to this application
> > > it will also be removed.
> > >
> > > Possible errors: org.bluez.Error.InvalidArguments
> > >
> > > org.bluez.Error.NotFound
> > >
> > > void UpdateServices()
> > >
> > > This method searches for HDP applications on the all remote
> > > devices and notifies them to the appropriate agents.
> > >
> > > -------------------------------------------------------------------------
> > > -------
> > >
> > > Service org.bluez
> > > Interface org.bluez.HealthService
> > > Object path [variable
> > > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> > >
> > > Methods:
> > > array GetProperties()
> > >
> > > Gets the information of the remote application published on its
> > > SDP record. The returned data format is as follows:
> > >
> > > {
> > >
> > > "end_points": [
> > >
> > > "mdepid": uint8,
> > > "role" : "source" or "sink" ,
> > > "specs" : [{
> > >
> > > "dtype" : uint16,
> > > "description" : string, (optional)
> > > }]
> > >
> > > ]
> > >
> > > }
> >
> > What's the name of this property? See the others APIs, properties need a
> > name.
>
> I didn't think about properties here (I'm not used to them) But here yes, a
> property fixes better than a method call.
The method should remain, look the the others APIs. They a have a
Properties section and GetProperties() show all of them.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH 1/3] Support for adding UUID128 to extended inquiry response
From: Marcel Holtmann @ 2010-07-09 17:35 UTC (permalink / raw)
To: Ron Shaffer; +Cc: Inga Stotland, linux-bluetooth, johan.hedberg
In-Reply-To: <4C375658.10904@codeaurora.org>
Hi Ron,
> >> void register_public_browse_group(void)
> >> diff --git a/src/sdpd.h b/src/sdpd.h
> >> index e93b0b6..8fd8bbe 100644
> >> --- a/src/sdpd.h
> >> +++ b/src/sdpd.h
> >> @@ -6,6 +6,7 @@
> >> * Copyright (C) 2002-2003 Maxim Krasnyansky <maxk@qualcomm.com>
> >> * Copyright (C) 2002-2010 Marcel Holtmann <marcel@holtmann.org>
> >> * Copyright (C) 2002-2003 Stephen Crane <steve.crane@rococosoft.com>
> >> + * Copyright (C) 2010, Code Aurora Forum. All rights reserved.
> >
> > I prefer that we actually see major code contributions that justify a
> > copyright here before you start adding it. And so let the maintainers
> > worry about that later on. I don't wanna see just random copyright
> > additions for obvious fixes or changes that are most likely even derived
> > from existing code.
> >
>
> I'm unsure of the reasoning unless the idea is to add copyright markings
> to show attribution and level of contribution. In our case it's just a
> legal marking.
>
> If the reasoning for adding or not adding is the former, does this not
> rise to that level, and for my own education and future reference why not?
we normally automatically add the copyright for companies or individual
when they make large contributions. Not just small extensions or fixes
where it is questionable if they are copyrightable anyway.
For just tracking contributions, we have GIT :)
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: hidp: Add support for hidraw HIDIOCGFEATURE and HIDIOCSFEATURE
From: Marcel Holtmann @ 2010-07-09 17:33 UTC (permalink / raw)
To: Alan Ott
Cc: David S Miller, Jiri Kosina, Michael Poole, Bastien Nocera,
Eric Dumazet, linux-bluetooth, linux-kernel, netdev
In-Reply-To: <4C372CEE.3070109@signal11.us>
Hi Alan,
> >>>>> I looked at this and I am bit worried that this should not be done in
> >>>>> this detail in the HIDP driver. Essentially HIDP is a pure transport
> >>>>> driver. It should not handle all these details. Can we make this a bit
> >>>>> easier for the transport drivers to support such features?
> >>>>>
> >>>>>
> >>>> I put these changes (most notably the addition of hidp_get_raw_report())
> >>>> in hidp because that's where the parallel function
> >>>> hidp_output_raw_report() was already located. I figured the input should
> >>>> go with the output. That said, if there's a better place for both of
> >>>> them (input and output) to go, let me know where you think it should be,
> >>>> and I'll get them moved into the proper spot.
> >>>>
> >>>> I'm not sure what you mean about HIDP being a pure transport driver.
> >>>>
> >>>>
> >>> what is usb-hid.ko doing here? I would expect a bunch of code
> >>> duplication with minor difference between USB and Bluetooth.
> >>>
> >> usbhid doesn't have a lot of code for hidraw. Two functions are involved:
> >> usbhid_output_raw_report()
> >> - calls usb_control_msg() with Get_Report
> >> usbhid_get_raw_report()
> >> - calls usb_control_msg() with Set_Report
> >> OR
> >> - calls usb_interrupt_msg() on the Ouput pipe.
> >>
> >> This is of course easier than bluetooth because usb_control_msg() is
> >> synchronous, even when requesting reports, mostly because of the nature
> >> of USB, where the request and response are part of the same transfer.
> >>
> >> For Bluetooth, it's a bit more complicated since the kernel treats it
> >> more like a networking interface (and indeed it is). My understanding is
> >> that to make a synchronous transfer in bluetooth, one must:
> >> - send the request packet
> >> - block (wait_event_*())
> >> - when the response is received in the input handler, wake_up_*().
> >>
> >> There's not really any code duplication, mostly because initiating
> >> synchronous USB transfers (input and output) is easy (because of the
> >> usb_*_msg() functions), while making synchronous Bluetooth transfers
> >> must be done manually. If there's a nice, convenient, synchronous
> >> function in Bluetooth similar to usb_control_msg() that I've missed,
> >> then let me know, as it would simplify this whole thing.
> >>
> > there is not and I don't think we ever get one. My question here was
> > more in the direction why HID core is doing these synchronously in the
> > first place. Especially since USB can do everything async as well.
>
> I'm open to suggestions. The way I see it is from a user space
> perspective. With Get_Feature being on an ioctl(), I don't see any clean
> way to do it other than synchronously. Other operating systems (I can
> say for sure Windows, Mac OS X, and FreeBSD) handle Get/Set Feature the
> same way (synchronously) from user space.
>
> You seem to be proposing an asynchronous interface. What would that look
> like from user space?
not necessarily from user space, but at least from HID core to HIDP and
usb-hid transports. At least that is what I would expect, Jiri?
Regards
Marcel
^ permalink raw reply
* Re: Proposed API for HDP
From: José Antonio Santos Cadenas @ 2010-07-09 17:12 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <20100709165508.GC22985@vigoh>
El Friday 09 July 2010 18:55:08 Gustavo F. Padovan escribió:
> Hi José,
>
> * José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-09 15:49:42
+0200]:
> > I wrote a new API based on the changes suggested in this thread.
> >
> > Regards.
> >
> > Jose.
> >
> >
> > BlueZ D-Bus Health API description
> > **********************************
> >
> > Santiago Carot-Nemesio <sancane@gmail.com>
> > José Antonio Santos-Cadenas <santoscadenas@gmail.com>
> > Elvis Pfützenreuter <epx@signove.com>
> >
> > Health Device Profile hierarchy
> > ===============================
> >
> > Service org.bluez
> > Interface org.bluez.HealthManager
> > Object path [variable prefix]/{hci0,hci1,...}
> >
> > Methods:
> > path RegisterApplication(object path, dict config)
> >
> > Returns the path of the new registered application. The path
> > parameter is the path of the object with the callbacks to
> > notify events (see org.bluez.HealthAgent at the end of this
> > document)
> > This petition starts an mcap instance on every adapter and also
> > register a proper record in the SDP if is needed.
> >
> > Dict is defined as bellow:
> > {
> >
> > "end_points" : [{ (optional)
> >
> > "role" : ("source" or "sink"), (mandatory)
> > "specs" :[{ (mandatory)
> >
> > "data_type" : uint16, (mandatory)
> > "description" : string, (optional)
> >
> > }]
> >
> > }]
> >
> > }
> >
> > Application will be closed by the call or implicitly when the
> > programs leaves the bus.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > void UnregisterApplication(path application)
> >
> > Closes the HDP application identified by the object path. Also
> > application will be closed if the process that started it leaves
> > the bus. If there is a SDP record associated to this application
> > it will also be removed.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.NotFound
> >
> > void UpdateServices()
> >
> > This method searches for HDP applications on the all remote
> > devices and notifies them to the appropriate agents.
> >
> > -------------------------------------------------------------------------
> > -------
> >
> > Service org.bluez
> > Interface org.bluez.HealthService
> > Object path [variable
> > prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
> >
> > Methods:
> > array GetProperties()
> >
> > Gets the information of the remote application published on its
> > SDP record. The returned data format is as follows:
> >
> > {
> >
> > "end_points": [
> >
> > "mdepid": uint8,
> > "role" : "source" or "sink" ,
> > "specs" : [{
> >
> > "dtype" : uint16,
> > "description" : string, (optional)
> > }]
> >
> > ]
> >
> > }
>
> What's the name of this property? See the others APIs, properties need a
> name.
I didn't think about properties here (I'm not used to them) But here yes, a
property fixes better than a method call.
>
> > boolean Echo(array{byte})
> >
> > Sends an echo petition to the remote service. Returns True if
> > response matches with the buffer sent. If some error is detected
> > False value is returned and the associated MCL is closed.
> >
> > path OpenDataChannel(byte mdepid, string conf)
>
> Do we really need to expose the mdepid to the application? Isn't there
> any way to abstract this for the application?
In fact I would like to avoid this ugly think (mdep) but we didn't find a way
to avoid it. In fact you should open a connection to an end point and bot the
initiator and the acceptor should be concerned of the end point beeng
connected. Of course find a way to avoid the use of mdep's will be great.
>
> > Creates a new data channel with the indicated config to the
> > remote MCAP Data End Point (MDEP).
> > The configuration should indicate the channel quality of
> > service using one of this values "reliable", "streaming", "any".
>
> What is "any" mean? and make it "Reliable", "Streaming" and "Any", with
> capital letters.
Any means that there is no preference for the quality of service of this data
channel. The remote will decide the Quality of service. This is the correct
way for a sink to create a new data channel, letting the source decide the
QoS.
>
> > Returns the data channel path.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.HealthError
> >
> > void ReconnectDataChannel(path data_channel)
> >
> > Reconnects a previously created data channel indicated by its
> > path.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.HealthError
> > org.bluez.Error.NotFound
> >
> > void DeleteDataChannel(path data_channel)
> >
> > Deletes a data channel so it will not be available.
> >
> > Possible errors: org.bluez.Error.InvalidArguments
> >
> > org.bluez.Error.NotFound
> > org.bluez.Error.HealthError
> >
> > void DeleteAllDataChannels()
> >
> > Deletes all data channels so they will not be available for
> > future use.
> >
> > Possible errors: org.bluez.Error.HealthError
> >
> > HealthAgent hierarchy
> > =====================
> >
> > (this object is implemented by the HDP user in order to receive
> > notifications)
> >
> > Service unique name
> > Interface org.bluez.HealthAgent
> > Object path freely definable
> >
> > Methods:
> > void ServiceDiscovered(object path)
> >
> > This method is called when a device containing an HDP
> > application is paired or when the method Update of the
> > HealthManager is called and new HealthServices are discovered.
> > The object path is the HealthService path. The method will be
> > called for each HealthService.
>
> This one could be a signal.
Shure, I'll change this.
>
> > (Not shure if this should be added)
> >
> > void ServiceRemoved(object path)
> >
> > This method is called if during an Update some HealthServices
> > have disappeared. The method is called once for each removed
> > HealthService.
>
> That one as well.
>
> > void DataChannelCreated(object path, path data_channel, string conf,
> >
> > fd file_descriptor)
> >
> > This method is called when a new data channel is created.
> >
> > The path contains the object path of the HealthService that
> > created the connection, the data_channel is the path that
> > identifies the data channel, conf is the quality of service of
> > the data channel ("reliable" or "streaming") and file_descriptor
> > the file descriptor for reading and writing data.
> >
> > void DataChannelReconnected(object path, path data_channel, string
conf,
> >
> > fd file_descriptor))
> >
> > This method is called when a closed data channel is reconnected
> > by the remote device.
> >
> > The path contains the object path of the HealthService that
> > reconnected the channel. data_channel is the path that
> > identifies the data channel, conf is the quality of service of
> > the data channel ("reliable" or "streaming") and file_descriptor
> > is the file descriptor for reading and writing data.
>
> Couldn't these two be integrated into one? What they do is just pass the
> fd to the application. The application can keep the information about if
> it is a Created or Reconnected data channel because it knows what is
> being requested.
Yes, as Elvis suggested, they will be fixed in to just one method.
>
> Also, is the conf parameter really needed? The application should know
> the channel type it asked.
Not really, it is receiving a new connection it doesn't request nothing. Is
the remote side the one that initiated the connection. So the configuration
should be notified.
>
> > void DataChannelRemoved(object path, path data_channel)
> >
> > This method is called when a data channel is deleted.
> >
> > After this call the data channel path will not be valid and can
> > be reused for future creation of data channels. The path is the
> > path of the HealthService that removes the data channel.
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-bluetooth" in the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] Support for adding UUID128 to extended inquiry response
From: Ron Shaffer @ 2010-07-09 17:03 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: Inga Stotland, linux-bluetooth, johan.hedberg
In-Reply-To: <1278609452.10421.31.camel@localhost.localdomain>
Marcel,
On 7/8/2010 12:17 PM, Marcel Holtmann wrote:
> Hi Inga,
>
>> void register_public_browse_group(void)
>> diff --git a/src/sdpd.h b/src/sdpd.h
>> index e93b0b6..8fd8bbe 100644
>> --- a/src/sdpd.h
>> +++ b/src/sdpd.h
>> @@ -6,6 +6,7 @@
>> * Copyright (C) 2002-2003 Maxim Krasnyansky <maxk@qualcomm.com>
>> * Copyright (C) 2002-2010 Marcel Holtmann <marcel@holtmann.org>
>> * Copyright (C) 2002-2003 Stephen Crane <steve.crane@rococosoft.com>
>> + * Copyright (C) 2010, Code Aurora Forum. All rights reserved.
>
> I prefer that we actually see major code contributions that justify a
> copyright here before you start adding it. And so let the maintainers
> worry about that later on. I don't wanna see just random copyright
> additions for obvious fixes or changes that are most likely even derived
> from existing code.
>
I'm unsure of the reasoning unless the idea is to add copyright markings
to show attribution and level of contribution. In our case it's just a
legal marking.
If the reasoning for adding or not adding is the former, does this not
rise to that level, and for my own education and future reference why not?
--
Ron Shaffer
Employee of the Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: Proposed API for HDP
From: Gustavo F. Padovan @ 2010-07-09 16:55 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201007091549.42621.santoscadenas@gmail.com>
Hi José,
* José Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-07-09 15:49:42 +0200]:
> I wrote a new API based on the changes suggested in this thread.
>
> Regards.
>
> Jose.
>
>
> BlueZ D-Bus Health API description
> **********************************
>
> Santiago Carot-Nemesio <sancane@gmail.com>
> José Antonio Santos-Cadenas <santoscadenas@gmail.com>
> Elvis Pfützenreuter <epx@signove.com>
>
> Health Device Profile hierarchy
> ===============================
>
> Service org.bluez
> Interface org.bluez.HealthManager
> Object path [variable prefix]/{hci0,hci1,...}
>
> Methods:
>
> path RegisterApplication(object path, dict config)
>
> Returns the path of the new registered application. The path
> parameter is the path of the object with the callbacks to
> notify events (see org.bluez.HealthAgent at the end of this
> document)
> This petition starts an mcap instance on every adapter and also
> register a proper record in the SDP if is needed.
>
> Dict is defined as bellow:
> {
> "end_points" : [{ (optional)
> "role" : ("source" or "sink"), (mandatory)
> "specs" :[{ (mandatory)
> "data_type" : uint16, (mandatory)
> "description" : string, (optional)
> }]
> }]
> }
>
> Application will be closed by the call or implicitly when the
> programs leaves the bus.
>
> Possible errors: org.bluez.Error.InvalidArguments
>
> void UnregisterApplication(path application)
>
> Closes the HDP application identified by the object path. Also
> application will be closed if the process that started it leaves
> the bus. If there is a SDP record associated to this application
> it will also be removed.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
>
> void UpdateServices()
>
> This method searches for HDP applications on the all remote
> devices and notifies them to the appropriate agents.
>
> --------------------------------------------------------------------------------
>
> Service org.bluez
> Interface org.bluez.HealthService
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
>
> Methods:
>
> array GetProperties()
>
> Gets the information of the remote application published on its
> SDP record. The returned data format is as follows:
>
> {
> "end_points": [
> "mdepid": uint8,
> "role" : "source" or "sink" ,
> "specs" : [{
> "dtype" : uint16,
> "description" : string, (optional)
> }]
> ]
> }
What's the name of this property? See the others APIs, properties need a name.
>
> boolean Echo(array{byte})
>
> Sends an echo petition to the remote service. Returns True if
> response matches with the buffer sent. If some error is detected
> False value is returned and the associated MCL is closed.
>
> path OpenDataChannel(byte mdepid, string conf)
Do we really need to expose the mdepid to the application? Isn't there
any way to abstract this for the application?
>
> Creates a new data channel with the indicated config to the
> remote MCAP Data End Point (MDEP).
> The configuration should indicate the channel quality of
> service using one of this values "reliable", "streaming", "any".
What is "any" mean? and make it "Reliable", "Streaming" and "Any", with
capital letters.
>
> Returns the data channel path.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.HealthError
>
> void ReconnectDataChannel(path data_channel)
>
> Reconnects a previously created data channel indicated by its
> path.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.HealthError
> org.bluez.Error.NotFound
>
> void DeleteDataChannel(path data_channel)
>
> Deletes a data channel so it will not be available.
>
> Possible errors: org.bluez.Error.InvalidArguments
> org.bluez.Error.NotFound
> org.bluez.Error.HealthError
>
> void DeleteAllDataChannels()
>
> Deletes all data channels so they will not be available for
> future use.
>
> Possible errors: org.bluez.Error.HealthError
>
> HealthAgent hierarchy
> =====================
>
> (this object is implemented by the HDP user in order to receive notifications)
>
> Service unique name
> Interface org.bluez.HealthAgent
> Object path freely definable
>
> Methods:
>
> void ServiceDiscovered(object path)
>
> This method is called when a device containing an HDP
> application is paired or when the method Update of the
> HealthManager is called and new HealthServices are discovered.
> The object path is the HealthService path. The method will be
> called for each HealthService.
This one could be a signal.
>
> (Not shure if this should be added)
> void ServiceRemoved(object path)
>
> This method is called if during an Update some HealthServices
> have disappeared. The method is called once for each removed
> HealthService.
That one as well.
>
> void DataChannelCreated(object path, path data_channel, string conf,
> fd file_descriptor)
>
> This method is called when a new data channel is created.
>
> The path contains the object path of the HealthService that
> created the connection, the data_channel is the path that
> identifies the data channel, conf is the quality of service of
> the data channel ("reliable" or "streaming") and file_descriptor
> the file descriptor for reading and writing data.
>
> void DataChannelReconnected(object path, path data_channel, string conf,
> fd file_descriptor))
>
> This method is called when a closed data channel is reconnected
> by the remote device.
>
> The path contains the object path of the HealthService that
> reconnected the channel. data_channel is the path that
> identifies the data channel, conf is the quality of service of
> the data channel ("reliable" or "streaming") and file_descriptor
> is the file descriptor for reading and writing data.
Couldn't these two be integrated into one? What they do is just pass the
fd to the application. The application can keep the information about if
it is a Created or Reconnected data channel because it knows what is
being requested.
Also, is the conf parameter really needed? The application should know
the channel type it asked.
>
> void DataChannelRemoved(object path, path data_channel)
>
> This method is called when a data channel is deleted.
>
> After this call the data channel path will not be valid and can
> be reused for future creation of data channels. The path is the
> path of the HealthService that removes the data channel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH] Fix signal strength for HFP in maemo6 telephony due to changed API in libcsnet.
From: Gustavo F. Padovan @ 2010-07-09 16:45 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1278686657-6997-1-git-send-email-dmitriy.paliy@nokia.com>
Hi Dmitriy,
* Dmitriy Paliy <dmitriy.paliy@nokia.com> [2010-07-09 17:44:17 +0300]:
> Signed-off-by: Dmitriy Paliy <dmitriy.paliy@nokia.com>
No need for Signed-off-by line on userspace code.
> ---
> audio/telephony-maemo6.c | 50 +++++++++++++++++++++------------------------
> 1 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
> index 31704c3..da3c19a 100644
> --- a/audio/telephony-maemo6.c
> +++ b/audio/telephony-maemo6.c
Looks you are doing at least 3 differents things in the same patch.
Changing the variables names, changing the o DBus interface and your
fixes. Split it in a proper atomic patches then we can figure out what
you are doing.
> @@ -134,11 +134,12 @@ struct csd_call {
> static struct {
> char *operator_name;
> uint8_t status;
> - int32_t signals_bar;
> + int32_t signal_bars;
> } net = {
> .operator_name = NULL,
> .status = NETWORK_REG_STATUS_UNKOWN,
> - .signals_bar = 0,
> + .signal_bars = 0, /* Init as 0 meaning inactive mode. In modem power off state it can */
> + /* be -1, but we treat all values as 0s regardless inactive or power off. */
> };
>
> static int get_property(const char *iface, const char *prop);
> @@ -181,7 +182,7 @@ static guint create_request_timer = 0;
> static struct indicator maemo_indicators[] =
> {
> { "battchg", "0-5", 5, TRUE },
> - { "signal", "0-5", 0, TRUE },
> + { "signal", "0-5", 0, TRUE }, /* signal strength in terms of bars */
> { "service", "0,1", 0, TRUE },
> { "call", "0,1", 0, TRUE },
> { "callsetup", "0-3", 0, TRUE },
> @@ -1227,43 +1228,38 @@ static void handle_registration_changed(DBusMessage *msg)
> update_registration_status(status);
> }
>
> -static void update_signal_strength(int32_t signals_bar)
> +static void update_signal_strength(int32_t signal_strength_bars)
> {
> - int signal;
> -
> - if (signals_bar < 0)
> - signals_bar = 0;
> - else if (signals_bar > 100) {
> - DBG("signals_bar greater than expected: %u", signals_bar);
> - signals_bar = 100;
> + if (signal_strength_bars < 0) {
> + DBG("signal strength bars smaller than expected: %d", signal_strength_bars);
> + signal_strength_bars = 0;
> + } else if (signal_strength_bars > 5) {
> + DBG("signal strength bars greater than expected: %d", signal_strength_bars);
> + signal_strength_bars = 5;
> }
Do you really need these DBG() here?
>
> - if (net.signals_bar == signals_bar)
> + if (net.signal_bars == signal_strength_bars)
> return;
>
> - /* A simple conversion from 0-100 to 0-5 (used by HFP) */
> - signal = (signals_bar + 20) / 21;
> -
> - telephony_update_indicator(maemo_indicators, "signal", signal);
> + telephony_update_indicator(maemo_indicators, "signal", signal_strength_bars);
>
> - net.signals_bar = signals_bar;
> + net.signal_bars = signal_strength_bars;
>
> - DBG("telephony-maemo6: signal strength updated: %u/100, %d/5", signals_bar, signal);
Don't overstep line 80.
> + DBG("telephony-maemo6: signal strength bars updated: %d/5", signal_strength_bars);
> }
>
> -static void handle_signal_strength_changed(DBusMessage *msg)
> +static void handle_signal_bars_changed(DBusMessage *msg)
> {
> - int32_t signals_bar, rssi_in_dbm;
> + int32_t signal_bars;
>
> if (!dbus_message_get_args(msg, NULL,
> - DBUS_TYPE_INT32, &signals_bar,
> - DBUS_TYPE_INT32, &rssi_in_dbm,
What happened to rssi_in_dbm? Why are you removing it?
> + DBUS_TYPE_INT32, &signal_bars,
> DBUS_TYPE_INVALID)) {
> - error("Unexpected parameters in SignalStrengthChanged");
> + error("Unexpected parameters in SignalBarsChanged");
> return;
> }
>
> - update_signal_strength(signals_bar);
> + update_signal_strength(signal_bars);
> }
>
> static gboolean iter_get_basic_args(DBusMessageIter *iter,
> @@ -1530,7 +1526,7 @@ static void get_property_reply(DBusPendingCall *call, void *user_data)
> dbus_message_iter_get_basic(&sub, &name);
> update_operator_name(name);
> } else if (g_strcmp0(prop, "SignalBars") == 0) {
> - uint32_t signal_bars;
> + int32_t signal_bars;
>
> dbus_message_iter_get_basic(&sub, &signal_bars);
> update_signal_strength(signal_bars);
> @@ -1899,8 +1895,8 @@ static DBusHandlerResult signal_filter(DBusConnection *conn,
> "OperatorNameChanged"))
> handle_operator_name_changed(msg);
> else if (dbus_message_is_signal(msg, CSD_CSNET_SIGNAL,
> - "SignalStrengthChanged"))
> - handle_signal_strength_changed(msg);
> + "SignalBarsChanged"))
> + handle_signal_bars_changed(msg);
> else if (dbus_message_is_signal(msg, "org.freedesktop.Hal.Device",
> "PropertyModified"))
> handle_hal_property_modified(msg);
> --
> 1.7.0.4
>
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH] Fix signal strength for HFP in maemo6 telephony due to changed API in libcsnet.
From: Hedberg Johan (Nokia-D/Helsinki) @ 2010-07-09 16:39 UTC (permalink / raw)
To: Dmitriy Paliy; +Cc: linux-bluetooth
In-Reply-To: <1278686657-6997-1-git-send-email-dmitriy.paliy@nokia.com>
Hi Dmitriy,
Congrats on your first patch submission! However, there are a few things
that need fixing:
On Fri, Jul 09, 2010, Dmitriy Paliy wrote:
> Signed-off-by: Dmitriy Paliy <dmitriy.paliy@nokia.com>
We don't use Signed-off-by in user space BlueZ, so please remove it.
Also make sure that your commit message lines don't exceed 74
characters. When you do "git log" on a 80-character wide terminal the
message should be propertly viewable. In this case the summary line
could just be "Fix signal strength for HFP in maemo6 telephony driver"
and then in the body of the commit message (which is encouraged to have
for any non-trivial commits) you can have a proper explanation of how
the csd API has changed. In general, being too verbose in the commit
message is less bad than not being verbose enough.
> - .signals_bar = 0,
> + .signal_bars = 0, /* Init as 0 meaning inactive mode. In modem power off state it can */
> + /* be -1, but we treat all values as 0s regardless inactive or power off. */
No over 79 character lines in the code please. In this case you can just
put the comment above instead of on the same line.
> - { "signal", "0-5", 0, TRUE },
> + { "signal", "0-5", 0, TRUE }, /* signal strength in terms of bars */
Same here.
> + if (signal_strength_bars < 0) {
> + DBG("signal strength bars smaller than expected: %d", signal_strength_bars);
> + signal_strength_bars = 0;
> + } else if (signal_strength_bars > 5) {
> + DBG("signal strength bars greater than expected: %d", signal_strength_bars);
> + signal_strength_bars = 5;
The DBG lines look like they're over 79 too.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox