Skip to content

Conversation

@Timple
Copy link

@Timple Timple commented Sep 26, 2025

After following this tutorial: https://ros-industrial.github.io/ros2_canopen/manual/rolling/user-guide/how-to-create-a-configuration.html

I had some errors, so I tried to fix the documentation.


configuration item; description
node_id; The node-ID (default: 255)
node_id; The node-ID (e.g. 255)
Copy link
Author

Choose a reason for hiding this comment

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

Not specifying it in bus.yml gives an error.

options:
dcf_path: install/{package_name}/share/{package_name}/config/{bus_config_name}
dcf_path: "@BUS_CONFIG_PATH@"
Copy link
Author

Choose a reason for hiding this comment

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

This is recommended I believe

.. code-block:: yaml
nodes:
- [unique slave name]:
Copy link
Author

Choose a reason for hiding this comment

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

This should not be a list but a map.


.. code-block:: console
mkdir launch
Copy link
Author

Choose a reason for hiding this comment

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

This directory was already created above.

mkdir launch
touch {...}.launch.py
touch launch/{...}.launch.py
Copy link
Author

Choose a reason for hiding this comment

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

Added launch/ to indicate we're suddenly operating in different directories.

launch/
DESTINATION share/${PROJECT_NAME}/launch/
launch
DESTINATION share/${PROJECT_NAME}
Copy link
Author

Choose a reason for hiding this comment

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

This is simply a cleanup. All other tutorials use this syntax, result is the same but this allows for more folders.

)
install(DIRECTORY
launch_tests/
Copy link
Author

Choose a reason for hiding this comment

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

This directory simply does not exist.

)
if(BUILD_TESTING)
Copy link
Author

Choose a reason for hiding this comment

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

I think this is just noise on the tutorial.

int boot_attempts = 0;
const int max_boot_attempts = 3; // 1 retry allowed
RCLCPP_WARN(this->node_->get_logger(), "Wait for device to boot...");
RCLCPP_INFO(this->node_->get_logger(), "Wait for device to boot...");
Copy link
Author

Choose a reason for hiding this comment

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

This is normal and expected/configured behavior

std::string master_bin_;
std::string can_interface_name_;
uint32_t timeout_;
uint32_t timeout_ = 2000;
Copy link
Author

Choose a reason for hiding this comment

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

Single source of truth. The logging and the actual default were already out of date.

@Timple Timple mentioned this pull request Sep 30, 2025
2 tasks
Comment on lines -209 to -212
find_package(canopen_core REQUIRED)
find_package(canopen_interfaces REQUIRED)
find_package(canopen_base_driver REQUIRED)
find_package(canopen_proxy_driver REQUIRED)
Copy link
Author

Choose a reason for hiding this comment

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

These are unused

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.

1 participant