fix #1474 move bag encryption plugins into separate library#1499
fix #1474 move bag encryption plugins into separate library#1499dirk-thomas merged 3 commits intoros:melodic-develfrom
Conversation
This setup is required by the class_loader, which is part of the pluginlib, see http://wiki.ros.org/class_loader#Caution_of_Linking_Directly_Against_Plugin_Libraries
|
One check failed because the unit test |
|
For us at the autonomos group on the Dahlem Center for Machine Learning and Robotics this proposed fix works well. With it we are able to record and play back our bag files using ROS Melodic. |
|
I'm also using this fix at Createc to enable our recording and playback nodelet's under Melodic. I've not discovered any issues. |
|
@ros-pull-request-builder retest this please |
|
@dirk-thomas, any chance this PR could be merged and integrated for Melodic? It's been open for 3 months and multiple people confirmed that it works. By the way, the "enhancement" label is a misclassification. This is not an enhancement, but a fix for a regression / bug in rosbag-storage. In my case the bug prevents multiple projects from actually working in Melodic, and was the biggest headache for our switch from Kinetic to Melodic. Please let us know if there's anything we can do to assist in merging the pull request. |
|
Unfortunately I won't be able to look at this (and other ROS 1 tickets) before January. |
|
Time flies, happy new year, @dirk-thomas :-) |
|
Hi @dirk-thomas. Just a friendly reminder that this bug is still bothering (apparently quite a few) people that are using the rosbag API in nodelets. Please let me know if there's anything that I could do to assist in merging the pull request. |
|
Just to let everybody know that we are still using this pull-request on a regular basis as otherwise we can not use the rosbag API in a nodelet :-( It would be appreciated if somebody could accept the PR so this bug can finally be resolved. |
|
This seems pretty harmless to me, and seems to work in our setup as well. I'll merge in 24h if there are no objections. |
|
The reason I hesitated to merge this earlier is that the patch does break API since it removed public API from a header. Though I guess it is not likely that external code uses the moved classes and if it does it should be fairly easy to address by changing an include. Thanks for the patch. |
|
Thank you!! |
) * fix ros#1474 move bag encryption plugins into separate library This setup is required by the class_loader, which is part of the pluginlib, see http://wiki.ros.org/class_loader#Caution_of_Linking_Directly_Against_Plugin_Libraries * fix unit tests of aes_encryptor
|
Please see #1687 which fixes a regression introduces with this patch. |
Cherry picks ros#1499
) * fix ros#1474 move bag encryption plugins into separate library This setup is required by the class_loader, which is part of the pluginlib, see http://wiki.ros.org/class_loader#Caution_of_Linking_Directly_Against_Plugin_Libraries * fix unit tests of aes_encryptor
|
How and when will this fix find its way into the standard Ubuntu Melodic packages? |
|
Just a ping to find out what the plan is for this PR to hit updated packages on apt server? Encountered this same issue integrating rosbag reading into an RVIZ panel today. Can confirm that compiling this version of rosbag_storage from source fixes the issue. |
The change is already on the |
Moves the bag encryption plugins into a separate library, i.e. keep it out of
librosbag_storage.soThis setup is required by pluginlib's
class_loader, see http://wiki.ros.org/class_loader#Caution_of_Linking_Directly_Against_Plugin_LibrariesThis fixes #1474 which was caused by
librosbag_storage.socontaining directly linked code (likerosbag:bag) and the encryption plugins. This setup is unsupported by class_loader. As described in the link, theclass_loadertries to make it work anyway for backward compatibility by guessing where the factory object for the plugin was loaded from.When using nodelets (which are plugins itself), the
class_loaderwas guessing wrong and assigned the factory objects for the encryption plugins to the nodelet's.solibrary. This causedcreateInstanceto fail because, according toencryptor_plugins.xml, the factory object should come fromlibrosbag_storage.so