facontidavide/ros_type_introspection

Master branch has less features than the ros-kinetic package

mehditlili opened this issue · 5 comments

I wanted to do some changes to the source code and switched from using the deb package to master. However, my ros node depending on ros_type_introspection did not compile anymore.
I took a closer look and noticed that the renamed_values vector doesn't exist anymore in the ROSTypeFlat structure. The class VarNumber doesn't overload the operator double() anymore and buildRosFlatType now requires a fifth parameter max_array_size.
Is there a reason for those changes?

With the new sync, master will be the same as kinetic. I changed the API in a non compatible way. If you look at the unit tests, you will see how the new API is used.
The most noteworthy difference is that renamed_value is a separate vector with use std::string instead of SString.
This resulted in a considerable speed improvement, especially if you reuse the vector multiple times to avoid memory allocation

The operator double () was removed because there are situations in which it causes numerical errors. I want the user to use the method convert() to be more aware about what is going on.

Thanks, it is more clear now. I have done some work on my side to make access to data simpler for my use case (one vector for all data, including strings), not sure about the efficiency though. As it is changing the API again, I won't do a pull request. Or I'll maybe put it in a different feature branch if you want to review it.

Send the PR, I will be happy to review it

Hi @mehditlili,

this is one of those bad/good news.

good news is that based on your suggestions, and the one of another user, I am preparing a PR for the 1.0 version that will introduce the changes we discussed.

#11

The bad news is that the API will change, but adapting your code should take you only 5 minutes.