diwic / dbus-rs

D-Bus binding for the Rust language
Other
585 stars 131 forks source link

dbus client improvement #478

Open ModProg opened 1 month ago

ModProg commented 1 month ago

I admit that I'm not very experience using dbus, but I wanted to control wpa_supplicant and wondered if I could get a nicer API for through generating some “path wrappers” that I called DbusObject which have Rust representations of the actual dbus interface generated through a macro.

Allowing you to define a "dbus object": https://github.com/ModProg/dbus-client/blob/8bb8db7e6ac1b58566e69be36e92a3b329b3904e/examples/wpa.rs#L6-L74 There is some documentation in my README

And using it: https://github.com/ModProg/dbus-client/blob/8bb8db7e6ac1b58566e69be36e92a3b329b3904e/examples/wpa.rs#L148-L165. And the documentation generated from the macro: https://modprog.github.io/dbus-client/wpa/struct.WpaSupplicant.html

I do notice that this is somewhat related to the dbus code generation from the xml interface, but AFAICT the generated rust code from the xml is far from this nice to use.

Not sure if something like that would make sense in dbus or better as a separate crate.

diwic commented 1 month ago

Hi,

Is this actually the same issue as #477 ? I have a hard time figuring out the difference.

ModProg commented 1 month ago

The other ist just about derive macros same as #459, but this was about defining the whole dbus API of some path, i.e. the functions it has etc. allowing you to e.g. do:

WpaSupplicant::system()?.get_Interfaces()?.first()?.Disconnect()?;

When WpaSupplicant and Interface are defined, as in the linked examples.

This idea consists of adding the trait DbusObject that bundles a connection, path and destination similarly to Proxy, but when combined with the dbus_object!{} macro you can simply generate rust wrappers for the dbus methods and properties:

dbus_object! {
  TheObject("dbus.destination", "/debus/path")
  "dbus.interface" {
      Method(args: impl Append) -> @SomethingImplDbusObject;
      OtherMethod(args: impl Append) -> RegularArgAndGet;
      mut MutableProperty: s; // `Vec<String>` (not sure about this, I just wanted to match the dbus signature)
      UnmutableProperty: a @SomethingImplDbusObject;      
  }
}

An example for what a dbus object defined this way could look: https://modprog.github.io/dbus-client/wpa/struct.WpaSupplicant.html

But I can see how this can be confusing, because I developed both in the same repo

diwic commented 1 month ago

Umm, so I'm less sure about this one, especially the made-up syntax of the dbus_object macro that does not seem to follow any known standard (?).

I do notice that this is somewhat related to the dbus code generation from the xml interface, but AFAICT the generated rust code from the xml is far from this nice to use.

Not sure I agree with that, could you give a more complete example? Maybe it is possible to make the generated rust code nicer to use instead?

diwic commented 1 month ago

E g, one could add system() and session() functions to Proxy if that makes ergonomics easier for people who just want to talk to a single path on the other side.

ModProg commented 1 month ago

Not sure I agree with that, could you give a more complete example? Maybe it is possible to make the generated rust code nicer to use instead?

At least the XML I got doesn't contain any docs, i.e., there is no way to tell which fields prop maps have or which interface is implemented by the returned o.

Umm, so I'm less sure about this one, especially the made-up syntax of the dbus_object macro that does not seem to follow any known standard (?).

Not sure if there is any I could use, my logic for the syntax was to more or less follow rust's impl blocks + the TheObject("dbus.destination", "/debus/path") because I didn't know if there was anything else to follow.

Then I just added extra syntax to allow using dbus signatures, i.e., a {s v} and to use @ImplsDbusObject, because I couldn't just implement Get for those as they need access to the dbus connection which is not available through the Get trait.

I'd be open to implement this any different way/syntax if you have suggestions, but I mostly wanted to make mapping something like https://w1.fi/wpa_supplicant/devel/dbus.html to a “type-save” and documented rust wrapper less cumbersome.

But as my POC shows, this can be implemented as a separate crate, so I can fully get behind saying this is out of scope for dbus-rs.

diwic commented 1 month ago

As for comments in the XML file being shown in the Rust code, there was some brief discussion here but it seems to have stalled: https://github.com/diwic/dbus-rs/discussions/454 Happy to review a PR for that.

Also it is possible for the XML to use a custom type instead of the standard Rust type (with the rs.dbus.ArgType annotation) if that helps?

For me it usually does not make sense to define a fixed path for an object; as there can be many objects of the same type (i e having the same interfaces) on different paths.

I start to see what you're trying to get out of this and I can see how it's useful, but for somebody who didn't write the code, it looks like rust, dbus signature types, and custom syntax mixed in a not so obvious way.

If you end up making your own crate maybe it is possible to use the API of dbus-codegen to generate the Rust code, thus sharing some of the code? That's up to you, of course.

ModProg commented 1 month ago

As for comments in the XML file being shown in the Rust code, there was some brief discussion here but it seems to have stalled: #454 Happy to review a PR for that.

Right maybe that could be part of it, but as it is free text, it would be hard to correctly extract e.g. which interface is implemented on the return type.

Also it is possible for the XML to use a custom type instead of the standard Rust type (with the rs.dbus.ArgType annotation) if that helps?

That could work, I'd probably need to extend that to be able to specify whether the type implements regular Get or "Get with connection i.e. is a proxylike type".

For me it usually does not make sense to define a fixed path for an object; as there can be many objects of the same type (i e having the same interfaces) on different paths.

Yes, thats why those are optional in the macro I built, to allow for both objects that are usually in the same place as WpaSupplicant or objects that can be at multiple paths like Interface. The path & destination in the parentheses are optional. And the DbusObject makes the functions that depend on such a default path available through the marker traits CommonDestination and CommonPath.

I start to see what you're trying to get out of this and I can see how it's useful, but for somebody who didn't write the code, it looks like rust, dbus signature types, and custom syntax mixed in a not so obvious way.

Yeah, I totally see that, and it could make sense to either fully commit to a rust syntax or a dbus syntax, and I definitely need to write documentation for this.

If you end up making your own crate maybe it is possible to use the API of dbus-codegen to generate the Rust code, thus sharing some of the code? That's up to you, of course.

Yeah, I'd totally be up to do that, but I think the current trait based approach of dbus-codegen wouldn't work as well for this, as it doesn't really allow specifying which interfaces are related to specific paths/return values from dbus-methods. And I'd need to add something like my DbusObject i.e. the Proxy as a trait to allow creating structs that behave more or less like Proxy but can have specific Interface implementations.

ModProg commented 1 month ago

I'll try to design a better picture of what I want to achieve, and maybe try to think of a few annotations to add to the introspection XML to replace the macro.

Maybe part of the issue is I started with wpa_supplicant and its introspection XML doesn't have any documentation.