Version 1.0.0 API stabalization

[acrisci]

I'm pretty happy with how things are right now so I think I'm going to mark all the apis of the library as stable in the next release.

[acrisci]

Actually now I'm thinking this should wait until decorators are stage 3.

[shannon]

First, just let me say that this project looks great and thanks for all your work.

I'm very new to dbus but I recently started a new project that requires communicating with the bluez dbus api. So I've found myself here, as this library looks the most modern, and feature rich.

I like to be forwarded leaning in general but especially when starting a new project and I like that this library uses modern JS paradigms. I like to see async/await, classes etc.

Having said that, the use of decorators has actually put up a fairly large barrier to entry. Currently I don't have any build step, now that Node 13 finally has ES module support. So I would hate to take a step back. Especially because the decorator spec is in perpetual flux and the end result may not actually be what is being used here (I do think the latest iteration of the proposal makes some major changes).

Personally, I'd like to see something similar to the way lit-element works with and without decorator support. You just define the properties as a static getter on the class.

So the example interface might look like this

class ExampleInterface extends Interface {
    static get properties() {
        return {
            SimpleProperty: { signature: 's', access: ACCESS_READWRITE },
            MapProperty:    { signature: 'a{sv}' },
        }
    }
    static get methods() {
        return {
            Echo:            { inSignature: 's',  outSignature: 's' },
            ReturnsMultiple: { inSignature: 'ss', outSignature: 'vv' },
            ThrowsError:     { inSignature: '',   outSignature: '' },
        }
    }
    static get signals() {
        return {
            HelloWorld:     { signature: 's' },
            SignalMultiple: { signature: 'ss' },
        }
    }

    SimpleProperty = 'foo';

    _MapProperty = {
        'foo': new Variant('s', 'bar'),
        'bat': new Variant('i', 53)
    };

    get MapProperty() {
        return this._MapProperty;
    }

    set MapProperty(value) {
        this._MapProperty = value;

        Interface.emitPropertiesChanged(this, {
            MapProperty: value
        });
    }

    Echo(what) {
        return what;
    }

    ReturnsMultiple(what, what2) {
        return [
            new Variant('s', what),
            new Variant('s', what2)
        ];
    }

    ThrowsError() {
        // the error is returned to the client
        throw new DBusError('org.test.iface.Error', 'something went wrong');
    }

    HelloWorld(value) {
        return value;
    }

    SignalMultiple(x) {
        return [
            'hello',
            'world'
        ];
    }
}

I did see the alternative method in https://github.com/dbusjs/node-dbus-next/blob/master/lib/service/interface.js#L262. So I'll be using that for now, but it seems like with the above approach the library could perform this step automatically like the decorators since it's all right there. So anyways, I just thought you might want that feedback here.

Thanks again.

[acrisci]

Yeah that does look a bit nicer than the static class method.

[Niels-Be]

So I have been using this for a while an one thing I often encounter is the Variant error (expected a Variant for value ...). E.g. a call requires an Object of Variants and I just pass in a normal JS Object.

From a usage perspective it would be nice if a signature requires a Variant but the user passed in a primitive, that the Variant is automatically created. Other libraries do that as well (see node-dbus).

Same goes for Variants in results. In my use cases 99% for the time the signature of the variant is never accessed and I only use the value. I can see that there are use cases for checking the signature of the Variant but I think the more common use case is just using the value (also JS does not care about types anyways).

So my proposal would be to remove Variants from results by default, but add an option e.g. to the Interface to return Variants. And for method calls accept both primitives and Variants.

[acrisci]

but the user passed in a primitive, that the Variant is automatically created.

The problem is there is not a one-to-one conversion between variant values and dbus types. A string can be a dbus object path or a dbus string. A number can be one of many number types. A list can be a dbus list or a struct. Guessing is made more complicated when considering you have to do it recursively. node-dbus has no way of specifying types which makes it unusable. This was actually the initial inspiration for making this library for use in mpris-service.

Same goes for Variants in results.

This is a little more workable because there is a one-to-one conversion from Variant to js type (just take the value). It makes the most sense for the properties interface because you already have type information. I wrote another dbus library that actually does unwrap those values with functions like iface.get_some_property() which I like better but can't be implemented here easily because of some namespacing issues.

I wouldn't support this for the low level interface because magic like this does not belong there. I don't think I would support this by default because 1) it's a pretty big breaking change that requires everyone to migrate their code and 2) it's conceptually a little more complicated for the results of proxy interface calls to be a special case to have all their results formatted like this. But maybe an optional parameter to the proxy interface to automatically unwrap variants would be ok. I'm open to discussion about how that might be done.

[lesha1201]

It would be nice to have an access to a message in method handler. Currently, we only have message body but sometimes you need to know, for example, who is sending the message (sender). It can be added without a breaking change by adding it as a last argument:

class ExampleInterface extends Interface {
    // ....

    @method({inSignature: 's', outSignature: 's'})
    Echo(what, message) {
        return what;
    }
    
    // Or it can be an object that contains any other additional info
    @method({inSignature: 's', outSignature: 's'})
    Echo(what, { message }) {
        return what;
    }
}

[acrisci]

@lesha1201 that's a good idea, open a new issue to discuss that.