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

Build Failure Corrected for develop branch #549

Closed
wants to merge 1 commit into from

Conversation

suab321321
Copy link

Signed-off-by: AbhinavSingh [email protected]

@minggangw
Copy link
Member

Would you please offer a detailed description of the issue you want to fix and what's the expected result? I notice the travis-ci shows no error currently (https://travis-ci.org/RobotWebTools/rclnodejs)

@minggangw minggangw self-requested a review January 15, 2020 14:00
@suab321321
Copy link
Author

@minggangw I was unable to build my rclnodejs from scratch.Whenever I was doing npm install it showed me error in two files executor.cpp and rcl_bindings.cpp(in both files there two functions which were taking more than required arguments)so I just corrected those.Opened this PR.

@suab321321
Copy link
Author

@minggangw remember in this issue umbrella issue #498 I offered to help but my rclnodejs was not building,I even showed you the errors.So yesterday I was trying to solve it again and somehow it got installed successfully so I thought it wil be good if I open this PR

Copy link
Member

@minggangw minggangw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR itself has conflicts with the developer branch currently, and I suspect that the ROS2 you installed is an old release instead of the latest one. So please double-check the ROS2 for either the binary package or the one built from scratch, or you can find some useful information here: https://github.com/RobotWebTools/rclnodejs#match-with-ros-20-stable-releases Thx!

@@ -96,8 +96,7 @@ void Executor::Run(void* arg) {

try {
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 2, 0, 0, 0, 0,
executor->context_, rcl_get_default_allocator());
rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 2, 0, 0, 0, rcl_get_default_allocator());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the rcl_wait_set_init function has the parameters as follows https://github.com/ros2/rcl/blob/master/rcl/include/rcl/wait.h#L124

@@ -119,9 +118,9 @@ void Executor::Run(void* arg) {
0u,
handle_manager->timer_count(),
handle_manager->client_count(),
handle_manager->service_count(),
handle_manager->service_count()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -390,7 +390,7 @@ NAN_METHOD(RclTake) {
rcl_subscription_t* subscription =
reinterpret_cast<rcl_subscription_t*>(subscription_handle->ptr());
void* msg_taken = node::Buffer::Data(info[1]->ToObject());
rcl_ret_t ret = rcl_take(subscription, msg_taken, nullptr, nullptr);
rcl_ret_t ret = rcl_take(subscription, msg_taken, nullptr);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

@suab321321 suab321321 Jan 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@minggangw yes you are right the function paramters are changed.I m using crsystal version of ros2 installed from binary.Sorry should have checked before opening PR

@suab321321 suab321321 closed this Jan 15, 2020
@minggangw
Copy link
Member

Never mind, hope you can run on the ROS2 with rclnoejs smoothly!

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.

2 participants