modifying pacmod3 ros2 driver to work with pacmod2 #22
Conversation
include/pacmod2/pacmod2_node.hpp
Outdated
| std::pair<std::shared_ptr<rclcpp::SubscriptionBase>, | ||
| std::shared_ptr<LockedData>>> can_subs_; | ||
|
|
||
| std::shared_ptr<std::thread> pub_thread_; |
There was a problem hiding this comment.
if this pub_thread_ is not shared with anyone else, use std::unique_ptr instead.
std::unique_ptr<std::thread> pub_thread_ = nullptr;
| void publish_cmds(); | ||
| //void set_enable(bool enable); | ||
|
|
||
| static constexpr auto SEND_CMD_INTERVAL = std::chrono::milliseconds(33); |
There was a problem hiding this comment.
If these two static variables are not referenced by other classes, we can move the definitions to its cpp file.
There was a problem hiding this comment.
can't declare static constexpr without an initializer
| this->create_subscription<pacmod2_msgs::msg::SystemCmdFloat>( | ||
| "accel_cmd", 20, | ||
| std::bind(&PACMod2Node::callback_accel_cmd, this, std::placeholders::_1)), | ||
| std::shared_ptr<LockedData>(new LockedData(AccelCmdMsg::DATA_LENGTH))); |
There was a problem hiding this comment.
In c++14, we can use std::make_shared<> to create a shared pointer. It is a preferred way.
https://learn.microsoft.com/en-us/cpp/cpp/how-to-create-and-use-shared-ptr-instances?view=msvc-170
There was a problem hiding this comment.
don't want to make this change because of this:
https://stackoverflow.com/questions/70027525/undefined-reference-to-initialized-static-member-variable-with-make-shared
work around is std::make_shared<LockedData>(+AccelCmdMsg::DATA_LENGTH) but I think this is weird. I can make the change if others disagree.
No description provided.