I'm resending this patch since I haven't gotten a response in the original
thread in weeks. Let me know if this looks OK.
Chris
Hi Bastien,
> > > This would require defining GObject properties and get and set
> > > operations. Also, the register function is what actually initiates
> > > the connection to the bus and requests the bus name. This sort of
> > > operation might fail and shouldn't be done in an object constructor.
> > > The register function seemed like the simplest solution.
> >
> > I did it the way I think it should be done to make it a lot simpler. It
> > works, but I am not sure if it is the best way. Feel free to improve it.
> > The stuff is in the CVS now.
>
> Chris is right, object creation should never fail.
then we should not use an object for the singleton support. The overhead
for an application to get it right is too much. The singleton support
should be as less intrusive as possible.
So maybe something like bluetooth_instance_init(...) and a corresponding
bluetooth_instance_cleanup() would be better.
This is purely from the application standpoint. If it internally uses an
BluetoothInstance object, I don't really care.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
On Tue, 2007-12-18 at 20:58 +0100, Marcel Holtmann wrote:
> Hi Chris,
>
> > patch looks good to me. One question though, why do we use the
> > extra
> > register function. Can't we include that in the _new function.
> > This
> > would make the code a little simpler. We still have to check
> > if the
> > object creation succeeded.
> >
> > This would require defining GObject properties and get and set
> > operations. Also, the register function is what actually initiates
> > the connection to the bus and requests the bus name. This sort of
> > operation might fail and shouldn't be done in an object constructor.
> > The register function seemed like the simplest solution.
>
> I did it the way I think it should be done to make it a lot simpler. It
> works, but I am not sure if it is the best way. Feel free to improve it.
> The stuff is in the CVS now.
Chris is right, object creation should never fail.
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Hi Chris,
> patch looks good to me. One question though, why do we use the
> extra
> register function. Can't we include that in the _new function.
> This
> would make the code a little simpler. We still have to check
> if the
> object creation succeeded.
>
> This would require defining GObject properties and get and set
> operations. Also, the register function is what actually initiates
> the connection to the bus and requests the bus name. This sort of
> operation might fail and shouldn't be done in an object constructor.
> The register function seemed like the simplest solution.
I did it the way I think it should be done to make it a lot simpler. It
works, but I am not sure if it is the best way. Feel free to improve it.
The stuff is in the CVS now.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
On Dec 18, 2007 2:10 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Chris,
>
> > Updated patch attached.
>
> patch looks good to me. One question though, why do we use the extra
> register function. Can't we include that in the _new function. This
> would make the code a little simpler. We still have to check if the
> object creation succeeded.
>
>
This would require defining GObject properties and get and set operations.
Also, the register function is what actually initiates the connection to the
bus and requests the bus name. This sort of operation might fail and
shouldn't be done in an object constructor. The register function seemed
like the simplest solution.
Chris
Hi Chris,
> Updated patch attached.
patch looks good to me. One question though, why do we use the extra
register function. Can't we include that in the _new function. This
would make the code a little simpler. We still have to check if the
object creation succeeded.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Updated patch attached.
Chris
On Dec 17, 2007 4:01 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Chris,
>
> > Please use use gtk_init_with_args and make the GOptionEntry
> > data global
> > and static. Look at bluetooth-analyzer for an example.
> >
> > This would force the singleton flag variable to be global. Is this
> > what you want?
>
> what is the difference? It is fine to have it global. I don't see any
> problem with it. I don't wanna introduce any complexity if it is not
> needed.
>
> Regards
>
> Marcel
>
>
>
> -------------------------------------------------------------------------
> SF.Net email is sponsored by:
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services
> for just about anything Open Source.
>
> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
> _______________________________________________
> Bluez-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>
Hi Chris,
> Please use use gtk_init_with_args and make the GOptionEntry
> data global
> and static. Look at bluetooth-analyzer for an example.
>
> This would force the singleton flag variable to be global. Is this
> what you want?
what is the difference? It is fine to have it global. I don't see any
problem with it. I don't wanna introduce any complexity if it is not
needed.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
On Dec 17, 2007 3:17 PM, Marcel Holtmann <[email protected]> wrote:
>
> I think that is simply too complicated. If we use --singleton, it all
> has to come into place. And if D-Bus fails, the application is not good
> anyway. So I don't see the real benefit. If at any time later it will be
> needed, we can introduce it then.
>
Agreed.
>
> Please use use gtk_init_with_args and make the GOptionEntry data global
> and static. Look at bluetooth-analyzer for an example.
>
This would force the singleton flag variable to be global. Is this what you
want?
>
> Also don't translate config options and errors that are printed on the
> command line. We only translate stuff inside the UI. The normal user
> won't see these anyway.
> <https://lists.sourceforge.net/lists/listinfo/bluez-devel>
>
OK.
Chris
Hi Chris,
> I wanted to distinguish between an instance already running and actual
> DBUS errors. If an instance is already running then we can exit with
> 0 since the intended action was successful (the launched application
> is now in the foreground). If, however, there are errors connecting
> to the bus then we exit with 1. It isn't a big deal though. I've
> attached an updated apps patch that gets rid of the retval parameter.
I think that is simply too complicated. If we use --singleton, it all
has to come into place. And if D-Bus fails, the application is not good
anyway. So I don't see the real benefit. If at any time later it will be
needed, we can introduce it then.
Please use use gtk_init_with_args and make the GOptionEntry data global
and static. Look at bluetooth-analyzer for an example.
Also don't translate config options and errors that are printed on the
command line. We only translate stuff inside the UI. The normal user
won't see these anyway.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
On Dec 17, 2007 1:53 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Chris,
>
> > Updated patches attached.
>
> I added the generic singleton instance patch to the CVS. I did some
> minor modifications. The interface name is org.bluez.Instance. No need
> to duplicate the word Bluetooth here. And I removed the retval parameter
> from the register function. It is useless. Use the return value for it.
> If it is TRUE, everything is okay. If FALSE, cleanup and exit(1).
>
>
>
I wanted to distinguish between an instance already running and actual DBUS
errors. If an instance is already running then we can exit with 0 since the
intended action was successful (the launched application is now in the
foreground). If, however, there are errors connecting to the bus then we
exit with 1. It isn't a big deal though. I've attached an updated apps
patch that gets rid of the retval parameter.
Chris
Hi Chris,
> Updated patches attached.
I added the generic singleton instance patch to the CVS. I did some
minor modifications. The interface name is org.bluez.Instance. No need
to duplicate the word Bluetooth here. And I removed the retval parameter
from the register function. It is useless. Use the return value for it.
If it is TRUE, everything is okay. If FALSE, cleanup and exit(1).
Please update the application patches and resend them. And double check
the whitespace stuff. I had to fix some of them in your other patch. Use
an editor that has a visual representation of tabs and whitespaces. It
really helps.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Updated patches attached.
Chris
On Dec 17, 2007 12:47 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Chris,
>
> > Attached is an updated patch with the changes that we talked about.
>
> the patch looks good, but using bluetooth_application_instance is too
> long for my taste. Use bluetooth_instance to make this shorter.
>
> Also use org.bluez.properties etc. for the service names
> and /org/bluez/properties etc. for the object path.
>
> I also like to have multiple separate patches. One that add the generic
> instance code and then additional patches that adds support for it to
> every application. This makes it a lot easier for me to review and apply
> it.
>
> Regards
>
> Marcel
>
>
>
> -------------------------------------------------------------------------
> SF.Net email is sponsored by:
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services
> for just about anything Open Source.
>
> http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
> _______________________________________________
> Bluez-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>
Hi Chris,
> Attached is an updated patch with the changes that we talked about.
the patch looks good, but using bluetooth_application_instance is too
long for my taste. Use bluetooth_instance to make this shorter.
Also use org.bluez.properties etc. for the service names
and /org/bluez/properties etc. for the object path.
I also like to have multiple separate patches. One that add the generic
instance code and then additional patches that adds support for it to
every application. This makes it a lot easier for me to review and apply
it.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Anyone? Anyone?
On Dec 6, 2007 12:59 PM, Chris Rivera <[email protected]> wrote:
> Attached is an updated patch with the changes that we talked about.
>
> Chris
>
>
> On Dec 3, 2007 11:59 AM, Marcel Holtmann <[email protected]> wrote:
>
> > Hi Chris,
> >
> > >
> > > I meant to look into it, but never got around it. Some small
> > > comments
> > > about it. Don't make the applet a singleton. That is totally
> > > unneeded
> > > since the applet will be loaded at login.
> > >
> > > I still think it should be a singleton. You wouldn't ever want two
> > > applets running unless you're testing, and we have the singleton flags
> > > for that. I don't think this hurts anything.
> >
> > I am okay with that. Please abstract everything into common/. And don't
> > forget to document it in the manual pages.
> >
> > > For the "Present" method. Don't use the D-Bus low-level calls.
> > > It should
> > > be all dbus-glib. Which means we have to abstract that into an
> >
> > > object. I
> > > don't know if there is a well defined way for this. If it is,
> > > it might
> > > be good to use that. If not, then propose something for
> > > freedesktop.org.
> > >
> > > I thought about doing this, but it seems like overkill to define a
> > > GObject to just expose one method. GObject isn't exactly terse
> > > either.
> >
> > In a simple way you can use GObject for it. It is better than using the
> > low-level D-Bus calls.
> >
> > > And we should probably have some generic methods inside
> > > common/ instead
> > > of doing it again in every program.
> > >
> > > Please follow the kernel coding style. I know it is odd for a
> > > GTK
> > > application, but it makes it a lot easier for me.
> > >
> > > Which parts of the patch violate the kernel coding style? The
> > > function parameter spacing?
> >
> > Mainly the spacing.
> >
> >
>
Attached is an updated patch with the changes that we talked about.
Chris
On Dec 3, 2007 11:59 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Chris,
>
> >
> > I meant to look into it, but never got around it. Some small
> > comments
> > about it. Don't make the applet a singleton. That is totally
> > unneeded
> > since the applet will be loaded at login.
> >
> > I still think it should be a singleton. You wouldn't ever want two
> > applets running unless you're testing, and we have the singleton flags
> > for that. I don't think this hurts anything.
>
> I am okay with that. Please abstract everything into common/. And don't
> forget to document it in the manual pages.
>
> > For the "Present" method. Don't use the D-Bus low-level calls.
> > It should
> > be all dbus-glib. Which means we have to abstract that into an
> > object. I
> > don't know if there is a well defined way for this. If it is,
> > it might
> > be good to use that. If not, then propose something for
> > freedesktop.org.
> >
> > I thought about doing this, but it seems like overkill to define a
> > GObject to just expose one method. GObject isn't exactly terse
> > either.
>
> In a simple way you can use GObject for it. It is better than using the
> low-level D-Bus calls.
>
> > And we should probably have some generic methods inside
> > common/ instead
> > of doing it again in every program.
> >
> > Please follow the kernel coding style. I know it is odd for a
> > GTK
> > application, but it makes it a lot easier for me.
> >
> > Which parts of the patch violate the kernel coding style? The
> > function parameter spacing?
>
> Mainly the spacing.
>
>
Hi Chris,
>
> I meant to look into it, but never got around it. Some small
> comments
> about it. Don't make the applet a singleton. That is totally
> unneeded
> since the applet will be loaded at login.
>
> I still think it should be a singleton. You wouldn't ever want two
> applets running unless you're testing, and we have the singleton flags
> for that. I don't think this hurts anything.
I am okay with that. Please abstract everything into common/. And don't
forget to document it in the manual pages.
> For the "Present" method. Don't use the D-Bus low-level calls.
> It should
> be all dbus-glib. Which means we have to abstract that into an
> object. I
> don't know if there is a well defined way for this. If it is,
> it might
> be good to use that. If not, then propose something for
> freedesktop.org.
>
> I thought about doing this, but it seems like overkill to define a
> GObject to just expose one method. GObject isn't exactly terse
> either.
In a simple way you can use GObject for it. It is better than using the
low-level D-Bus calls.
> And we should probably have some generic methods inside
> common/ instead
> of doing it again in every program.
>
> Please follow the kernel coding style. I know it is odd for a
> GTK
> application, but it makes it a lot easier for me.
>
> Which parts of the patch violate the kernel coding style? The
> function parameter spacing?
Mainly the spacing.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
On Dec 3, 2007 11:25 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Chris,
>
> I meant to look into it, but never got around it. Some small comments
> about it. Don't make the applet a singleton. That is totally unneeded
> since the applet will be loaded at login.
I still think it should be a singleton. You wouldn't ever want two applets
running unless you're testing, and we have the singleton flags for that. I
don't think this hurts anything.
> For the well known names use org.bluez.properties and org.bluez.wizard
> and don't define constants for it. Simply use the string.
That's fine.
>
> For the "Present" method. Don't use the D-Bus low-level calls. It should
> be all dbus-glib. Which means we have to abstract that into an object. I
> don't know if there is a well defined way for this. If it is, it might
> be good to use that. If not, then propose something for freedesktop.org.
I thought about doing this, but it seems like overkill to define a GObject
to just expose one method. GObject isn't exactly terse either.
>
>
> And we should probably have some generic methods inside common/ instead
> of doing it again in every program.
>
> Please follow the kernel coding style. I know it is odd for a GTK
> application, but it makes it a lot easier for me.
> <https://lists.sourceforge.net/lists/listinfo/bluez-devel>
Which parts of the patch violate the kernel coding style? The function
parameter spacing?
Chris
Hi Chris,
> I'm resending this patch since I haven't gotten a response in the
> original thread in weeks. Let me know if this looks OK.
I meant to look into it, but never got around it. Some small comments
about it. Don't make the applet a singleton. That is totally unneeded
since the applet will be loaded at login.
For the well known names use org.bluez.properties and org.bluez.wizard
and don't define constants for it. Simply use the string.
For the "Present" method. Don't use the D-Bus low-level calls. It should
be all dbus-glib. Which means we have to abstract that into an object. I
don't know if there is a well defined way for this. If it is, it might
be good to use that. If not, then propose something for freedesktop.org.
And we should probably have some generic methods inside common/ instead
of doing it again in every program.
Please follow the kernel coding style. I know it is odd for a GTK
application, but it makes it a lot easier for me.
Regards
Marcel
-------------------------------------------------------------------------
SF.Net email is sponsored by: The Future of Linux Business White Paper
from Novell. From the desktop to the data center, Linux is going
mainstream. Let it simplify your IT future.
http://altfarm.mediaplex.com/ad/ck/8857-50307-18918-4
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel