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

[Hybrid Planning] WIP: Address review #718

Closed

Conversation

sjahr
Copy link
Contributor

@sjahr sjahr commented Oct 3, 2021

Description

Address reviews of #488

Comment on lines 71 to 84
# Install libraries
install(TARGETS ${LIBRARIES}
EXPORT export_${PROJECT_NAME}
ARCHIVE DESTINATION lib
LIBRARY DESTINATION lib
RUNTIME DESTINATION lib/${PROJECT_NAME}
RUNTIME DESTINATION bin
INCLUDES DESTINATION include)

# Install binary
install(TARGETS hybrid_planning_test_node
ARCHIVE DESTINATION lib
LIBRARY DESTINATION lib
RUNTIME DESTINATION bin
DESTINATION lib/${PROJECT_NAME}
)

Copy link
Member

Choose a reason for hiding this comment

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

You can convert this into one install command like this (@JafarAbdi showed me this):

install(
  TARGETS 
    ${LIBRARIES}
    hybrid_planning_test_node
  EXPORT ${PROJECT_NAME}Targets
  ARCHIVE DESTINATION lib
  LIBRARY DESTINATION lib
  RUNTIME DESTINATION lib/${PROJECT_NAME}
  INCLUDES DESTINATION include
)

install(DIRECTORY ${THIS_PACKAGE_INCLUDE_DIRS} DESTINATION include)

install(DIRECTORY test/launch DESTINATION share/${PROJECT_NAME})
install(DIRECTORY test/config DESTINATION share/${PROJECT_NAME})

ament_export_include_directories(include)
ament_export_libraries(export_${PROJECT_NAME})
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ament_export_libraries(export_${PROJECT_NAME})
ament_export_libraries(${PROJECT_NAME}Targets)

@JafarAbdi pointed out to me that this naming convention is more standard cmake than the export_ prefix we have been using.

@@ -65,7 +65,7 @@ GlobalPlannerComponent::GlobalPlannerComponent(const rclcpp::NodeOptions& option
}
else
{
initialized_ = this->init();
initialized_ = this->initialize();
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried the changes to initialize in the constructor instead of in a timer?

@AndyZe
Copy link
Member

AndyZe commented Oct 14, 2021

This currently doesn't work for me. Same for you?

ros2 launch moveit_hybrid_planning hybrid_planning.launch.py

[rviz2-3] [INFO] [1634226190.040198790] [moveit_robot_model.robot_model]: Loading robot model 'panda'...
[rviz2-3] [INFO] [1634226190.377771555] [moveit_rdf_loader.rdf_loader]: Loaded robot model in 0.00141904 seconds
[rviz2-3] [INFO] [1634226190.377805126] [moveit_robot_model.robot_model]: Loading robot model 'panda'...
[rviz2-3] [ERROR] [1634226190.384232534] [moveit_background_processing.background_processing]: Exception caught while processing action 'loadRobotModel': std::bad_alloc

@sjahr
Copy link
Contributor Author

sjahr commented Oct 15, 2021

This currently doesn't work for me. Same for you?

ros2 launch moveit_hybrid_planning hybrid_planning.launch.py

[rviz2-3] [INFO] [1634226190.040198790] [moveit_robot_model.robot_model]: Loading robot model 'panda'...
[rviz2-3] [INFO] [1634226190.377771555] [moveit_rdf_loader.rdf_loader]: Loaded robot model in 0.00141904 seconds
[rviz2-3] [INFO] [1634226190.377805126] [moveit_robot_model.robot_model]: Loading robot model 'panda'...
[rviz2-3] [ERROR] [1634226190.384232534] [moveit_background_processing.background_processing]: Exception caught while processing action 'loadRobotModel': std::bad_alloc

No sorry, I have not experienced this error. Could you send the whole terminal output?

@sjahr
Copy link
Contributor Author

sjahr commented Oct 15, 2021

This currently doesn't work for me. Same for you?
ros2 launch moveit_hybrid_planning hybrid_planning.launch.py

[rviz2-3] [INFO] [1634226190.040198790] [moveit_robot_model.robot_model]: Loading robot model 'panda'...
[rviz2-3] [INFO] [1634226190.377771555] [moveit_rdf_loader.rdf_loader]: Loaded robot model in 0.00141904 seconds
[rviz2-3] [INFO] [1634226190.377805126] [moveit_robot_model.robot_model]: Loading robot model 'panda'...
[rviz2-3] [ERROR] [1634226190.384232534] [moveit_background_processing.background_processing]: Exception caught while processing action 'loadRobotModel': std::bad_alloc

No sorry, I have not experienced this error. Could you send the whole terminal output?

@AndyZe Just guessing: It might be related to this missing patch #734 which fixes a potential deadlock situation. Could you try to rebase on main?

tylerjw and others added 3 commits October 20, 2021 16:11
Alphabetize in CMakeLists

Load filter coefficient in the new plugin

Do filtering in the plugin

Implement 'reset'

Purge the previous filter implementation

Fix parameter parsing

Update unit tests

Clang format

Code cleanup. This causes an issue with reset() -- need to look into it

Do not use const for size_t

Fix logic error

Sync config files

Alphabetize moveit_core subdirectories

Move the smoothing plugin to moveit_core

Bug fixes

Load the plugin name from yaml

Remove commented code. Rename the plugin in a generic way

Minor cleanup. Delete unused file.

Minor cleanup per Nathan's code review

Implement a filteredHalt() function

Rename the package from smoothing_plugins->online_signal_smoothing

Small comments and efficiency improvement

Add Butterworth plugin package export

Add default constructor for SmoothingBaseClass

Add missing pluginlib dependency to moveit_servo

Skeleton of the smoothing plugin

Load filter coefficient in the new plugin

Do filtering in the plugin

Implement 'reset'

Purge the previous filter implementation

Fix parameter parsing

Clang format

Code cleanup. This causes an issue with reset() -- need to look into it

Do not use const for size_t

Fix logic error

Alphabetize moveit_core subdirectories

Move the smoothing plugin to moveit_core

Bug fixes

Load the plugin name from yaml

Remove commented code. Rename the plugin in a generic way

Minor cleanup. Delete unused file.

Minor cleanup per Nathan's code review

Rename the package from smoothing_plugins->online_signal_smoothing

Logic fixups, per Nathan

Delete default constructor/destructor

Post-rebase cleanup

Try moving the base class to a separate library

Ensure we don't pulish pos if user did not request it (& vel & accel)

Try to fix CI for galactic, in particular

Restore default constructor and destructor.
@tylerjw
Copy link
Member

tylerjw commented Oct 29, 2021

Closing in favor of #763

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.

None yet

8 participants