Add convenience function for property change

[hellow554]

Is your feature request related to a problem? Please describe.
Currently, when I try to track the change of a property I have two options:

1. Polling: Poll the property regularly and store it into the desired variable everytime I poll it
2. Subscribing to the PropertiesChanged signal

I think we can agree, that 1. is somewhat bad. 2. is feasible, but not convenient. There's no higher level on the convenience API side.

Currently, you have to do this:

PROXY->uponSignal("PropertiesChanged")
    .onInterface("org.freedesktop.DBus.Properties")
    .call([&](const std::string                           &interface_name,
              const std::map<std::string, sdbus::Variant> &changed_properties,
              const std::vector<std::string> &) {
        if (interface_name == "myInterface") {
            for (const auto &[key, value] : changed_properties) {
                if (key == "DesiredProperty") {
                    current_state = value.get<std::string>();
                }
            }
        }
    });

I mean it works, but doing this in each and every project is somewhat cumbersome and not very user friendly.

Describe the solution you'd like
I would like to see something like this added on top of the convenience API

PROXY->onPropertyChange("DesiredProperty")
    .onInterface("myInterface")
    .call([&](const std::string &changed_property) {
        current_state = changed_property;
    });

Of course, std::string is interchangeable, just like other functions accept anything that a sdbus::Variant can be coerced to.
In the background sdbus-c++ should listen on the PropertiesChanged signal on said interface and call the function, when the property changes.

Additional context
I'm happy to implement a working draft and discuss the future of this feature request with you.

[hellow554]

Upon further inspection, there is the Properties_proxy.
But by using it, suddenly you have to worry about classes and store them somewhere, which is not what I want to achieve.
Also, it has the same weird (not disrespectful) API that the current solution has, which means I have to check for interfaces and property names.

I wanted to achieve some abstraction.
One could argue, that you can achieve my goal by extending the Properties_proxy class and implementing the things I want to achieve in that class, but that's not what I want.
I want a direct method on the IProxy class that I can work with. Is that somewhat understandable?

[sangelovic]

@hellow554

I get your point. Properties_proxy is implemented so as to precisely mirror the org.freedesktop.DBus.Properties interface as specified in D-Bus specification. PropertiesChanged signal has all those parameters -- this is not an invention of sdbus-c++, but 1:1 implementation of these standard D-Bus interfaces. It has to be that way.

I would agree with extending IProxy interface with the API you suggested. A few remarks / questions:

• Will we implement a slot-returning variant or a floating slot variant (see overloads with return_slot_t tag)? I think we could implement both like we do in maybe all other similar cases in sdbus-c++. The floating slot variant could reuse the same collection for slots as used by generic signals: https://github.com/Kistler-Group/sdbus-cpp/blob/master/src/Proxy.cpp#L185.
• Actually, thinking of it, the PropertyChangeSubscriber or however the new helper class in ConvenienceClasses.h/.inl would be called would be a simple wrapper around existing IProxy::uponSignal()... or IProxy::registerSignalHander(), doing the searching of a property and casting it from Variant to the desired type. So what we would need to change in IProxy would be just to add a non-virtual uponPropertyChange(), in similar pattern as IProxy::uponSignal() is defined. Benefits:
  • No need to care about where to store floating slots; the functionality is re-used.
  • Nice levels of abstraction and re-use
  • Adding non-virtual method doesn't change ABI of IProxy, so we are backwards compatible.
• Will we handle changed properties, or also invalidated properties? Both types are part of PropertiesChanged signal. Invalidated properties do not carry value in the PropertiesChanged signal. If we support both, will we have a separate IProxy.uponInvalidatedProperty()? This would mean that the client must know in advance whether the property he is seeking to observe is annotated with org.freedesktop.DBus.Property.EmitsChangedSignal as either true or as invalidates. I would think there are cases when he doesn't know that and just wants to know if a property changed, and to see its value if it can be provided, or not see its value if the property is marked as invalidates. So I would tend to have only one API for both, and to differentiate, could we use std::optional<Type> as the parameter of the call method? (Like this, taken from your example: .call([&](const std::optional<std::string> &changed_property). I think std::optional here would idiomatically express here that the value may or may not be present here.

What are you thoughts? Will you prepare a PR?

[hellow554]

Hey Stanislav,
Thank you so much for your Response.
I'm glad to see that you are open for the implementation of this idea. That makes me really happy.

this is not an invention of sdbus-c++, but 1:1 implementation of these standard D-Bus interfaces. It has to be that way.

Yeah, I know. There was no intention of being disrespectful.

Will we implement a slot-returning variant or a floating slot variant ?

I think both, Returning a slot and a non slot variant would be good. Although my applications are all long running and I don't care about the slots at all 🙈 but I see the need and use case for it.

See, I tried to implement a draft to see if it just possible to implement this idea and I haven't thought about refusing the signal handler at all. That's a brilliant idea. That makes things a lot easier, I guess.

Will we handle changed properties, or also invalidated properties

That's a good question. I never cared about invalidated signals, but I do see a lot of ways to use them in my application. Thanks for bringing up ideas 😁

I agree that having a unified API is the way to go. Also using an optional is the right type for this.
What I'm being afraid of is that by using an optional I have a lot of code to repeat over and over again, because the first thing in every callback would to see if the optional holds a value.
Maybe we can implement the API that it either accepts and optional in case the user wants to care for invaliditet properties or only a type im case the user does not care.
For the case that the user does not care and the prop is invalidated I see at least two options:

1. An exception is thrown -- my least preferred option
2. The invalidition is ignored and the callback is not called

As for implementing this idea: I'm currently on Parental leave and am taking care of my son. Therefore I have no to very little time to do things on the computer, sadly.

So I would not be upset if you do the hard work 😇 but also if you don't have time or capacity I think I would be able to do some work at the end of the year.

[sangelovic]

Hi Marcel. Congratulations to having a son! I, too, became father just 9 months ago. Less time for hobby stuff on my side, too ;-)

Regarding optional -- I don't think it will complicate your code in your case? I mean -- if you are sure in your implementation that the properties you're watching are "open", then you just can simply dereference the optional. Just like you dereference a pointer if it's an invariant in your code that the pointer is always valid. (Of course, best is to document your invariants with e.g. an assert, so you could have assert(changed_property);). I don't think this is a lot of extra code?

So you still have a choice -- you can dereference right away (so no extra code at all), or you can check if unsure (with some extra checking and handling).

However, I realized optional may not be a good solution at all. A signal can carry multiple values, leading to callback with multiple parameters. Each one would be optional? It doesn't make sense for each one. It's a universal, single information -- are property values present or not. I remembered that sdbus-c++ already has similar solution in some async callbacks (e.g. here: https://github.com/Kistler-Group/sdbus-cpp/blob/master/include/sdbus-c%2B%2B/ConvenienceApiClasses.inl#L622). The first parameter of those callbacks is std::optional<Error>, and this is used to convey error information to the client, and if that is set then the other value parameters are not valid. Throwing an exception doesn't help, because it doesn't reach the client code, it bubbles up to the event loop processing inside sdbus-c++ and stops there. Also, I don't like simply ignoring the callback -- this may easily confuse clients and cause bugs. I also don't like simply calling the callback with default constructed values in case of invalidated properties. I think we shall carry the information somehow whether the values are present or not.

Or we simply split it into two ways of subscribing if unifying them brings unnecessary complication -- we can have uponChangedProperty and uponInvalidatedProperty. At least it would be clear and the behavior well defined.

What do you think? Would such a separate handling be OK for you? Or we introduce an additional (optional or required) parameter for these propertychanged callbacks?

Regarding implementation -- if you would very much like to have a creating programming time and do it, I'll wait even until end of year, I don't hurry with it. If it's not super interesting or a priority to you for coming months, then I'll implement it myself. I have no problem with either ;-)

[sangelovic]

Sorry Marcel for the confusion. I realized I had confused signal values for property values. Of course, a property, unlike signal, can only contain a single value of a single type. So just discard much of my elaboration above 🙂 optional is back in the game.