Pelagicore / gdbus-codegen-glibmm

Code generator for C++ D-Bus stubs and proxies using Giomm/Glibmm
GNU Lesser General Public License v2.1
23 stars 25 forks source link

Minor register object improvments and add registered_object() method #78

Closed martin-ejdestig closed 5 years ago

martin-ejdestig commented 5 years ago

Started out by simplifying some code in a service by adding a bool registered_object() const method to stub instead of storing state separately and mentioning object path in error messages (instead of logging it at call site on failure). Did some other minor fixes and cleanups as well that I happened to see when looking at register_object(). See commit log for details.

Originally split the "Improve logging" commit into multiple ones but decided to merge them. Can break them up again if wanted.

mardy commented 5 years ago

I'm not sure about the "one registration per stub" limitation. In Qt, at least, I can register the same object on different object paths, if I'm not wrong. Does the previous implementation work, or is it a limitation of GDBus that one cannot register the same stub on two different paths?

Actually, the previous implementation is too restrictive: one should be able to register the same object on two different D-Bus connections having the same path (and this can be useful when you use a p2p D-Bus connection with different clients).

martin-ejdestig commented 5 years ago

I'm not sure about the "one registration per stub" limitation. In Qt, at least, I can register the same object on different object paths, if I'm not wrong.

Hum, I do not understand what you mean. It is not "one registration for that stub class".

A stub instance is supposed to map against a single object on the bus...? It does not make sense (to me at least) to have multiple objects on the bus go to the same virtual C++ methods, for D-Bus methods etc, of a single stub instance.

Does the previous implementation work, or is it a limitation of GDBus that one cannot register the same stub on two different paths?

Hum, GDBus does not know anything about a "stub".

What is not reused now between different stub instances is the Gio::DBus::NodeInfo and Gio::DBus::InterfaceVTable . We need to fix that to fix #66 though, I think. It does not make sense to share m_registeredObjectId... nor m_connection. m_interfacename can perhaps be shared somehow... but it is just a string.

Actually, the previous implementation is too restrictive: one should be able to register the same object on two different D-Bus connections having the same path (and this can be useful when you use a p2p D-Bus connection with different clients).

Just create another stub instance. Done. :)

mardy commented 5 years ago

A stub instance is supposed to map against a single object on the bus...? It does not make sense (to me at least) to have multiple objects on the bus go to the same virtual C++ methods, for D-Bus methods etc, of a single stub instance.

Probably it's not such a common case, but it does make sense. :-) You might have an object that you want to expose at two different object paths. The service could be creating multiple object trees at different path locations (maybe a tree for each different client request), and a few of these objects might actually be the same underlying instance.

Actually, the previous implementation is too restrictive: one should be able to register the same object on two different D-Bus connections having the same path (and this can be useful when you use a p2p D-Bus connection with different clients).

Just create another stub instance. Done. :)

Well, no, because you want the different clients to see the same object. You could of course create two instances, but then you'd need to invent a mechanism to mirror the object state. It's much easier if one simply derives from the stub class and keeps one stub per object instance.

martin-ejdestig commented 5 years ago

OK, I see what you mean now. Expose the exact same object on different object paths on the same bus. If different buses are wanted then different Gio::DBus::Connection:s for each bus have to be stored, I am guessing.

The stub does not work like that at all at the moment though? Would have to store a list of:

So, I do not know.... do you want to open a feature/enhancement issue about making reusing the same stub for different object paths possible?

What commits of this pr do you want me to drop?

martin-ejdestig commented 5 years ago

@mardy I changed "bool registered_object()" to "int usage_count()" which will make more sense when a stub instance can be used for multiple paths. Kept the "use id to check if already registered" commit though since it is the correct thing to do they way the stub works right now, I think. Will have to change when #81 is implemented, of course.

martin-ejdestig commented 5 years ago

@mardy Merging this now (want the usage_count() in another project). Hope you did not want anything else addressed. :)