search

facontidavide / ros_type_introspection

Deserialize ROS messages that are unknown at compilation time
MIT License
61 stars 30 forks source link

Master branch has less features than the ros-kinetic package #7

Closed mehditlili closed 7 years ago

mehditlili commented 7 years ago

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?

facontidavide commented 7 years ago

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

facontidavide commented 7 years ago

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.

mehditlili commented 7 years ago

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.

facontidavide commented 7 years ago

Send the PR, I will be happy to review it

facontidavide commented 7 years ago

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.

https://github.com/facontidavide/ros_type_introspection/pull/11

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