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

Add some additional methods to Message #287

Closed
wants to merge 2 commits into from
Closed

Conversation

ids1024
Copy link
Contributor

@ids1024 ids1024 commented Oct 14, 2020

No description provided.

@diwic
Copy link
Owner

diwic commented Oct 15, 2020

Hi and thanks for the PR!
Usually I just pull a PR that just adds methods, but this one had me wondering. Mostly because you're more likely to misuse these particular ones rather than use them, according to my gut feeling. What is your actual use case for these methods?

@ids1024
Copy link
Contributor Author

ids1024 commented Oct 15, 2020

I'm trying to send a response to a method call in a separate thread (with the same Arc<SyncConnection>), and it can't use .method_return() or .error() since Message can't be cloned to send to the other thread.

I thought about implementing Clone for Message, but looking at the documentation for dbus_message_copy, that seems too problematic.

(Perhaps porting the code in question to use dbus-tokio would allow dealing this this sort of thing better...)

@diwic
Copy link
Owner

diwic commented Oct 16, 2020

Hmm...I'm not following, but this does not seem to be the right solution to your problem. Send is implemented for Message, but I don't understand why you need to create a clone to send (instead of sending the original)?

@ids1024
Copy link
Contributor Author

ids1024 commented Oct 19, 2020

The code was using dbus::tree. The method handlers for that are passed just an &Message, then I was having it return an empty list of messages to send, but spawn a thread that will send the reply on the same connection.

I don't think there's technically anything wrong with that, but it's definitely a bit of an ugly hack. I've since started porting the old code to use dbus-tokio and dbus-crossroads, which should be cleaner.

@diwic
Copy link
Owner

diwic commented Oct 21, 2020

Given your reply I believe it would be okay to close this PR without merging it.

It could possibly be useful to have two methods, maybe error_reply_from_serial(reply_serial, name, message_text) -> Message and reply_from_serial(reply_serial) -> Message because it is easier to send and copy a reply_serial than a full message.

@diwic diwic closed this Oct 21, 2020
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

2 participants