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

Allow arbitrary structs in an sdbus interface #420

Closed
hellow554 opened this issue Mar 18, 2024 · 5 comments
Closed

Allow arbitrary structs in an sdbus interface #420

hellow554 opened this issue Mar 18, 2024 · 5 comments

Comments

@hellow554
Copy link
Contributor

hellow554 commented Mar 18, 2024

Is your feature request related to a problem? Please describe.
In our product we have a large amount of structs going back and forth between multiple programs.
For this to work, we have a common file between all projects that contains something like this:

using systemStatus1 = sdbus::Struct<uint8_t, uint8_t, uint8_t, uint8_t, uint8_t, uint8_t>;
using systemStatus2 = sdbus::Struct<uint32_t, uint32_t, uint32_t, uint32_t, uint8_t, uint8_t, uint8_t>;
[...]

This is of course not a great interface, because even I can't tell what these members do.

So, I tried to come up with a lot of template magic code and tried to convert a regular struct/class to a sdbus::Struct and it worked.
The question is now, are you interested in allowing such thing.

Describe the solution you'd like
I have a solution ready that currently allows to write
to_sdbus_struct(system_status_1) which converts a struct SystemStatus1 { ... } to sdbus::Struct<...> and to_cpp_struct wich converts it the other way around.
We could integrate this directly into sdbus-c++ and make automatic conversion from and to sdbus types possible

This would allow us and other people to use structs as interface description instead of the using I wrote above.

There are a few downsides:

  • floats aren't allowed in the struct, because it's not part of the dbus interface specification
  • padded or packed structs aren't possible
  • Currently uses some boost preprocessor macro magic to allow structs up to 32 (adjustable) elements to be used
  • Any struct with more members can be supported by using more compile time
  • Nested structs are not supported (yet)

Please tell me what you think.
I can provide some code examples and you tell me how you like it or not.

@sangelovic
Copy link
Collaborator

Hi Marcel, thanks for the detailed description and the proposal, and yes, this interesting and useful topic is in my backlog and there already was an issue/suggestion raised in this respect before. I started prototyping a non-intrusive user struct description solution but then got interrupted. I wanted to release v2 first, but there were still ideas coming to implement given this breaking-change major release opportunity, and limited time. I hope this is coming to its finish line.

I'll let you know once I get back to this topic and you and I can share own solutions and discuss. From quickly reading over your list of disadvantages, boost::mpl dependency is something which we most likely won't be able to accept... Let's see then.

@sangelovic
Copy link
Collaborator

sangelovic commented Apr 29, 2024

@hellow554 I got to back to this now. While reflecting on the problem and the potential solution, I realized we already have a very solid foundation for extending support for user-defined structs, as a part of generic solution for teaching sdbus-c++ about user-defined types that I introduced in August last year.

See the sdbus-c++ cookbook, section Extending sdbus-c++ type system with an example of my::Struct support. There are also unit tests showcasing this here.

Clients need to simply define the Message serialization and deserialization operator for their structs, which are simple one-liners, building on top of std::tuple, and also very efficient thanks to forward_as_tuple. Also, they need to specialize signature_of. The usual stuff when extending sdbus-c++ with custom types. They can copy the example and adjust struct name.

This will enable them to pass these structs directly to sdbus-c++ API on both server and client side, as if they were sdbus::Structs. With this I guess we have all needed for accomplish your wishes (unless I missed something).

What do you think? This would mean we don't need any extra code for conversions between structs etc. Have you tried that approach?

If higher convenience is needed, we could potentially provide one macro for structs that would define the above-mentioned needed overloads/specializations. Something like SDBUSCPP_DEFINE_STRUCT(my::Struct, i, s, l);.

@sangelovic
Copy link
Collaborator

A very nice bonus is that we automatically get support for nested structs. Struct members can have types that are other user-defined structs, or user-defined containers (like std::lists)...

Also, we could extend the XML syntax for the xml-to-cpp codegen. An attribute or a sub-element could be added under a method or signal or property element that would define that for signature (isad) please use my::Struct instead of generated sdbus::Struct<int, std::string, std::vector<double>>.

@hellow554
Copy link
Contributor Author

See the sdbus-c++ cookbook, section Extending sdbus-c++ type system with an example of my::Struct support

That section looks different than the last time I looked at it, seems to be a long time ago 🙈
That is totally fine, although some boiler code is needed, but that's fine, I guess.

TBH: I never used the xml-to-cpp so far, maybe it's time to take a look into it.

Thanks for your long and good explanation of your point of view.

@sangelovic
Copy link
Collaborator

Hey @hellow554, FYI, I introduced the SDBUSCPP_REGISTER_STRUCT macro already a few months ago, here: #440. It turns the registration of a user-defined struct into a one-liner.

Today I also opened a merge request which adds more functionality to the SDBUSCPP_REGISTER_STRUCT macro -- the ability to serialize the struct as an a{sv} dictionary and deserialize the a{sv} dictionary into a struct.

Just in case it could interest you, or help improve your code. You can also share your feedback in case something more could be added or improved :o)

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

No branches or pull requests

2 participants