Added publish period attribute as a parameter

[ahcorde]

By default the publish rate is hardcoded to 1 minute

This PR allows to change this value using ROS 2 parameters

[thomas-moulard]

Thank you for the pull request! Would you need this parameter to not be read-only?

For something like a publishing period, maybe it would be preferable to declare the parameter as read-only (see https://github.com/ros2/rcl_interfaces/blob/master/rcl_interfaces/msg/ParameterDescriptor.msg and declare_parameter signature). This would alleviate the possibility of bug when the parameter value is changed dynamically.

[ahcorde]

Sure,

where I should document the meaning and units of this parameter?

[zmichaels11]

The parameter along with its default value should be documented within the README.
Otherwise this LGTM. Thanks for your contribution!

[zmichaels11]

Suggested change

	publish_period_ = std::chrono::milliseconds(publish_period_param);
	publish_period_ = std::chrono::milliseconds{publish_period_param};

[zmichaels11]

Nit: It might be useful to extract "publish_period" into a constant

[mm318]

We're working on making all the node constructors use rclcpp::NodeOptions, which will most likely supersede all the changes in this pull request.

[dabonnie]

We're working on making all the node constructors use rclcpp::NodeOptions, which will most likely supersede all the changes in this pull request.

Not sure if you're planning on it, but it would be great to have those changes in a separate PR :-)

[thomas-moulard]

@ahcorde I think it would be good to document the parameter in the ParameterDescriptor directly.
https://github.com/ros2/rcl_interfaces/blob/master/rcl_interfaces/msg/ParameterDescriptor.msg

See the field description. i would also set the range if it supports half-open intervals (not sure)

[ahcorde]

closing this PR, this other PR #73 is more complete