Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

The following changes since commit 646f0c7e6b557c5413825ce7b04bee52bf0129e8:

  Move remote name and version requests to hciops (2010-10-10 22:44:25 +0100)

are available in the git repository at:
  git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

José Antonio Santos Cadenas (6):
      Remove mdl_conn from struct hdp_channel
      Remove app paramter from hdp_establish_mcl, it is not needed
      Imcoming connection in control channel should be ERTM
      Fix multiple emission of main channel property signal.
      Return MCAP_MDL_BUSY when health channel can't be created
      Implement callback for responding echo petitions

Santiago Carot-Nemesio (6):
      Add a new function to create channels
      Process request of of echo channels creation
      Correctly notify the deletion of the reliable data channel
      Add extra checks for avoid notifiying incoming echo channels
      Delete all channels DBus interface when the instance is removed
      Check first reliable configuration during channel creation

 health/hdp.c       |  249 +++++++++++++++++++++++++++++++++++----------------
 health/hdp_types.h |    3 +-
 health/hdp_util.c  |    3 -
 health/hdp_util.h  |    1 -
 health/mcap.c      |    1 +
 5 files changed, 174 insertions(+), 83 deletions(-)

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Johan Hedberg]

Hi,

On Mon, Oct 11, 2010, Jose Antonio Santos Cadenas wrote:
> The following changes since commit 646f0c7e6b557c5413825ce7b04bee52bf0129e8:
> 
>   Move remote name and version requests to hciops (2010-10-10 22:44:25 +0100)
> 
> are available in the git repository at:
>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
> 
> José Antonio Santos Cadenas (6):
>       Remove mdl_conn from struct hdp_channel
>       Remove app paramter from hdp_establish_mcl, it is not needed
>       Imcoming connection in control channel should be ERTM
>       Fix multiple emission of main channel property signal.
>       Return MCAP_MDL_BUSY when health channel can't be created
>       Implement callback for responding echo petitions
> 
> Santiago Carot-Nemesio (6):
>       Add a new function to create channels
>       Process request of of echo channels creation
>       Correctly notify the deletion of the reliable data channel
>       Add extra checks for avoid notifiying incoming echo channels
>       Delete all channels DBus interface when the instance is removed
>       Check first reliable configuration during channel creation
> 
>  health/hdp.c       |  249 +++++++++++++++++++++++++++++++++++----------------
>  health/hdp_types.h |    3 +-
>  health/hdp_util.c  |    3 -
>  health/hdp_util.h  |    1 -
>  health/mcap.c      |    1 +
>  5 files changed, 174 insertions(+), 83 deletions(-)

Could you try to ping me on IRC when possible so we can solve a few
issues with these patches. It seems e.g. that you're doing GIOChannel
reference counting incorrectly in one place. There are also some english
language errors in the commit messages that would be good to fix (let's
see if you can spot them yourselves, otherwise I'll point them out when
we go over the other issues ;)

Johan

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

Hi,

2010/10/11 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi,
>
> On Mon, Oct 11, 2010, Jose Antonio Santos Cadenas wrote:
>> The following changes since commit 646f0c7e6b557c5413825ce7b04bee52bf0129e8:
>>
>>   Move remote name and version requests to hciops (2010-10-10 22:44:25 +0100)
>>
>> are available in the git repository at:
>>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
>>
>> José Antonio Santos Cadenas (6):
>>       Remove mdl_conn from struct hdp_channel
>>       Remove app paramter from hdp_establish_mcl, it is not needed
>>       Imcoming connection in control channel should be ERTM
>>       Fix multiple emission of main channel property signal.
>>       Return MCAP_MDL_BUSY when health channel can't be created
>>       Implement callback for responding echo petitions
>>
>> Santiago Carot-Nemesio (6):
>>       Add a new function to create channels
>>       Process request of of echo channels creation
>>       Correctly notify the deletion of the reliable data channel
>>       Add extra checks for avoid notifiying incoming echo channels
>>       Delete all channels DBus interface when the instance is removed
>>       Check first reliable configuration during channel creation
>>
>>  health/hdp.c       |  249 +++++++++++++++++++++++++++++++++++----------------
>>  health/hdp_types.h |    3 +-
>>  health/hdp_util.c  |    3 -
>>  health/hdp_util.h  |    1 -
>>  health/mcap.c      |    1 +
>>  5 files changed, 174 insertions(+), 83 deletions(-)
>
> Could you try to ping me on IRC when possible so we can solve a few
> issues with these patches. It seems e.g. that you're doing GIOChannel
> reference counting incorrectly in one place. There are also some english
> language errors in the commit messages that would be good to fix (let's
> see if you can spot them yourselves, otherwise I'll point them out when
> we go over the other issues ;)

I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
think that I've found the issues:

- The iochannel should be unrefereed when the watcher is explicitly removed
- The commit message s/paramter/parameter

Are there any more?

I will solve them in half an hour.

>
> Johan
>

Jose.

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

Hi,

2010/10/12 Jose Antonio Santos Cadenas <santoscadenas@gmail.com>:
> Hi,
>
> 2010/10/11 Johan Hedberg <johan.hedberg@gmail.com>:
>> Hi,
>>
>> On Mon, Oct 11, 2010, Jose Antonio Santos Cadenas wrote:
>>> The following changes since commit 646f0c7e6b557c5413825ce7b04bee52bf0129e8:
>>>
>>>   Move remote name and version requests to hciops (2010-10-10 22:44:25 +0100)
>>>
>>> are available in the git repository at:
>>>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
>>>
>>> José Antonio Santos Cadenas (6):
>>>       Remove mdl_conn from struct hdp_channel
>>>       Remove app paramter from hdp_establish_mcl, it is not needed
>>>       Imcoming connection in control channel should be ERTM
>>>       Fix multiple emission of main channel property signal.
>>>       Return MCAP_MDL_BUSY when health channel can't be created
>>>       Implement callback for responding echo petitions
>>>
>>> Santiago Carot-Nemesio (6):
>>>       Add a new function to create channels
>>>       Process request of of echo channels creation
>>>       Correctly notify the deletion of the reliable data channel
>>>       Add extra checks for avoid notifiying incoming echo channels
>>>       Delete all channels DBus interface when the instance is removed
>>>       Check first reliable configuration during channel creation
>>>
>>>  health/hdp.c       |  249 +++++++++++++++++++++++++++++++++++----------------
>>>  health/hdp_types.h |    3 +-
>>>  health/hdp_util.c  |    3 -
>>>  health/hdp_util.h  |    1 -
>>>  health/mcap.c      |    1 +
>>>  5 files changed, 174 insertions(+), 83 deletions(-)
>>
>> Could you try to ping me on IRC when possible so we can solve a few
>> issues with these patches. It seems e.g. that you're doing GIOChannel
>> reference counting incorrectly in one place. There are also some english
>> language errors in the commit messages that would be good to fix (let's
>> see if you can spot them yourselves, otherwise I'll point them out when
>> we go over the other issues ;)
>
> I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
> think that I've found the issues:
>
> - The iochannel should be unrefereed when the watcher is explicitly removed
> - The commit message s/paramter/parameter
>
> Are there any more?
>
> I will solve them in half an hour.

I've rebased the branch with the changes that I described before. I've
also fixed an other issue related with the mdep checks, now a macro is
used to make the code more readable and easy to mantain.

Regards.

Jose.

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Johan Hedberg]

Hi Jose,

On Tue, Oct 12, 2010, Jose Antonio Santos Cadenas wrote:
> I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
> think that I've found the issues:
> 
> - The iochannel should be unrefereed when the watcher is explicitly removed

Yes, though do you need to have a reference at all? As long as you have
the GLib watch callback GLib itself holds a reference, so if you don't
need one just remove chan->echo_chan and do an unref after calling
add_watch.

> - The commit message s/paramter/parameter
> 
> Are there any more?

s/Imcoming/Incoming/
s/DBus/D-Bus/

And the following doesn't make much sense to me:

"Delete all channels DBus interface when the instance is removed"

Could you rephrase it somehow, maybe by putting most of it in the
message body instead of the summary line. Is it trying to say "Delete
the D-Bus interfaces of all channels"? The summary line shouldn't be too
complex. You can just say "Fix D-Bus channel removal when removing
instances" and then in the message body do the more detailed
explanation.

Johan

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

Hi,

2010/10/12 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi Jose,
>
> On Tue, Oct 12, 2010, Jose Antonio Santos Cadenas wrote:
>> I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
>> think that I've found the issues:
>>
>> - The iochannel should be unrefereed when the watcher is explicitly removed
>
> Yes, though do you need to have a reference at all? As long as you have
> the GLib watch callback GLib itself holds a reference, so if you don't
> need one just remove chan->echo_chan and do an unref after calling
> add_watch.
>
>> - The commit message s/paramter/parameter
>>
>> Are there any more?
>
> s/Imcoming/Incoming/

Solved this one

> s/DBus/D-Bus/
>
> And the following doesn't make much sense to me:
>
> "Delete all channels DBus interface when the instance is removed"
>
> Could you rephrase it somehow, maybe by putting most of it in the
> message body instead of the summary line. Is it trying to say "Delete
> the D-Bus interfaces of all channels"? The summary line shouldn't be too
> complex. You can just say "Fix D-Bus channel removal when removing
> instances" and then in the message body do the more detailed
> explanation.

I've changed the commit message and added an explanation. I hope now is clearer.

>
> Johan
>

Regards.

Jose.

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

2010/10/12 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi Jose,
>
> On Tue, Oct 12, 2010, Jose Antonio Santos Cadenas wrote:
>> I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
>> think that I've found the issues:
>>
>> - The iochannel should be unrefereed when the watcher is explicitly removed
>
> Yes, though do you need to have a reference at all? As long as you have
> the GLib watch callback GLib itself holds a reference, so if you don't
> need one just remove chan->echo_chan and do an unref after calling
> add_watch.

Sorry I've forgot this one. Gonna solve it.

>
>> - The commit message s/paramter/parameter
>>
>> Are there any more?
>
> s/Imcoming/Incoming/
> s/DBus/D-Bus/
>
> And the following doesn't make much sense to me:
>
> "Delete all channels DBus interface when the instance is removed"
>
> Could you rephrase it somehow, maybe by putting most of it in the
> message body instead of the summary line. Is it trying to say "Delete
> the D-Bus interfaces of all channels"? The summary line shouldn't be too
> complex. You can just say "Fix D-Bus channel removal when removing
> instances" and then in the message body do the more detailed
> explanation.
>
> Johan
>

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

2010/10/12 Jose Antonio Santos Cadenas <santoscadenas@gmail.com>:
> 2010/10/12 Johan Hedberg <johan.hedberg@gmail.com>:
>> Hi Jose,
>>
>> On Tue, Oct 12, 2010, Jose Antonio Santos Cadenas wrote:
>>> I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
>>> think that I've found the issues:
>>>
>>> - The iochannel should be unrefereed when the watcher is explicitly removed
>>
>> Yes, though do you need to have a reference at all? As long as you have
>> the GLib watch callback GLib itself holds a reference, so if you don't
>> need one just remove chan->echo_chan and do an unref after calling
>> add_watch.
>
> Sorry I've forgot this one. Gonna solve it.

Solved, I've removed the chan->echo_chan attribute and directly unref
the io_channel once the watcher is set.

>
>>
>>> - The commit message s/paramter/parameter
>>>
>>> Are there any more?
>>
>> s/Imcoming/Incoming/
>> s/DBus/D-Bus/
>>
>> And the following doesn't make much sense to me:
>>
>> "Delete all channels DBus interface when the instance is removed"
>>
>> Could you rephrase it somehow, maybe by putting most of it in the
>> message body instead of the summary line. Is it trying to say "Delete
>> the D-Bus interfaces of all channels"? The summary line shouldn't be too
>> complex. You can just say "Fix D-Bus channel removal when removing
>> instances" and then in the message body do the more detailed
>> explanation.
>>
>> Johan
>>
>

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Johan Hedberg]

Hi Jose,

On Tue, Oct 12, 2010, Jose Antonio Santos Cadenas wrote:
> >> On Tue, Oct 12, 2010, Jose Antonio Santos Cadenas wrote:
> >>> I'm sorry but I can't connect to the IRC rigth now. Nevertheless, I
> >>> think that I've found the issues:
> >>>
> >>> - The iochannel should be unrefereed when the watcher is explicitly removed
> >>
> >> Yes, though do you need to have a reference at all? As long as you have
> >> the GLib watch callback GLib itself holds a reference, so if you don't
> >> need one just remove chan->echo_chan and do an unref after calling
> >> add_watch.
> >
> > Sorry I've forgot this one. Gonna solve it.
> 
> Solved, I've removed the chan->echo_chan attribute and directly unref
> the io_channel once the watcher is set.

Thanks. The patches have now been pushed upstream.

Johan

Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

The following changes since commit 6b9deca15d0824ad5b3c38f8c0dd3a3c482572ea:

  TODO: Device Name Characteristic for Low Energy (2010-10-13 22:26:59 +0300)

are available in the git repository at:
  git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

José Antonio Santos Cadenas (7):
      Connect echo channel
      Initial steps to send echo data
      Generate random echo packet
      Create a function for closing mcl correctly
      Notify channel deletion correctly
      Separate private echo data from data channels
      Add initial support for deleting an echo channel

Santiago Carot-Nemesio (8):
      Add support to initiate echo
      Remove unnecessary variable
      Set watcher to check echo response
      Check echo response
      Add timeout to wait for echo reply
      Abort echo channel if connection was not successful
      Remove MDL when delete operation fails with INVALID_MDLID error
      Remove unnecessary function

 health/hdp.c       |  431 +++++++++++++++++++++++++++++++++++++++++-----------
 health/hdp_types.h |    5 +-
 health/mcap.c      |   14 ++-
 3 files changed, 361 insertions(+), 89 deletions(-)

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Johan Hedberg]

Hi Jose,

On Thu, Oct 14, 2010, Jose Antonio Santos Cadenas wrote:
> The following changes since commit 6b9deca15d0824ad5b3c38f8c0dd3a3c482572ea:
> 
>   TODO: Device Name Characteristic for Low Energy (2010-10-13 22:26:59 +0300)
> 
> are available in the git repository at:
>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
> 
> José Antonio Santos Cadenas (7):
>       Connect echo channel
>       Initial steps to send echo data
>       Generate random echo packet
>       Create a function for closing mcl correctly
>       Notify channel deletion correctly
>       Separate private echo data from data channels
>       Add initial support for deleting an echo channel
> 
> Santiago Carot-Nemesio (8):
>       Add support to initiate echo
>       Remove unnecessary variable
>       Set watcher to check echo response
>       Check echo response
>       Add timeout to wait for echo reply
>       Abort echo channel if connection was not successful
>       Remove MDL when delete operation fails with INVALID_MDLID error
>       Remove unnecessary function
> 
>  health/hdp.c       |  431 +++++++++++++++++++++++++++++++++++++++++-----------
>  health/hdp_types.h |    5 +-
>  health/mcap.c      |   14 ++-
>  3 files changed, 361 insertions(+), 89 deletions(-)

Pushed upstream. Thanks.

Johan

Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Jose Antonio Santos Cadenas]

The following changes since commit a4b1673b878a358d2492f24b46e92ea36d8f8bbf:

  Fix hidd to use ServiceName attribute for device name (2010-10-20
13:37:36 +0300)

are available in the git repository at:
  git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

José Antonio Santos Cadenas (6):
      Receive an string instead of an integer for ChannelType
      Modify test-health script to make it more interactive
      Notify main channel when it is locally opened
      Emit a valid path when main channel is deleted
      Delete data channels when their device is removed.
      Check if the channel mode is correct when opening data channels

Santiago Carot-Nemesio (6):
      Remove obsolete comment from MCAP
      Fix segmentation fault freeing uninitialized pointers
      Check l2cap configuration when data channels are connected
      Close the data channel if remote side changes the configuration
      Enable support to change mode for incoming data channels connections
      Change data channel mode for incoming connections

 health/hdp.c      |  106 +++++++++++++++++++++++++--
 health/hdp_util.c |   35 +++++++---
 health/mcap.c     |   15 ++++-
 health/mcap_lib.h |    3 +
 test/test-health  |  207 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 340 insertions(+), 26 deletions(-)

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Johan Hedberg]

Hi Jose,

On Thu, Oct 21, 2010, Jose Antonio Santos Cadenas wrote:
> The following changes since commit a4b1673b878a358d2492f24b46e92ea36d8f8bbf:
> 
>   Fix hidd to use ServiceName attribute for device name (2010-10-20
> 13:37:36 +0300)
> 
> are available in the git repository at:
>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
> 
> José Antonio Santos Cadenas (6):
>       Receive an string instead of an integer for ChannelType
>       Modify test-health script to make it more interactive
>       Notify main channel when it is locally opened
>       Emit a valid path when main channel is deleted
>       Delete data channels when their device is removed.
>       Check if the channel mode is correct when opening data channels
> 
> Santiago Carot-Nemesio (6):
>       Remove obsolete comment from MCAP
>       Fix segmentation fault freeing uninitialized pointers
>       Check l2cap configuration when data channels are connected
>       Close the data channel if remote side changes the configuration
>       Enable support to change mode for incoming data channels connections
>       Change data channel mode for incoming connections
> 
>  health/hdp.c      |  106 +++++++++++++++++++++++++--
>  health/hdp_util.c |   35 +++++++---
>  health/mcap.c     |   15 ++++-
>  health/mcap_lib.h |    3 +
>  test/test-health  |  207 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  5 files changed, 340 insertions(+), 26 deletions(-)

Pushed upstream. Thanks.

Johan

Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Santiago Carot-Nemesio]

  Fix typos in adapter documentation (2010-11-16 13:39:43 +0000)

are available in the git repository at:
  git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

José Antonio Santos Cadenas (9):
      Add reference counter to hdp_application
      Add reference counter to hdp_channel
      Notify a channel deleted when remote side has already deleted it
      Fix typos in mcap
      Set reference counter for mcls to gint
      Fix message error
      Remove magic number to check ECHO MDEPs in HDP
      Fixes to echo
      Check if the mcl insiede the device is correctly set before use it

Santiago Carot-Nemesio (10):
      Add reference counter to hdp_device
      Remove MCL's before removing the application
      Add missed unrefs for hdp_tmp_dc_data
      Add reference counter to mcap_instances
      Add reference counter to mcap_mdl
      Remove old reference to mcap session in the name of variables
      Code refactorization in MCAP
      Fix dereference to NULL pointers during data channels creation
      Return proper response code if there is an error creating echo channel
      Check if MCAP Instance is already released when a callbacks comes back.

 health/hdp.c           |  266 ++++++++++++++++++++++++++++++++----------------
 health/hdp_types.h     |    4 +
 health/hdp_util.c      |   55 ++++++++++-
 health/hdp_util.h      |    8 ++
 health/mcap.c          |  242 +++++++++++++++++++++++++------------------
 health/mcap_internal.h |    9 +-
 health/mcap_lib.h      |   13 ++-
 health/mcap_sync.c     |   22 ++--
 8 files changed, 407 insertions(+), 212 deletions(-)

Re: Pull request git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream

[Johan Hedberg]

Hi,

On Thu, Nov 18, 2010, Santiago Carot-Nemesio wrote:
>   Fix typos in adapter documentation (2010-11-16 13:39:43 +0000)
> 
> are available in the git repository at:
>   git://gitorious.org/bluez-mcap-hdp/mcap-hdp.git for_upstream
> 
> José Antonio Santos Cadenas (9):
>       Add reference counter to hdp_application
>       Add reference counter to hdp_channel
>       Notify a channel deleted when remote side has already deleted it
>       Fix typos in mcap
>       Set reference counter for mcls to gint
>       Fix message error
>       Remove magic number to check ECHO MDEPs in HDP
>       Fixes to echo
>       Check if the mcl insiede the device is correctly set before use it
> 
> Santiago Carot-Nemesio (10):
>       Add reference counter to hdp_device
>       Remove MCL's before removing the application
>       Add missed unrefs for hdp_tmp_dc_data
>       Add reference counter to mcap_instances
>       Add reference counter to mcap_mdl
>       Remove old reference to mcap session in the name of variables
>       Code refactorization in MCAP
>       Fix dereference to NULL pointers during data channels creation
>       Return proper response code if there is an error creating echo channel
>       Check if MCAP Instance is already released when a callbacks comes back.
> 
>  health/hdp.c           |  266 ++++++++++++++++++++++++++++++++----------------
>  health/hdp_types.h     |    4 +
>  health/hdp_util.c      |   55 ++++++++++-
>  health/hdp_util.h      |    8 ++
>  health/mcap.c          |  242 +++++++++++++++++++++++++------------------
>  health/mcap_internal.h |    9 +-
>  health/mcap_lib.h      |   13 ++-
>  health/mcap_sync.c     |   22 ++--
>  8 files changed, 407 insertions(+), 212 deletions(-)

In general, please try to avoid such big sets of patches and send them
in smaller bits (as they get created) to linux-bluetooth. That makes
things easier for me and I am able to do a more thorough review.
However, I did a quick skim through of the patches and only found one
coding style issue (C++ style comment) which I fixed my self. So the
patches have now all been pushed upstream.

Johan