altdesktop / python-dbus-next

🚌 The next great DBus library for Python with asyncio support
https://python-dbus-next.readthedocs.io/en/latest/
MIT License
187 stars 59 forks source link

In docs: signal handlers MUST specify signature #102

Closed msftcangoblowm closed 2 years ago

msftcangoblowm commented 2 years ago

In Client Interface, signal handlers must specify the signature. This can be confirmed in proxy_object.BaseProxyInterface:_addSignal decorator.

def on_signal_fn(fn):
        fn_signature = inspect.signature(fn)
        if not callable(fn) or len(fn_signature.parameters) != len(intr_signal.args):
            raise TypeError(

So if the signal handlers doesn't contain a matching signature then a TypeError is supposed to be thrown. My experience: never saw a TypeError get thrown!

Therefore, without looking at the source code, there is no way to know why normal signals aren't being triggered.

For example,

def on_signal_simple(str_hello):
    print(f"{str_hello} world")

Will never be triggered.

def on_signal_simple(str_hello: 's'):
    print(f"{str_hello} world")

Will get triggered.

on_property_changed actually does get triggered! Even without the signature. Which only indicates that org.freedesktop.DBus.Properties.PropertyChanged signal handling is handled separately.

Please update the misleading Client Interface documentation, so signal handling is intuitive.

acrisci commented 2 years ago

This method is not intended to require annotations because the type information is in the introspection XML. Would you please investigate the root cause of the issue? A failing test would be nice. Fixing it would be better. Thanks.

msftcangoblowm commented 2 years ago

ISSUE 1

Compared:

Both pass condition len(fn_signature.parameters) != len(intr_signal.args). So i was mistaken.

ISSUE 2 (new issue)

if signal handler annotation is provided, isn't checked with the annotation of the args.

In python

In DBus

Do you want to compare annotations if annotations are provided?

In which case, i'm willing to create the fix

acrisci commented 2 years ago

The library always does this. Types are checked against the introspection XML in the high level client (second time saying htis). In the low level client, the signature of the message is always guaranteed to match the body. If this is ever not the case, you may report the bug.