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

Fix DBus conversions #185

Open
TTWNO opened this issue May 21, 2024 · 4 comments · May be fixed by #206
Open

Fix DBus conversions #185

TTWNO opened this issue May 21, 2024 · 4 comments · May be fixed by #206

Comments

@TTWNO
Copy link
Member

TTWNO commented May 21, 2024

DBus/zbus Problems

Ok, so I keep getting bugged by having two functions that appear to do similar things on the surface.
Namely, converting from a zbus::Message to a specific event type.

Current Situation

Right now, in the main branch, this occurs in two steps:

  1. An implementation of the TryFrom<&zbus::Message>
    • This takes a Message reference, and extracts the three required parts to be processed by step two:
      • The object path() and application as a sender(), collectively refered to as an ObjectRef.
      • A body type that is unique to every event.
    • If either of these properties cannot be extracted, then it causes an error.
  2. An implemantation of the BusProperties trait: from_message_parts
    • This takes those said "parts" refered to above and finally creates the event from said parts.
    • This can technically fail, but is unlikely to since (in theory) the message has been routed to the appropriate from_message_parts implementaiton based on if the message matches the correct member(), interface(), and body signature.
    • It should be noted that this method is often redundant since most event types actually only need the ObjectRef and do not even need a deserialized body of any kind.
      • One does not simply use a default implementation! Since this actually constructs a specific type, there isn't any easy way to blanket implement this.
      • This can also be phrased as: "a trait implementaiton cannot construct Self, since it depends on certain fields existing in the struct"

Luuk's PR

Luuk has an approach which simplifies the public API.

  1. A MessageExt trait which can find out if it matches the event, before deserializing: matches_event<E: BusProperties>(Message) -> bool
    • This uses the methods of BusProperties to determine if the message maches the event's DBus interface, member, and body signature.
  2. An implementation of the TryFrom<&zbus::Message>
    • This will use step one to determine if it matches the event type.
    • If step one returns true, then step three is attempted.
    • If step one returns false, then find out why.
  3. An implemantation of the BusProperties trait: from_message
    • This takes the Message and converts it to the specific event type; it takes advantage of TryFrom<&Message> to fill each field with few expressions.
    • It may fail, but usually will not because it will be checked for its type in step one.

This adds an extra step, but adds a convenient API for checking if an event type is matched by a particular Message.

The Problem

All of these are extra steps to what in theory is one operation: to fallibly convert a Message into a specific event type.
The reason we are in this mess is because I have been strongly against writing the boilerplate to do this manually for each event.
So, let's break down the operations that are not unique to one event type and how we may abstract that away.

  1. Checking if the BusProperties of an event match that of the Message. Luuk's implementaiton, which adds this via MessageExt::matches_event<E: BusProperties>(&Message) -> bool is one way to do this.
  2. Extracting various field types. This has been done since long ago via TryFrom<&Message>.
  3. Constructing simple events, which do not extract the body of the event.
    • This could be done with the following trait, implemented via a macro_rules! macro.

    • Something like

       pub trait NoBodyEvent {
       	fn from_object_ref(obj: ObjectRef) -> Self;
       }
    • This should allow a blanket implementation for TryFrom<&Message> for T: NoBodyEvent.

    • TODO: naming better

  4. Auto TryFrom; if the individual fields in a given struct are able themselves to be TryFrom<T>'d, then why not the entire struct?
    • This would work wonders for boilerplate, at the expense of introducitng another proceduaral macro.
    • Additionally, it looks like we'd have to implement it ourselves. Annoyingly, derive more does not have it. Since worse than adding a proc macro is adding a insignificantly used crate that may have little to no maintainership.

Ideally, we want to be able to do this with certain constraints; we would not want to:

  • Comparing or deserializing twice; for example, by checking a signature, then attempting to extract that Body's shape from the Message.
  • Without allocating and then immediately throwing away the data; for example, deserializing, getting an error (which may allocate a string), then immediately using .is_ok() and dropping the data.
  • Without making the atspi-common crate non-optionally depending on zbus.
    • Relavent only because zbus::Message is not re-exported by another crate; it is defined in zbus itself.
    • zbus will not currently compile on wasm32-* architectures.
    • This is due to a downstream goal of wanting Odilia to be able to use WASM in its future addon system, and atspi events are crucal in this mission.
    • Therefore Feature flagging anything mentioning Message will be necessary.

This proves somewhat challenging, no matter the order of operation, or what the public API is.

Potential Solution

Please add possible solutions as comments.

@luukvanderduim
Copy link
Collaborator

Thanks for writing this down. Good starting point for what might be a description of our architecture, something I have wanted to guide choices.

I don't want to muddy the water, the above is complicated enough, however in my mind's eye we offer two API's:

  1. The enum-polymorphism path is able to receive any Signal and converts it into an Event enum.
  2. The Message path. Users who find themselves having Messages can use what we offer to convert to user facing types.

@TTWNO
Copy link
Member Author

TTWNO commented May 23, 2024

Not sure I understand the first part. I don't think there is a Signal type?

The second part is true though.

@luukvanderduim
Copy link
Collaborator

No, there is no Rust Signal type. Signals are the broadcasted messages on DBus we call events.

@TTWNO
Copy link
Member Author

TTWNO commented Jun 27, 2024

New idea, we add UFET::from_parts_unchecked, this will be done with a trait, obviously.

This will construct the event without checking the interface/method combo. This allows us to write:

impl TryFrom<Message> for Event {
    match ... {
        ("interface", "member") => MyUEFT::from_parts_unchecked(obj_ref, body)?,
        // ...
    }
}

It can still fail, and it will still return a Result (the body/kind deserialization may fail after all), but this avoids checking the same items twice (one of our current problems). Now the zbus::Body is a type, we can accept a specific type into this function (and BusProperties::from_parts(...)) without generics.

Not sure if you can see my edits, but I re-invented ::try_from_parts but with the Body type instead of a generic associated type, lol!

@TTWNO TTWNO linked a pull request Jun 28, 2024 that will close this issue
2 tasks
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 a pull request may close this issue.

2 participants