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

Handle special case of std::vector<bool> #411

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

oKermorgant
Copy link

@oKermorgant oKermorgant commented Jul 7, 2023

This PR modifies the interface_factories template file and adds a special case to handle fields which are std:vector<bool>.

The current approach to stream a std::vector is to memcpy its content into/from the stream.

Unfortunately, it does not work for std::vector<bool> as there is not a single bool inside such.

In this PR the template file generates a special call when a vector<bool> is detected.

This should answer #391, #393

@Timple
Copy link
Contributor

Timple commented Jul 11, 2023

@oKermorgant You probably should signoff on your commits: https://github.com/ros2/ros1_bridge/pull/411/checks?check_run_id=14855763467

@oKermorgant
Copy link
Author

oKermorgant commented Jul 27, 2023

The last push is to handle vector<bool> that lie in services, the code was generated elsewhere.
In order to handle this I had to use index-based for-loops instead of iterators (you cannot iterate on a vector<bool> and hope to get a pointer to a bool...).

@mikaelarguedas
Copy link
Member

fixes the bool list issue for me 👍 (setup: Ubuntu 24.04, ROS 2 Jazzy / ROS Noetic)

Thanks @oKermorgant !

@logic-bomb-gmr
Copy link

logic-bomb-gmr commented Nov 6, 2024

build fine for me as well, thanks @oKermorgant.
setup: Ubuntu 22.04.5 LTS with ROS 2 Humble and ROS Noetic.

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.

4 participants