dbus2 / zbus-old

Rust D-Bus crate.
https://gitlab.freedesktop.org/dbus/zbus
Other
49 stars 13 forks source link

Can not serialize Option<T> #110

Closed zeenix closed 1 year ago

zeenix commented 4 years ago

In GitLab by @dragonn on Oct 25, 2020, 17:26

When I try return an Option from a dbus server app I got an runtime error:

 Message creation error: Type `m(ss)` is not compatible with `D-Bus` format

The code compiles fine. Any chance to fix or workaround that?

zeenix commented 4 years ago

Yeah, this is a bit tricky. D-Bus doesn't have the concept of null/none but GVariant does. If you (or anyone) has an idea of how to resolve this at compile-time, that would be great.

Any chance to fix or workaround that?

Work around is simple:

match value {
  Some(v) => send_or_serialize_value(&v),
  None => Err(SomeError),
}

i-e don't send Option but rather the Some value.

zeenix commented 4 years ago

Perhaps it'd have made for a better API if the encoding format (D-Bus vs. GVariant) where handled through generics (like byte order is) but changing to that would be an API break. Perhaps we can think of some new generics-based API and deprecate the current API but I wouldn't have time for that anything soon.

zeenix commented 4 years ago

In GitLab by @dragonn on Oct 25, 2020, 20:28

Oh ok.. I am not sure if I understand you workaround, I also tried sending Result over it but that didn't even compile. Can you explain it a lite bit more?

About a generic API, maybe a nice idea would be to dbus_interface macro allow to select a serialize/deserialize API. The same in dbus_proxy. This way the user could decide with format to use. I for example would use bincode for that. And since the default would be what is at the current moment it would not brake any API.

BTW. A litte offtopic but that just an idea. dbus_proxy already uses a trait, it would be nice when that macro leaves that trait in place and you could use it for implementing the serve said at the same time. This way the dbus interface would be defined in on place and I would be easier to keep it consistent on both client and server said (of course when you are writing both at the same time).

zeenix commented 4 years ago

Oh ok.. I am not sure if I understand you workaround,

The work-around is not to send an Option but rather the value (if any) in it. i-e unwrap the Option and send the contents if you get any.

I also tried sending Result over it but that didn't even compile.

Cause zvariant::Type isn't implemented for Result and it shouldn't be since there is nothing it can be mapped to.

About a generic API, maybe a nice idea would be to dbus_interface macro allow to select a serialize/deserialize API. The same in dbus_proxy. This way the user could decide with format to use

That won't work cause on D-Bus level, there is no choice but to use the D-Bus format. GVariant format was supposed to be D-Bus 2.0 but that didn't happen and we ended up with GVariant as a separate format that's used mostly as an efficient simple database format.

Having said that, we do allow peer-to-peer connection and there you can use whichever format you prefer. We just need to somehow provide an API to select the format for only that type of connection.

I for example would use bincode for that

Not sure what advantage that gives? D-Bus is already pretty efficient and GVariant makes it super efficient.

BTW. A litte offtopic but that just an idea. dbus_proxy already uses a trait, it would be nice when that macro leaves that trait in place and you could use it for implementing the serve said at the same time.

I actually wanted it that way (that proxy and interface macros can share code) and pushed for it when @elmarco submitted the MR for macros. However, after a bit of discussion we decided that is not feasible. You can dig out the MR to read the discussion.

zeenix commented 4 years ago

In GitLab by @dragonn on Oct 25, 2020, 22:22

Not sure what advantage that gives? D-Bus is already pretty efficient and GVariant makes it super efficient.

Well the advantage is that it supports almost every Rust type witch easy. Of curse that would not be "standard d-bus" but in my situation I am writing the server and the client in Rust so it doesn't meter to me so much. But I understand sticking to that with should be standart.

I actually wanted it that way (that proxy and interface macros can share code) and pushed for it when @elmarco submitted the MR for macros. However, after a bit of discussion we decided that is not feasible. You can dig out the MR to read the discussion.

Well dig into that just out of curiosity, in my situation that would make my program much more "elegant" in terms of code Thanks for answering my questions. I thing I got all the answers so you can close that issue if you wish.

zeenix commented 4 years ago

Not sure what advantage that gives? D-Bus is already pretty efficient and GVariant makes it super efficient.

Well the advantage is that it supports almost every Rust type witch easy.

The same should be true for zvariant's formats as well. For types that are missing, we can either add them to zvariant (if they are generic enough/used widely, see #98) or you can implement zvariant::Type yourself (it's a very simple trait).

I actually wanted it that way (that proxy and interface macros can share code) and pushed for it when @elmarco submitted the MR for macros. However, after a bit of discussion we decided that is not feasible. You can dig out the MR to read the discussion.

Well dig into that just out of curiosity, in my situation that would make my program much more "elegant" in terms of code

I can imagine. You could also add a more specific implementation of zvariant::Type, serde::Serialize and serde::Deserialize for Option<T> (where T is your specific type) in your code.

I thing I got all the answers so you can close that issue if you wish.

I'm glad to hear. I've one question though. Would it help at all if zvariant allowed you to serialize/deserialize Option<T> for D-Bus format if the value was Some but return an error if it was None? Right now you get error for both.

zeenix commented 4 years ago

In GitLab by @dragonn on Oct 26, 2020, 13:26

I've one question though. Would it help at all if zvariant allowed you to serialize/deserialize Option<T> for D-Bus format if the value was Some but return an error if it was None? Right now you get error for both.

I don't think so, how would the client behave when I got an error? At the moment I only used busctl for testing and it just got stuck when they is no "answer" when I tried to run a function with does return an Option (that is how i got that error message from the first post).

zeenix commented 4 years ago

I've one question though. Would it help at all if zvariant allowed you to serialize/deserialize Option<T> for D-Bus format if the value was Some but return an error if it was None? Right now you get error for both.

I don't think so, how would the client behave when I got an error?

Same as you'd now but instead of manually unwrapping the Option, you'd instead handle the error after the call.

At the moment I only used busctl for testing and it just got stuck when they is no "answer" when I tried to run a function with does return an Option (that is how i got that error message from the first post).

Interesting that busctl doesn't error out on incompatible signature.

zeenix commented 4 years ago

In GitLab by @dragonn on Oct 26, 2020, 14:03

Ok. So if it errors out then I think that would help, that would allow to indicate to the client "nothing to to do, go on". I have a function called get_notification with gets if they is available a notification to handle, if not then it would be fine to return an Error instead of None. Just so my client knows that he can try to get it in some time again.

zeenix commented 4 years ago

Ok. So if it errors out then I think that would help, that would allow to indicate to the client "nothing to to do, go on"

Actually that should already be the case. What is the call that gives you the error on the service/server side?

What I proposed is that you only get an error on trying to send None to other side, the rest remains the same for the service implementation. Although thinking about this more, this will require a bit of smartness in zbus to modify the type signatures to be compliant with the spec so not sure it's worth it.

I have a function called get_notification with gets if they is available a notification to handle, if not then it would be fine to return an Error instead of None. Just so my client knows that he can try to get it in some time again.

I would suggest you to design your API around the lack of Option in D-Bus instead. An error is not a good way to do this. You could e.g either use signal instead to notify the client that there is a notification available, add another method that tells you if there is a notification available before you try to get it or use some special value to indicate None.

zeenix commented 3 years ago

mentioned in commit c5b55763321c8c277150443cc5fd980678b52ec4

zeenix commented 3 years ago

@dragonn So with !190, you get a build-time error when you try to use Option<T> with zbus.

zeenix commented 3 years ago

mentioned in commit 8ea9cb6cde4d4ff05a31ee2d41bc3b451ba1738c