dbus_interface properties requires impl From<&Value> #258

Closed zeenix closed 1 year ago

In GitLab by @bilelmoussaoui on Jan 31, 2022, 16:34

Currently, passing a some value to the property setter requires that the property is passed by ref or you end up with having to implement impl From<&Value> for T, see the test case added in https://gitlab.freedesktop.org/dbus/zbus/-/merge_requests/463.

I believe either:

The conversion by ref should only needed when needed, maybe if the type is not a basic copy type?
Never needed as I believe most of the time you don't want to pass a ref but rather by value?

Hmm.. seems I never looked into this. :( Also, this is even worse it seems since even &MyPropertyType doesn't work (#277).

marked #277 as a duplicate of this issue

marked this issue as related to #277

mentioned in commit ce847e5160dd7252ed3bc0458e77a0e2ded60495

In GitLab by @sre on Feb 18, 2023, 21:12

Hi,

I ran into this issue (I guess). The following demo code does not run with zbus = { git = "https://gitlab.freedesktop.org/dbus/zbus", default-features = false, features = ["tokio"] }. Removing the #[dbus_interface(property)] annotations results in the code compiling fine. Looks like the issue has not been fully solved yet?

-- Sebastian

use zbus::dbus_interface;

#[derive(serde::Deserialize, serde::Serialize, zbus::zvariant::Type)]
struct Test {
    a: String,
    b: String,
}

struct Demo {
    test: Test,
}

#[dbus_interface(name = "dev.sebastianreichel.zbus-demo")]
impl Demo {
    #[dbus_interface(property)] // removing this and the following annotation fixes compilation
    async fn test(&self) -> &Test {
        &self.test
    }

    #[dbus_interface(property)]
    async fn set_test(&mut self, test: Test) {
        self.test = test;
    }
}

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    let demo = Demo {
        test: Test {
            a: "".to_string(),
            b: "".to_string(),
        },
    };

    let _connection = zbus::ConnectionBuilder::session()?
        .name("dev.sebastianreichel.zbus-demo")?
        .serve_at("/dev/sebastianreichel/zbus-demo", demo)?
        .build()
        .await?;

    std::future::pending::<()>().await;

    Ok(())
}

Removing the #[dbus_interface(property)]

That's the property getter and return value while this issue was about input arg of property setter. What you're seeing is the exact opposite. Please file a new issue for it.

In GitLab by @sre on Feb 18, 2023, 23:10

Both, the getter and the setter generated a warning for me. But I can create a new issue of course.

Then you're likely missing the conversion. Properties' types need to be converted to/from Value (or OwnedValue), unlike methods.

zvariant provides derive macros for this.

Looking at your code, that's exactly what's happening.

In GitLab by @sre on Feb 19, 2023, 02:56

:+1: it works when adding zvariant::Value derive to the Test struct. I don't think I saw this being documented anywhere. I suggest adding a sentence in the property section in https://docs.rs/zbus/latest/zbus/attr.dbus_interface.html and/or mentioning it in https://dbus.pages.freedesktop.org/zbus/server.html.

I don't think I saw this being documented anywhere. I suggest adding a sentence in the property section in https://docs.rs/zbus/latest/zbus/attr.dbus_interface.html and/or mentioning it in https://dbus.pages.freedesktop.org/zbus/server.html.

Yeah, I realized last night when I wrote the previous comment and was going to link to those docs cause I was pretty sure this fact was documented. :smile:

I'll add to my todo..

dbus2 / zbus-old

dbus_interface properties requires impl From<&Value> #258