plugin: setpoint_raw: move getParam to initializer #1669
Conversation
mavros/src/plugins/setpoint_raw.cpp
Outdated
| @@ -71,6 +77,8 @@ class SetpointRawPlugin : public plugin::PluginBase, | |||
| ros::Subscriber local_sub, global_sub, attitude_sub; | |||
| ros::Publisher target_local_pub, target_global_pub, target_attitude_pub; | |||
|
|
|||
| std::unique_ptr<double> thrust_scaling_parameter; | |||
There was a problem hiding this comment.
I think that you do not need to dynamically allocate that double. You just need a sign of unset scaling, so NAN is good for that.
There was a problem hiding this comment.
I'm actually inclined to use boost::optional. It would be elegant and does away with dynamic allocs. ROS already depends on boost so the added dependency is not a big issue. What do you think?
There was a problem hiding this comment.
Well, i checked that there are std::optional in C++17, so it's ok to use boost::optional for ROS1. But i would rather limit boost dependency.
There was a problem hiding this comment.
@vooon Well, I'd hate to let my design choices get in the way of this PR. Just switched to setting a -1.0 sentinel value for thrust_scaling and pushed.
Repeatedly getting the thrust_scaling parameter in a callback that can be invoked from a fast control loop may fail spuriously and trigger a fatal error
Currently
attitude_cb()attempts to get thethrust_scalingparameter. This is problematic as documented in this question, and is exacerbated whenattitude_cb()is called in a fast control loop. In which case,getParam()may return false spuriously and trip a fatal error. This has been observed when a control loop is run at 150Hz on a Jetson Nano.This PR moves the
getParam()call to initialize to avoid this problem, at the cost of removing the ability to dynamically changethrust_scaling(which I believe is unlikely to be important)