Refactor to use rcl #207
Refactor to use rcl #207
Conversation
jacquelinekay
force-pushed
the
use_rcl
branch
3 times, most recently
from
5cf7b32
to
1154d0d
Compare
…tions and add to waitset
| get_client_handle() const; | ||
|
|
||
| virtual std::shared_ptr<void> create_response() = 0; | ||
| virtual std::shared_ptr<rmw_request_id_t> create_request_header() = 0; | ||
| virtual void handle_response( | ||
| std::shared_ptr<rmw_request_id_t> request_header, std::shared_ptr<void> response) = 0; | ||
|
|
||
| private: | ||
| protected: |
There was a problem hiding this comment.
Any rational here? making members protected is possibly the right thing to do, but it's not clear why you decided to do this.
There was a problem hiding this comment.
I decided to push some member variables that are not templated but need to be accessible by the child class into the parent class.
|
welp. Lots of regressions since the last time I tested this. Looking into it |
|
Well the code review looks ok to me (other than comments). I'll try to help you look into the regressions. |
|
One last round for good measure |
|
I had to ifdef out the custom allocators in rcl because the way I did it was causing every Windows test to segfault. I also I had to fix some problems with inheritance in timers on Windows. The multithreaded tests don't pass on Windows on my vm. I'm going to run CI to see what the status is but I'm tempted to just merge this so that I can unblock @wjwwood and then I can work on fixing up the technical debt in parallel. http://ci.ros2.org/job/ci_linux/1214/ |
|
Another round with the atomics fix for Windows (this fixes the multithreaded tests locally for me): ros2/rcl#49 |
|
OMG that last CI job has 18 failures, which is as many as the nightly Windows release job!! woohoo |
|
Are they the same ones :)? |
|
The changes lgtm. Anything else need to be done to merge this? |
| g_is_interrupted.store(false); | ||
| rmw_ret_t status = rmw_init(); |
There was a problem hiding this comment.
The call to rmw_init was removed but not replaces with any equivalent. As a result the rmw init function is not being called anymore. See discussion on the mailing list: https://groups.google.com/forum/#!topic/ros-sig-ng-ros/jr6x3vWy4ls
There was a problem hiding this comment.
I believe rcl_init is being called one line below. I believe the problem is that rcl_init does not call rmw_init, not that rclcpp does not call rmw_init.
There was a problem hiding this comment.
rcl_init does not call rmw_init, I'll open a pull request for this:
However, there is no rmw_shutdown right now and rmw_init is generally not used, so I think I probably didn't use it because I knew we had to change some things about how init/shutdown worked with the rmw interface anyways. I'm not sure it should be merged in it's asymmetric state, or if we should address this when we fix the rmw init/shutdown code.
It is dangerous to keep type index float when do SET_RESIZE. It is possible that index greater than new size. Thus, set index to 0. Signed-off-by: jwang <jing.j.wang@intel.com>
TODO: