2009-05-15 19:40:35

by Mario Limonciello

[permalink] [raw]
Subject: [PATCH 1/3] Add support for command line arguments on hid2hci tool

This means that hid2hci won't have to run
on every bootup for every machine with bluetooth installed. It instead
gets ran on demand if a
product that is called out from the dbus rules file contains the
correct attributes and/or VID/PID.

It also makes it easier for users to manually test new VID/PID
combinations that may or may not
be yet supported by bluez.
---
scripts/Makefile.am | 12 ++-
scripts/bluetooth.default | 3 -
scripts/bluetooth.init | 7 --
scripts/hid2hci.rules | 38 ++++++++++
tools/hid2hci.c | 179
+++++++++++++++++++-------------------------
5 files changed, 124 insertions(+), 115 deletions(-)
create mode 100644 scripts/hid2hci.rules

Attaching patch as Exchange would mangle otherwise.

Thanks,
--
Mario Limonciello
*Dell | Linux Engineering*
[email protected]


Attachments:
0001-Add-support-for-command-line-arguments-on-hid2hci-to.patch (13.52 kB)
signature.asc (260.00 B)
OpenPGP digital signature
Download all attachments

2009-05-16 13:35:51

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add support for command line arguments on hid2hci tool

Hi Mario,

> > please restrict the commit message to 70-72 chars per line.
> >
> > And please do the following changes:
> >
> > s/radiomode/mode/ since it is not about the radio part here.
> >
> > s/hidproxy/csr/ if you wanna expose this then call it what it is.
> >
> > And go over the coding style once more since there are some cases where
> > it breaks.
> >
> > Also please split Makefile/rules changes from the actual code changes. I
> > want two patches here. One that fixes the code and another one that
> > takes care of the udev integration.
> >
> > Other than that, looks pretty good.
> >
> Thanks for the feedback. I've hopefully addressed all of your
> concerns. If you still have problems with coding style, can you please
> point them out specifically? The attached patch is the code portion of
> the split up patch. Again it's attached so my mail server doesn't
> mangle it.

patch has been applied. However I was serious about the 70-72 chars
width of the commit message. Had to manually fix that.

Also it is Bluetooth and not bluetooth when writing about it. That is a
trademark issue :)

Regards

Marcel



2009-05-15 22:07:07

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add support for command line arguments on hid2hci tool

Hi Marcel:

Marcel Holtmann wrote:
> Hi Mario,
>
> please restrict the commit message to 70-72 chars per line.
>
> And please do the following changes:
>
> s/radiomode/mode/ since it is not about the radio part here.
>
> s/hidproxy/csr/ if you wanna expose this then call it what it is.
>
> And go over the coding style once more since there are some cases where
> it breaks.
>
> Also please split Makefile/rules changes from the actual code changes. I
> want two patches here. One that fixes the code and another one that
> takes care of the udev integration.
>
> Other than that, looks pretty good.
>
> Regards
>
> Marcel
>
Thanks for the feedback. I've hopefully addressed all of your
concerns. If you still have problems with coding style, can you please
point them out specifically? The attached patch is the code portion of
the split up patch. Again it's attached so my mail server doesn't
mangle it.

Regards
--
Mario Limonciello
*Dell | Linux Engineering*
[email protected]


Attachments:
0001-Add-support-for-command-line-arguments-on-hid2hci-to.patch (9.04 kB)
signature.asc (260.00 B)
OpenPGP digital signature
Download all attachments

2009-05-15 20:11:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/3] Add support for command line arguments on hid2hci tool

Hi Mario,

> This means that hid2hci won't have to run
> on every bootup for every machine with bluetooth installed. It instead
> gets ran on demand if a
> product that is called out from the dbus rules file contains the
> correct attributes and/or VID/PID.
>
> It also makes it easier for users to manually test new VID/PID
> combinations that may or may not
> be yet supported by bluez.
> ---
> scripts/Makefile.am | 12 ++-
> scripts/bluetooth.default | 3 -
> scripts/bluetooth.init | 7 --
> scripts/hid2hci.rules | 38 ++++++++++
> tools/hid2hci.c | 179
> +++++++++++++++++++-------------------------
> 5 files changed, 124 insertions(+), 115 deletions(-)
> create mode 100644 scripts/hid2hci.rules
>
> Attaching patch as Exchange would mangle otherwise.

please restrict the commit message to 70-72 chars per line.

And please do the following changes:

s/radiomode/mode/ since it is not about the radio part here.

s/hidproxy/csr/ if you wanna expose this then call it what it is.

And go over the coding style once more since there are some cases where
it breaks.

Also please split Makefile/rules changes from the actual code changes. I
want two patches here. One that fixes the code and another one that
takes care of the udev integration.

Other than that, looks pretty good.

Regards

Marcel