Fix resource management in UAS node#1961
Conversation
ac225e1 to
8a0e968
Compare
vooon
left a comment
There was a problem hiding this comment.
UAS test fails. Jazzy's uncrustify also unhappy.
| Eigen::Vector3d af, | ||
| Eigen::Vector3d const & p, | ||
| Eigen::Vector3d const & v, | ||
| Eigen::Vector3d const & af, |
There was a problem hiding this comment.
It's usually questionable thing, what's faster - just copy 4 float64's or access trough a pointer all fields.
To me that looks as an unneeded preliminary optimisation.
There was a problem hiding this comment.
"just copy 4 float64's" -- 9 doubles, to be precise; which is 72 bytes.
"or access trough a pointer all fields" -- how do you think the fields are accessed if the object is passed by value?
Eigen::Vector3d is a type that is bigger than just a couple of bytes and it has a copy constructor. It is a good practice to pass such types by const reference.
| std::dynamic_pointer_cast<rclcpp::Node>(uas_)->get_fully_qualified_name(), options)) | ||
| {} | ||
| UASPtr uas, const std::string & subnode_name, | ||
| const rclcpp::NodeOptions & options = rclcpp::NodeOptions()); |
There was a problem hiding this comment.
S.t. all plugins don't have to recompile when something changes here.
There was a problem hiding this comment.
And it's rarely changed, and if so, I'd prefer to recompile everything...
There was a problem hiding this comment.
Why do you prefer to recompile everything?
On the The test executable crashes before most of the tests have a chance to fail. Could you maybe make the tests in the |
- Removed unnecessary dynamic_pointer_cast<>'s Removed unused Plugin ctor
Prohibit copying and moving of UAS Removed startup_delay_timer
UASyaml-cppin a correct way.