Skip to content

Comments

Refactor simppl::dbus::Any#37

Draft
COM8 wants to merge 40 commits intomartinhaefner:masterfrom
AP-Sensing:any2_encoder
Draft

Refactor simppl::dbus::Any#37
COM8 wants to merge 40 commits intomartinhaefner:masterfrom
AP-Sensing:any2_encoder

Conversation

@COM8
Copy link
Contributor

@COM8 COM8 commented Sep 19, 2023

This PR addresses #36 by adding support for sending a received simppl::dbus::Any.

How do we archive this?

Once we receive something, we convert it to an intermediate representation aka IntermediateAny{Vec, Map, Tuple, ...} utilizing std::any. Then when the user requests the data, we perform the proper type conversion.

Does this change the simppl::dbus::Any interface and does it break existing implementations?

No.

Why wasn't this possible with the current implementation?

The live time of the dbus message iterator was broken.

Wouldn't it be easier to simply fix the live time of the iterator?

Yes, but have fun doing that :) while creating a copy of it.

Things that need to be done

  • Fix structs inside simppl::dbus::Any

@martinhaefner
Copy link
Owner

I finally had a look inside your implementation. Going over the intermediate representation is a good workaround for the switch from compile time to runtime polymorphism. I tried to stick to compile time only, as the whole library tries to do so and it reduces one further copy of the data. The drawback of my approach is that I have to keep a reference to the original message's data stream - and this was a kind of hack, you probably recognized how I got to the pointer.

Are you willing to provide a solution for structs? The struct is nothing other than a tuple, so maybe the solution could be straight forward?

@COM8
Copy link
Contributor Author

COM8 commented Sep 24, 2023

Thanks for taking a look at it! Yes, I will solve the struct thing. Just needs a bit more template magic. Sadly this will have to wait for ~2 weeks or so while I have to focus on other parts at work.

@GermanAizek
Copy link
Contributor

@COM8 its cool idea

@COM8
Copy link
Contributor Author

COM8 commented Oct 16, 2023

A quick update from my side. I'm still fighting with a bit of std::enable_if magic.

@martinhaefner
Copy link
Owner

Thanks for all your work. I finally had a look into your changes and found two issues:

  • had to remove the bits/utility.h header include from any.h in order to compile on gcc 11.4
  • the properties unittest did not work since there I used an enumeration as key type. So I had to add one layer of type resolution for the is<T> and as<T> functions (quick and ugly) because otherwise the following condition in the check() function fails:
return any.type() == typeid(T);

If you write it as follows

return any.type() == typeid(typename get_underlying_type<T>::type);

with

template<typename T>
struct get_underlying_type
{
	// enum to int mapping is simppl specific
	typedef typename std::conditional<std::is_enum<T>::value, int, T>::type type;
};

it works.

The same applies for the convert() function where I patched the any_cast to

return (T)std::any_cast<typename get_underlying_type<T>::type>(any);

@COM8
Copy link
Contributor Author

COM8 commented Dec 30, 2023

@martinhaefner thanks for taking a look at it! I added your suggestions.

Sadly I'm failing right now in converting structs in a generic way to dbus tupels. I'm able to obtain the dbus type from the struct but converting fails. Sadly I do not understand how you are doing it. Do you map the struct memory to a tuple memory? Since there is nothing like reflection in C++ I need a few pointers from your side, how you solved it before.

@martinhaefner
Copy link
Owner

martinhaefner commented Jan 2, 2024

Thanks for your work.

I must admit, that the struct serialization is a kind of a hack and error-prone if not used adequate. Therefore, I have also added the boost fusion version which always works the correct way, which is yet another way to (de-)serialize DBus structures.

Now, what am I doing: having a pod struct, i.e. a struct without virtual functions, the memory layout of the struct is the same as for the recursive struct I generate via the make_serializer_type<> template function. Then, you can just cast the struct's type to the serializer's type. The generated serializer's type is a list of recursive types with corresponding data which you can easily traverse with template functions. An example:

struct A
{
   int i;
   double d;
};

A a{42, 3.1415);

typedef SerializerTuple<SerializerTuple<int, simppl::NilType>, std::string> adequate_list_type;
adequate_list_type* lt = (adequate_list_type*)&a;

The SerializerTuple is a recursively inheriting structure, with each layer of inheritence holding one member of the struct in the data_ member variable.

My access functions always recurse into the list, but it should be possible to access the data in the same way as for tuples via index, assuming some more template magic.

So, the internal representation may be the same as for tuples, you just have to walk over the data in a different way.

@COM8
Copy link
Contributor Author

COM8 commented Feb 12, 2025

I'm sorry but I tried it now multiple times and failed multiple times to add struct support. I'm just not smart enough for all those template magic all around struct to tuple conversion.

@martinhaefner
Copy link
Owner

Ok, let me have a look into the code. But give me some time, I did not do much template programming in the last time :-(

What I will try to do is to extract the types back from the serializer_type and then build a new typelist that contains the code for populating an IntermediateAnyTuple.

As a next step we have to integrate the boost version of struct serialization, but that should be easier I think.

@COM8
Copy link
Contributor Author

COM8 commented Feb 17, 2025

Awesome, thanks!
The boost stuff is something I can have a look into.
Also feel free to let me know in case I can do something.

Once the struct stuff is done, I will take care of the PR to bring it past the last mile.

@martinhaefner
Copy link
Owner

I have created a new branch any2_encoder in my repository from your pull request. Converting structs from and to intermediate state works now (see comment in test). But I did not really check if this also works recursively. I think, this should be a good starting point for further work.

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 this pull request may close these issues.

3 participants