Make bus_name optional for proxy object

[jameshilliard]

Some applications send signals without well defined senders, as such the bus_name parameter needs to be made optional in order for us to receive these messages.

Fixes #20, #80

[acrisci]

What exactly are you doing with this?

[jameshilliard]

What exactly are you doing with this?

Seems to be needed for getting cups spooler signals:

CUPS_SPOOLER_NODE = Node(None,
                 is_root=True,
                 interfaces=[
                     Interface('com.redhat.PrinterSpooler',
                               signals=[
                                   Signal('PrinterAdded',
                                          args=[
                                              Arg('s', ArgDirection.OUT, 'name')
                                          ]),
                                   Signal('PrinterRemoved',
                                          args=[
                                              Arg('s', ArgDirection.OUT, 'name')
                                          ]),
                                   Signal('QueueChanged',
                                          args=[
                                              Arg('s', ArgDirection.OUT, 'name')
                                          ]),
                                   Signal('JobQueuedLocal',
                                          args=[
                                              Arg('s', ArgDirection.OUT, 'name'),
                                              Arg('u', ArgDirection.OUT, 'id'),
                                              Arg('s', ArgDirection.OUT, 'username')
                                          ]),
                                   Signal('JobStartedLocal',
                                          args=[
                                              Arg('s', ArgDirection.OUT, 'name'),
                                              Arg('u', ArgDirection.OUT, 'id'),
                                              Arg('s', ArgDirection.OUT, 'username')
                                          ])
                               ])
                 ])

async def cups_register_spooler():
        bus = await MessageBus(bus_type=BusType.SYSTEM).connect()
        introspection = CUPS_SPOOLER_NODE
        obj = bus.get_proxy_object(None, '/com/redhat/PrinterSpooler', introspection)
        notifier = obj.get_interface('com.redhat.PrinterSpooler')

        notifier.on_printer_added(printer_added)
        notifier.on_printer_removed(printer_removed)
        notifier.on_queue_changed(printer_queue_changed)
        notifier.on_job_queued_local(printer_job_queued_local)
        notifier.on_job_started_local(printer_job_started_local)

        logger.info("handler connected")

The messages look like this:

signal time=1637787503.992144 sender=:1.26 -> destination=(null destination) serial=13 path=/com/redhat/PrinterSpooler; interface=com.redhat.PrinterSpooler; member=PrinterAdded
   string "npx"
signal time=1637787549.450888 sender=:1.30 -> destination=(null destination) serial=5 path=/com/redhat/PrinterSpooler; interface=com.redhat.PrinterSpooler; member=PrinterRemoved
   string "npx"
signal time=1637788046.334966 sender=:1.16 -> destination=(null destination) serial=30 path=/com/redhat/PrinterSpooler; interface=com.redhat.PrinterSpooler; member=QueueChanged
   string "npx"
signal time=1637788118.253921 sender=:1.16 -> destination=(null destination) serial=35 path=/com/redhat/PrinterSpooler; interface=com.redhat.PrinterSpooler; member=JobQueuedLocal
   string "npx"
   uint32 1
   string "root"
signal time=1637788118.274079 sender=:1.16 -> destination=(null destination) serial=37 path=/com/redhat/PrinterSpooler; interface=com.redhat.PrinterSpooler; member=JobStartedLocal
   string "npx"
   uint32 1
   string "root"

[acrisci]

In all of these cases, the address of the sender is defined. They are :1.26, :1.30, and :1.16. The ProxyObject is intended to be used for a single object at a single address or any address that takes a well-known bus name. Lifting this assumption works for signals but breaks every other feature of the ProxyObject such as method calls.

I don't believe this fixes the issue with the peer-to-peer bus connections. I think this is unrelated to those issues.

I'd like for you to consider using the low level interface for this use case instead. Add a match rule on the path and interface you would like and then listen to the messages you get and route them to the handler. I think that would be a bit cleaner than this.

[jameshilliard]

In all of these cases, the address of the sender is defined. They are :1.26, :1.30, and :1.16.

Yeah, but they are temporary addresses(used to fire signals and disconnect AFAIU) and a usable bus_name is not defined for them either.

The ProxyObject is intended to be used for a single object at a single address or any address that takes a well-known bus name.

Hmm, it seems to work for these signals with this change, what would the correct high level interface be for receiving these sort of signals?

Lifting this assumption works for signals but breaks every other feature of the ProxyObject such as method calls.

True, I added some checks to make sure method and property calls without a bus_name throw proper error messages.

I don't believe this fixes the issue with the peer-to-peer bus connections. I think this is unrelated to those issues.

Making the bus_name optional seemed to be what they needed I thought.

I'd like for you to consider using the low level interface for this use case instead. Add a match rule on the path and interface you would like and then listen to the messages you get and route them to the handler. I think that would be a bit cleaner than this.

I didn't see a good way to do that with the low level interface as a lot of the required functionality is in the high level proxy_object interface.

[jameshilliard]

Here's what the name assignment looks like:

method call time=1638162961.855147 sender=:1.44 -> destination=org.freedesktop.DBus serial=1 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=Hello
method return time=1638162961.855198 sender=org.freedesktop.DBus -> destination=:1.44 serial=1 reply_serial=1
   string ":1.44"
signal time=1638162961.855228 sender=org.freedesktop.DBus -> destination=(null destination) serial=57 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameOwnerChanged
   string ":1.44"
   string ""
   string ":1.44"
signal time=1638162961.855261 sender=org.freedesktop.DBus -> destination=:1.44 serial=2 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameAcquired
   string ":1.44"

[acrisci]

It would look something like this

bus = await MessageBus().connect()

add_match_msg = Message(destination='org.freedesktop.DBus',
                        path='/org/freedesktop/DBus',
                        interface='org.freedesktop.DBus',
                        member='AddMatch',
                        signature='s',
                        body=['interface=com.redhat.PrinterSpooler'])

await bus.call(add_match_msg)

def message_handler(message):
    if message.member == 'PrinterAdded':
        on_printer_added(*message.body)
    elif message.member == 'PrinterRemoved':
        on_printer_removed(*message.body)

bus.add_message_handler(message_handler)
await bus.wait_for_disconnect()

[jameshilliard]

It would look something like this

Hmm, what would the high level equivalent for that be?

Isn't the purpose of the high level client to avoid having to manually write message handlers like that?

The way I see it used in dbus-python seems somewhat similar to the high level interface in python-dbus-next although the style is a bit different:

bus.add_signal_receiver (self.handle_dbus_signal,
                         path=self.DBUS_PATH,
                         dbus_interface=self.DBUS_IFACE)

I don't see any manual AddMatch messages calls to do this(I assume the python-dbus library handles this logic all internally) like proxy object here does.

[acrisci]

Hmm, what would the high level equivalent for that be?

There's no equivalent for this in the high level client and I don't see this as a problem. The low level client is meant to be used when you want to deal with the whole bus. The high level client is only designed for working with particular objects.

Isn't the purpose of the high level client to avoid having to manually write message handlers like that?

Yes, the high level client will do that for you, but there are complexity trade offs. When comparing the two ways of doing it, I think the low level version is more expressive of what you are doing. It will be easier to maintain the low level code because less is hidden within the library.

[jameshilliard]

The low level client is meant to be used when you want to deal with the whole bus. The high level client is only designed for working with particular objects.

That's what I thought, since we don't want to deal with the whole bus I didn't want to use the low level message handler client. We're not interested in the whole bus, only the specific object interface signals.

Yes, the high level client will do that for you, but there are complexity trade offs. When comparing the two ways of doing it, I think the low level version is more expressive of what you are doing. It will be easier to maintain the low level code because less is hidden within the library.

I mean, doing this using the low level code is def more complex to maintain on the application side of things, maybe we need a more purpose specific high level client interface here for receiving signals than the normal proxy object interface? The regular python-dbus signal receiving API is a good bit higher level than the low level client here.

[acrisci]

I mean, doing this using the low level code is def more complex to maintain on the application side of things

I guess we'll have to agree to disagree. I think the low level code is better than the high level code with the proposed change.

maybe we need a more purpose specific high level client interface here for receiving signals than the normal proxy object interface?

I think what you are doing is very specific to your application. You may use the low level code to build the abstractions you need. I don't want to maintain any feature that isn't obviously useful to a typical user.

[jameshilliard]

I think the low level code is better than the high level code with the proposed change.

This signal matching feature could probably be added there, I'll give that a shot.

I think what you are doing is very specific to your application. You may use the low level code to build the abstractions you need. I don't want to maintain any feature that isn't obviously useful to a typical user.

So this is functionality commonly used in dbus-python via add_signal_receiver but yeah it's done differently from this as it doesn't appear to use the proxy object I'll see if I can add this add_signal_receiver feature to the low level interface in that case.

[acrisci]

You're going in the right direction with going about how to justify your feature, but just because something is in dbus-python doesn't necessarily mean I want it in this library. Make an issue to discuss the design and approach and we will come to a consensus on if this feature is worth the maintenance cost I will incur and how it can be done in the best way.

[jameshilliard]

@acrisci I ended up doing some preliminary experimentation in #105 with an inverted proxy object mapper, it doesn't yet add any bus_name optional signal receivers but it refactors the way mappings are handled.