Skip to content

Conversation

bash
Copy link

@bash bash commented Oct 2, 2025

Sadly all constructors are generated manually:

  • new/new_future and new_for_bus/new_for_bus_future were not generated at all by gir (the *_future versions were generated, but I don't know how to set only the callback version to manually generated)
  • new_sync and new_for_bus_sync were generated mostly fine. I only had to fix how the cancellable param is passed to ffi:
    - cancellable.to_glib_none().0,
    + cancellable.map(|p| p.as_ref()).to_glib_none().0,
    This looks like a bug in gir because for other methods with a cancellable the .map(...) part is added just fine.

@bash bash force-pushed the gio-dbus-object-manager-client branch from 66daac1 to 9585bff Compare October 4, 2025 19:41
@bash
Copy link
Author

bash commented Oct 4, 2025

I noticed that the implementation of GDBusObjectManagerClient uses a mutex internally, so I marked it as send+sync in Gir.toml.

Sadly the docs don't mention the MT safety like they do for other types such as DBusProxy, so I'm not entirely sure 😅 :

A GDBusProxy instance can be used from multiple threads [...]

@bilelmoussaoui
Copy link
Member

You can update the gir submodule & regen, that should get rid of some of the manual code i hope :)

@bash
Copy link
Author

bash commented Oct 4, 2025

Wohoo that worked like a charm ✨

flags: DBusObjectManagerClientFlags,
name: &str,
object_path: &str,
get_proxy_type_func: Option<DBusProxyTypeFn>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not useful API. You can't construct a None very easily. It would be better to have two variants of this function, one with callback and one without

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no, None is easy because you have it already defined as Box<dyn>. But the case where a callback is provided is not very convenient now as it requires wrapping in Some() and Box::new().

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah dang it, I changed the async versions already to use two variants but forgot to do the same for the sync and the async+callback ones. Fixed in 23dac07 and 9728dcf

Copy link
Member

@sdroege sdroege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me otherwise. Do you also want to provide a small example making use of this?

@sdroege sdroege requested a review from bilelmoussaoui October 7, 2025 05:55
@bash
Copy link
Author

bash commented Oct 7, 2025

Do you also want to provide a small example making use of this?

Yes, my plan was to make DBusProxy subclassable and then do an example with UDisks (because that's what I am using it for right now ^^).

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.

3 participants