From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Message-ID: From: Claudio Takahasi To: bluez-devel@lists.sourceforge.net Subject: Re: [Bluez-devel] hcid patch In-Reply-To: <1128373377.8472.44.camel@blade> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_4060_6309862.1128429212132" References: <1128160192.8555.6.camel@localhost.localdomain> <1128373377.8472.44.camel@blade> Sender: bluez-devel-admin@lists.sourceforge.net Errors-To: bluez-devel-admin@lists.sourceforge.net Reply-To: bluez-devel@lists.sourceforge.net List-Unsubscribe: , List-Id: BlueZ development List-Post: List-Help: List-Subscribe: , List-Archive: Date: Tue, 4 Oct 2005 09:33:32 -0300 ------=_Part_4060_6309862.1128429212132 Content-Type: multipart/alternative; boundary="----=_Part_4061_17858956.1128429212132" ------=_Part_4061_17858956.1128429212132 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Hi Marcel, Your suggestions were done! Regarding service_table_t, if we remove it, the logic will be based on "if"and "else", see the HAL message filter function. In my opinion service_table_t is more clear. If you think that is really necessary remove the error handling and the service_table_t I can do it in the next patch. Regards, Claudio. On 10/3/05, Marcel Holtmann wrote: > > Hi Claudio, > > > I kept some log information, if you want remove it I can send another > > patch. The error code definition was included. I am using "hciX" in > > the path name until we receive a answer from dbus list. Services under > > the path /org/bluez/Devices weren't implemented yet. > > > > Is there other changes in this patch? > > this thing looks quite good, but now we need to clean up some other > parts of your constants and function naming. And please try to keep the > BlueZ/kernel coding style. > > The overall BLUEZ_DBUS prefix looks like a good idea at the beginning, > but I think we don't need it at all. So this should be enough: > > #define DEVICE_PATH "/org/bluez/Device" > #define DEVICE_INTERFACE "org.bluez.Device" > > #define MANAGER_PATH "/org/bluez/Manager" > #define MANAGER_INTERFACE "org.bluez.Manager" > > Please also stop to shortcut some names. For example DFT for DEFAULT. > Simple write the full name, because then it is easy to read this code. > > The other thing is that I don't like the difference between *_REQ and > *_SIG in the names/constants. Please don't use them. > > Also please don't include any RFCOMM, PAN, HID etc. related things, > because they are not important right now and they make my life of > merging the initial step into better D-Bus a lot harder. > > For the error part we will use org.bluez.Error and not EFailed or > anything like this. Since all the error stuff is currently not used, I > don't think it is a good idea to add it to the initial patch. Looking at > HAL the use of org.bluez.NoSuchDevice, etc. seems to make sense, too. > > Do we really need that service_table_t thingy now? I think we should > start without it. > > Regards > > Marcel > > > > > ------------------------------------------------------- > This SF.Net email is sponsored by: > Power Architecture Resource Center: Free content, downloads, discussions, > and more. http://solutions.newsforge.com/ibmarch.tmpl > _______________________________________________ > Bluez-devel mailing list > Bluez-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bluez-devel > -- --------------------------------------------------------- Claudio Takahasi Nokia's Institute of Technology - INdT claudio.takahasi@indt.org.br ------=_Part_4061_17858956.1128429212132 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline
Hi Marcel,

Your suggestions were done!
Regarding service_table_t, if we remove it, the logic will be based on "if"and "else", see the HAL message filter function. In= my opinion service_table_t is more clear.

If you think that is really necessary remove the error handling and the ser= vice_table_t I can do it in the next patch.

Regards,
Claudio.





On 10/3/05, Marcel Holtmann <marcel@h= oltmann.org> wrote:
Hi Claudio,

> I kept some log information, if you want remove it = I can send another
> patch. The error code definition was included. I= am using "hciX" in
> the path name until we receive a answ= er from dbus list. Services under
> the path /org/bluez/Devices weren't implemented yet.
>
&g= t; Is there other changes in this patch?

this thing looks quite good= , but now we need to clean up some other
parts of your constants and fun= ction naming. And please try to keep the
BlueZ/kernel coding style.

The overall BLUEZ_DBUS prefix looks l= ike a good idea at the beginning,
but I think we don't need it at all. S= o this should be enough:

#define DEVICE_PATH    =          "/org/bluez/Device&qu= ot;
#define DEVICE_INTERFACE        = ;"org.bluez.Device"

#define MANAGER_PATH   =          "/org/bluez/Mana= ger"
#define MANAGER_INTERFACE       = "org.bluez.Manager"

Please also stop to shortcut some name= s. For example DFT for DEFAULT.
Simple write the full name, because then it is easy to read this code.<= br>
The other thing is that I don't like the difference between *_REQ an= d
*_SIG in the names/constants. Please don't use them.

Also pleas= e don't include any RFCOMM, PAN, HID etc. related things,
because they are not important right now and they make my life of
me= rging the initial step into better D-Bus a lot harder.

For the error= part we will use org.bluez.Error and not EFailed or
anything like this.= Since all the error stuff is currently not used, I
don't think it is a good idea to add it to the initial patch. Looking a= t
HAL the use of org.bluez.NoSuchDevice, etc. seems to make sense, too.<= br>
Do we really need that service_table_t thingy now? I think we should
start without it.

Regards

Marcel




-----= --------------------------------------------------
This SF.Net email is = sponsored by:
Power Architecture Resource Center: Free content, download= s, discussions,
and more. http:= //solutions.newsforge.com/ibmarch.tmpl
_____________________________= __________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/list= info/bluez-devel



-- ---------------------------------------------------------
Claudio Takaha= si
Nokia's Institute of Technology - INdT
claudio.takahasi@indt.org.br
------=_Part_4061_17858956.1128429212132-- ------=_Part_4060_6309862.1128429212132 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: attachment
Hi Marcel,

Your suggestions were done!
Regarding service_table_t, if we remove it, the logic will be based on "if"and "else", see the HAL message filter function. In my opinion service_table_t is more clear.

If you want I

Regards,
Claudio.





On 10/3/05, Marcel Holtmann <marcel@holtmann.org> wrote:
Hi Claudio,

> I kept some log information, if you want remove it I can send another
> patch. The error code definition was included. I am using "hciX" in
> the path name until we receive a answer from dbus list. Services under
> the path /org/bluez/Devices weren't implemented yet.
>
> Is there other changes in this patch?

this thing looks quite good, but now we need to clean up some other
parts of your constants and function naming. And please try to keep the
BlueZ/kernel coding style.

The overall BLUEZ_DBUS prefix looks like a good idea at the beginning,
but I think we don't need it at all. So this should be enough:

#define DEVICE_PATH             "/org/bluez/Device"
#define DEVICE_INTERFACE        "org.bluez.Device"

#define MANAGER_PATH            "/org/bluez/Manager"
#define MANAGER_INTERFACE       "org.bluez.Manager"

Please also stop to shortcut some names. For example DFT for DEFAULT.
Simple write the full name, because then it is easy to read this code.

The other thing is that I don't like the difference between *_REQ and
*_SIG in the names/constants. Please don't use them.

Also please don't include any RFCOMM, PAN, HID etc. related things,
because they are not important right now and they make my life of
merging the initial step into better D-Bus a lot harder.

For the error part we will use org.bluez.Error and not EFailed or
anything like this. Since all the error stuff is currently not used, I
don't think it is a good idea to add it to the initial patch. Looking at
HAL the use of org.bluez.NoSuchDevice, etc. seems to make sense, too.

Do we really need that service_table_t thingy now? I think we should
start without it.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel



--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
claudio.takahasi@indt.org.br
------=_Part_4060_6309862.1128429212132-- ------------------------------------------------------- This SF.Net email is sponsored by: Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl _______________________________________________ Bluez-devel mailing list Bluez-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bluez-devel