search

tmds / Tmds.DBus

D-Bus for .NET
MIT License
283 stars 53 forks source link

It's currently not possible to represent a variant value wrapped into another variant value. #282

Open kekekeks opened 5 months ago

kekekeks commented 5 months ago

E. g. something like this:

$ qdbus --literal org.freedesktop.portal.Desktop /org/freedesktop/portal/desktop org.freedesktop.portal.Settings.Read org.freedesktop.appearance color-scheme
[Variant: [Variant(uint): 2]]

It's read as VariantValue with VariantValueType.Int32, however it's not possible to construct such variant object to pass it to dbus.

tmds commented 5 months ago

It's read as VariantValue with VariantValueType.Int32, however it's not possible to construct such variant object to pass it to dbus.

VariantValue is not meant for writing.

The library has separate types for reading and writing variants (VariantValue and Variant) to enable optimizing for the reading and the writing use-cases separately.

For the VariantValue reading that means it collapses inner variants to improve the reading UX and reduce allocations.

The use-case for which these types don't work is when you read a variant from D-Bus and you want to send that value back over D-Bus. Is that your use-case?

If this is not your use-case, you should update your code so it expects the VariantValue to be unwrapped.

affederaffe commented 5 months ago

No, the issue is for reading VariantValues like the value of the call mentioned above. In my opinion, we should prefer spec conformability over speed. Wrapped Variants are already pretty rare as is, one boxing allocation to store the inner VariantVlaue in the _o field shouldn't be a game changer in terms of performance.

tmds commented 5 months ago

The type is meant for the use case of reading the value of a variant. For that use-case, not unwrapping nested variants means pushing the unwrap to the user. The implemented behavior serves both the UX and performance.

Wrapped Variants are already pretty rare as is

It's not uncommon for a variant to contain a dictionary of string to variant.

I'm going to close this as the behavior is intentional.

ash-microconnect commented 3 weeks ago

The use-case for which these types don't work is when you read a variant from D-Bus and you want to send that value back over D-Bus. Is that your use-case?

If this is not your use-case, you should update your code so it expects the VariantValue to be unwrapped.

Sorry to resurrect an old issue, but I do have the "read then write" back use case mentioned and am getting blocked by the two flavours of Variant and VariantValue. Specifically, I am getting data from org.freedesktop.NetworkManager.Settings.Connection.GetSettings and wanting to write back (modified) values using org.freedesktop.NetworkManager.Settings.Connection.Update. The signature for the data in both cases is a{sa{sv}}. Do you have a suggestion for translating the data from VariantValue to Variant, bearing in mind that the "inner" variant could be complex (e.g., array of dict, etc.)?

tmds commented 2 weeks ago

a{sa{sv}}

Doesn't this nested dictionary structure enable you to pass in only the values your app wants to update?

ash-microconnect commented 2 weeks ago

No, at least not according to their docs:

Update (IN a{sa{sv}} properties);

Update the connection with new settings and properties (replacing all previous settings and properties) and save the connection to disk.

(I've tried to follow the NM source code to see how they handle this DBus message in order to confirm the behaviour, but find the code quite impenetrable.)

tmds commented 2 weeks ago

No, at least not according to their docs:

Give it a try.

(I've tried to follow the NM source code to see how they handle this DBus message in order to confirm the behaviour, but find the code quite impenetrable.)

I think you are looking for nm_connection_update_secrets.

Currently the library doesn't allow reading a variant as an opaque value to be written back. I've opened an issue for it: https://github.com/tmds/Tmds.DBus/issues/303.

ash-microconnect commented 2 weeks ago

I can confirm by experimentation that the NM docs are accurate - existing properties are not retained if omitted from the dictionary during an update.

tmds commented 1 week ago

Re-opening for adding support to VariantValue for nested variants. This is needed to enable such nested variants to be written back to the bus (https://github.com/tmds/Tmds.DBus/issues/303).

The goal is to track the nesting in VariantValue while keeping the UX(/performance) mostly unchanged. That is: when reading a dictionary of string to variant or an array of variant, the user should not have to unwrap those variants. Ideally, the only breaking change that is introduced would be when variants are stored in variants. This should be an unlikely scenario.