Linux bluetooth development
 help / color / mirror / Atom feed
* 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

* 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: 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 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: 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/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: [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: 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] 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 (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 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 (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

* [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

* [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

* 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 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

* [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

* 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

* 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] 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

* [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] 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

* 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

* [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: [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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox