Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[nemo-qml-plugin-dbus] add QDBusVariant type to typedCall marshaling #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CODeRUS
Copy link

@CODeRUS CODeRUS commented Feb 13, 2015

No description provided.

@monich
Copy link
Member

monich commented Feb 13, 2015

What's 'z'? Can't find it in any spec.

@CODeRUS
Copy link
Author

CODeRUS commented Feb 13, 2015

Actually i don't know what single letter to assign for this type

@deztructor
Copy link

QDbusVariant is just a superset of QVariant? Do you need a specialization for it?

@CODeRUS
Copy link
Author

CODeRUS commented Feb 13, 2015

I wouldn't make a PR if it will work with just QVariant. QDBusVariant is necessary to have.

@deztructor
Copy link

@CODeRUS Can you explain any use case?

@CODeRUS
Copy link
Author

CODeRUS commented Feb 13, 2015

    DBusInterface {
        id: mceRequestIface
        service: 'com.nokia.mce'
        path: '/com/nokia/mce/request'
        iface: 'com.nokia.mce.request'
        bus: DBus.SystemBus

        function setValue(key, value) {
            typedCall('set_config', [{"type":"s", "value":key}, {"type":"z", "value":value}])
        }

        function getValue(key) {
            typedCall('get_config', [{"type":"s", "value":key}], function (value) {
                var temp = values
                temp[key] = value
                values = temp
            })
        }

        Component.onCompleted: {
            getValue(key_threshold_value)
            getValue(key_powersave_enable)
            getValue(key_powersave_force)
        }
    }

sending set_config with anything except QDBusVariant is not allowed

@monich
Copy link
Member

monich commented Feb 13, 2015

Should the 'v' case be updated instead of inventing a new letter?

@spiiroin
Copy link
Contributor

To me the whole function feels wrong. Instead of assuming random js variable can be returned
as qvariant and then pushed to dbus message in sensible way - why not pass appendable dbus args thingy + the js value and return true/false depending on whether the conversion could be made.

@CODeRUS
Copy link
Author

CODeRUS commented Feb 13, 2015

@monich can be if it don't break anything using QVariant

@thp
Copy link
Contributor

thp commented Feb 13, 2015

How does this relate to #30 ? Maybe we can solve the issue with either of these fixes?

@CODeRUS
Copy link
Author

CODeRUS commented Feb 13, 2015

@thp it's not related. Just both PR is about using QDBusVariant from qml side. One is for receiving QDBusVariant value and mine is for sending QDBuaVariant

@spiiroin
Copy link
Contributor

After 2nd look... The existing 'v' case does not guarantee that dbus variant gets used = is broken. The 'z' one by coderus does -> drop the non-standard 'z' and use the logic to handle the 'v' case?

@thp
Copy link
Contributor

thp commented Apr 7, 2015

This needs updating if we want to merge it.

@thp
Copy link
Contributor

thp commented May 7, 2015

@CODeRUS Do you think we can get this rebase against the master and merged?

@spiiroin
Copy link
Contributor

spiiroin commented May 7, 2015

This should not be needed anymore since the variant handling got eventually fixed in #30

@CODeRUS: Do you agree that this pull req can be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants