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

wb_robot_cleanup fix for python plugins #781

Merged
merged 6 commits into from
Jul 7, 2023

Conversation

angel-ayala
Copy link
Contributor

trying to solve #476 by adding a stop() method in python plugins

@angel-ayala angel-ayala requested a review from ygoumaz June 20, 2023 20:20
@angel-ayala
Copy link
Contributor Author

I was able to handle the SIGINT inside the drive, however the ros2_supervisor.py process cannot end properly, present an exit code 1.

Copy link
Contributor

@ygoumaz ygoumaz left a comment

Choose a reason for hiding this comment

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

Very nice! Thank you for the contribution. Could you please add an entry in both related changelogs?

I will further investigate if we can terminate cleanly the supervisor node too.

webots_ros2_driver/src/PythonPlugin.cpp Outdated Show resolved Hide resolved
@ygoumaz
Copy link
Contributor

ygoumaz commented Jun 23, 2023

Have you tried to run some examples on Iron distribution (I am not sure if the following error is also on Humble), because I found a problem in examples that don't use Python plugins, namely webots_ros2_tiago.

If I start it:

ros2 launch webots_ros2_tiago robot_launch.py 

and then interrupt it with Ctrl+C, I get the following logs:

[webots_controller_Tiago_Lite-5] Warning: class_loader.ClassLoader: SEVERE WARNING!!! Attempting to unload library while objects created by this loader exist in the heap! You should delete your objects before attempting to unload the library or destroying the ClassLoader. The library will NOT be unloaded.
[webots_controller_Tiago_Lite-5]          at line 127 in ./src/class_loader.cpp
[webots_controller_Tiago_Lite-5] Warning: class_loader.ClassLoader: SEVERE WARNING!!! Attempting to unload library while objects created by this loader exist in the heap! You should delete your objects before attempting to unload the library or destroying the ClassLoader. The library will NOT be unloaded.
[webots_controller_Tiago_Lite-5]          at line 127 in ./src/class_loader.cpp
[webots_controller_Tiago_Lite-5] [ros2run]: Segmentation fault
[ERROR] [webots_controller_Tiago_Lite-5]: process has died [...]

It is not related to the changes of this PR but I wonder if we should not try to fix this issue at the same time. What do you think?

@angel-ayala
Copy link
Contributor Author

I have not been able to test it in Iron, I will run it in Humble to see if it also happens.

@angel-ayala
Copy link
Contributor Author

angel-ayala commented Jun 23, 2023

I was checking, that maybe the problem is related to the return added in WebotsNode.step()

int WebotsNode::step() {

inside the condition when the signal is captured, I'm returning -1 without calling wb_robot_step (code diff)

if (gShutdownSignalReceived && !mWaitingForUrdfRobotToBeRemoved) {
  mRemoveUrdfRobotPublisher->publish(mRemoveUrdfRobotMessage);
  mWaitingForUrdfRobotToBeRemoved = true;
  return -1
}

once the SIGINT is received, should it call wb_robot_step? this is the only changed behavior that affects both Python and C++ plugins, what do you think @ygoumaz?

Anyway, I will check it out

@ygoumaz
Copy link
Contributor

ygoumaz commented Jun 26, 2023

This error also occurs in the master branch, so it doesn't seem to be related to the changes of this PR. We can already merge this to fix the termination of Python plugins and I will open an issue for C++ plugins on Iron.

@ygoumaz ygoumaz modified the milestones: 2023.1.0, 2023.1.1 Jun 26, 2023
@ygoumaz
Copy link
Contributor

ygoumaz commented Jul 6, 2023

@angel-ayala, can you update the changelogs, so that we can merge this PR before releasing 2023.1.1?

@angel-ayala
Copy link
Contributor Author

yeah sure, I was finishing some issues with my project's code. I will update this today.

Copy link
Contributor

@ygoumaz ygoumaz left a comment

Choose a reason for hiding this comment

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

Thank you!

@angel-ayala angel-ayala merged commit fa0c82e into cyberbotics:master Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants