Skip to content

Read only parameters #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wjwwood merged 48 commits into from
Apr 23, 2019
Merged

Read only parameters #495

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
73553ec
in progress broken test_time_source
sloretz Jun 8, 2018
88a74ff
style
sloretz Jun 8, 2018
bc8457d
test undeclared params
sloretz Jun 8, 2018
a6fc8de
Only get parameter if it is set
sloretz Jun 8, 2018
7e19756
doc fixup
wjwwood Oct 18, 2018
2597f46
use override rather than virtual in places
wjwwood Oct 18, 2018
347449d
rename ParameterInfo_t to ParameterInfo and just use struct, no typedef
wjwwood Oct 18, 2018
304ba62
add method to access ParameterValue within a Parameter
wjwwood Oct 18, 2018
6e5aae9
enable get<Parameter> and get<ParameterValue> on Parameter class
wjwwood Oct 18, 2018
38c7670
avoid const pass by value
wjwwood Oct 18, 2018
c11654e
match type of enum in C++ to type used in message definition
wjwwood Oct 18, 2018
b3325aa
fixup after rebase
wjwwood Jan 30, 2019
189c02e
more fixup after rebase
wjwwood Jan 30, 2019
85b0377
replace create_parameter with declare_parameter
wjwwood Feb 1, 2019
598325d
provide implementation for templated declare_parameter method
wjwwood Feb 1, 2019
6b14049
style
wjwwood Feb 4, 2019
46c207f
do not use const reference when it's a primitive (like bool)
wjwwood Feb 22, 2019
8e28200
typo
wjwwood Feb 22, 2019
5273ad9
follow to bool change that wasn't staged
wjwwood Feb 23, 2019
06ddfc4
fixup tests
wjwwood Feb 23, 2019
04f812c
added lots of docs, alternative API signatures, and some of the tests
wjwwood Apr 10, 2019
206ffe3
more tests and associated fixes
wjwwood Apr 11, 2019
89018de
address documentation feedback
wjwwood Apr 12, 2019
9e03b08
fixup previously added tests
wjwwood Apr 15, 2019
ac1b6a6
add tests and fixes for describe_parameter(s) and get_parameter_types
wjwwood Apr 15, 2019
73b4c28
remove old parameter tests
wjwwood Apr 16, 2019
78a94d9
use const reference where possible
wjwwood Apr 16, 2019
bf1d149
address comments
wjwwood Apr 16, 2019
00da0a6
fix tests for deprecated methods
wjwwood Apr 16, 2019
570c7c2
address feedback
wjwwood Apr 17, 2019
3ef385d
significantly improve the reliability of the time_source tests
wjwwood Apr 18, 2019
01362ff
uncrustify, cpplint, and cppcheck fixes
wjwwood Apr 18, 2019
c48db2d
Revert "significantly improve the reliability of the time_source tests"
wjwwood Apr 18, 2019
4673ce9
Merge remote-tracking branch 'origin/master' into read_only_parameters
wjwwood Apr 18, 2019
ce9af32
only declare use_sim_time parameter if not already declared
wjwwood Apr 18, 2019
fb78acd
fixup rclcpp_lifecycle
wjwwood Apr 19, 2019
b7e4c0d
fixup tests
wjwwood Apr 19, 2019
efb3f85
add missing namespace scope which fails on Windows
wjwwood Apr 19, 2019
4ba4eeb
extend deprecation warning suppression to support Windows too
wjwwood Apr 19, 2019
cb72e58
fix compiler warnings and missing visibility macro
wjwwood Apr 19, 2019
7e79bba
remove commented left over tests
wjwwood Apr 19, 2019
933ac09
fix compiler warning on Windows
wjwwood Apr 19, 2019
444de25
suppress deprecation warning on include of file in Windows
wjwwood Apr 19, 2019
df7b036
avoid potential loss of data warning converting int64_t to int
wjwwood Apr 19, 2019
ea3285f
trying to fix more loss of data warnings
wjwwood Apr 20, 2019
68a4afe
fix test_node
wjwwood Apr 22, 2019
d9ad096
add option to automatically declare parameters from initial parameter…
wjwwood Apr 23, 2019
e29cdf1
remove redundant conditional
wjwwood Apr 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions rclcpp/include/rclcpp/node.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,36 @@ class Node : public std::enable_shared_from_this<Node>
const rmw_qos_profile_t & qos_profile = rmw_qos_profile_services_default,
rclcpp::callback_group::CallbackGroup::SharedPtr group = nullptr);

/// Declare and initialize a parameter.
/**
* This method is used to declare that a parameter exists on this node.
* If a run-time user has provided an an initial value then it will be set in this method,
* otherwise the default_value will be set.
* \param[in] name the name of the parameter
Comment thread
wjwwood marked this conversation as resolved.
Outdated
* \param[in] default_value An initial value to be used if a run-time user did not override it.
* \param[in] read_only if True then this parameter may not be changed after initialization.
* \throws std::runtime_error if parameter has already been declared.
* \throws std::runtime_error if a parameter name is invalid.
* \throws rclcpp::exceptions::InvalidParameterValueException if initial value fails to be set.
*/
RCLCPP_PUBLIC
void
declare_parameter(
const std::string & name,
const rclcpp::ParameterValue & default_value = rclcpp::ParameterValue(),
bool read_only = false);

@clalancette clalancette Feb 20, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we want to make this more generic than just a boolean for read_only. For instance, how would we declare a parameter that is read-only, must be in range 0.0 - 1.0, and that the type must be a double? I'm not suggesting that we need to implement all of that in this PR, but I'd like to understand how the API could allow for things like that.

@wjwwood wjwwood Feb 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It will need to either evolve to include more arguments or a structure that defines the meta data for the parameter. I don't know which is better right now, so that's why I didn't change it, and presumably why @sloretz started it this way.

@clalancette clalancette Feb 22, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm mostly worried about having to change the API later to add those additional features; if we don't do it now, then it will be an API break later and will be harder to do. Personally, it sounds like there are enough additional features we'd want to add that it would warrant adding a structure here. Thoughts?

@wjwwood wjwwood Feb 22, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought about this at the time, and I couldn't come up with how constraints might work (like must be in this range, or one of these choices, etc.) which put me off from deciding they should all live in a single struct, but I guess no matter what we come up with we can fit it in an options struct or "description"/"descriptor" struct, since that's the matching IDL equivalent (ParameterDescriptor.msg).


/// Declare and initialize a parameter with a type.
/**
* See the non-templated declare_parameter() on this class for details.
*/
template<typename ParameterT>
void
declare_parameter(
const std::string & name,
const ParameterT & default_value,
bool read_only = false);

RCLCPP_PUBLIC
std::vector<rcl_interfaces::msg::SetParametersResult>
set_parameters(const std::vector<rclcpp::Parameter> & parameters);
Expand Down
10 changes: 10 additions & 0 deletions rclcpp/include/rclcpp/node_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,16 @@ Node::register_param_change_callback(CallbackT && callback)
this->node_parameters_->register_param_change_callback(std::forward<CallbackT>(callback));
}

template<typename ParameterT>
void
Node::declare_parameter(
const std::string & name,
const ParameterT & default_value,
bool read_only)
{
this->declare_parameter(name, rclcpp::ParameterValue(default_value), read_only);
}

template<typename ParameterT>
void
Node::set_parameter_if_not_set(
Expand Down
59 changes: 37 additions & 22 deletions rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ namespace rclcpp
namespace node_interfaces
{

// Internal struct for holding useful info about parameters
struct ParameterInfo
{
/// True if a user called declare_parameter()
bool is_declared = false;

/// Current value of the parameter.
rclcpp::ParameterValue value;

/// A description of the parameter
rcl_interfaces::msg::ParameterDescriptor descriptor;
};

/// Implementation of the NodeParameters part of the Node API.
class NodeParameters : public NodeParametersInterface
{
Expand All @@ -56,67 +69,65 @@ class NodeParameters : public NodeParametersInterface
bool use_intra_process,
bool start_parameter_services,
bool start_parameter_event_publisher,
const rmw_qos_profile_t & parameter_event_qos_profile);
const rmw_qos_profile_t & parameter_event_qos_profile,
bool allow_undeclared_parameters);

RCLCPP_PUBLIC
virtual
~NodeParameters();

RCLCPP_PUBLIC
virtual
void
declare_parameter(
const std::string & name,
const rclcpp::ParameterValue & default_value,
bool read_only) override;

RCLCPP_PUBLIC
std::vector<rcl_interfaces::msg::SetParametersResult>
set_parameters(
const std::vector<rclcpp::Parameter> & parameters);
const std::vector<rclcpp::Parameter> & parameters) override;

RCLCPP_PUBLIC
virtual
rcl_interfaces::msg::SetParametersResult
set_parameters_atomically(
const std::vector<rclcpp::Parameter> & parameters);
const std::vector<rclcpp::Parameter> & parameters) override;

RCLCPP_PUBLIC
virtual
std::vector<rclcpp::Parameter>
get_parameters(const std::vector<std::string> & names) const;
get_parameters(const std::vector<std::string> & names) const override;

RCLCPP_PUBLIC
virtual
rclcpp::Parameter
get_parameter(const std::string & name) const;
get_parameter(const std::string & name) const override;

RCLCPP_PUBLIC
virtual
bool
get_parameter(
const std::string & name,
rclcpp::Parameter & parameter) const;
rclcpp::Parameter & parameter) const override;

RCLCPP_PUBLIC
virtual
bool
get_parameters_by_prefix(
const std::string & prefix,
std::map<std::string, rclcpp::Parameter> & parameters) const;
std::map<std::string, rclcpp::Parameter> & parameters) const override;

RCLCPP_PUBLIC
virtual
std::vector<rcl_interfaces::msg::ParameterDescriptor>
describe_parameters(const std::vector<std::string> & names) const;
describe_parameters(const std::vector<std::string> & names) const override;

RCLCPP_PUBLIC
virtual
std::vector<uint8_t>
get_parameter_types(const std::vector<std::string> & names) const;
get_parameter_types(const std::vector<std::string> & names) const override;

RCLCPP_PUBLIC
virtual
rcl_interfaces::msg::ListParametersResult
list_parameters(const std::vector<std::string> & prefixes, uint64_t depth) const;
list_parameters(const std::vector<std::string> & prefixes, uint64_t depth) const override;

RCLCPP_PUBLIC
virtual
void
register_param_change_callback(ParametersCallbackFunction callback);
register_param_change_callback(ParametersCallbackFunction callback) override;

private:
RCLCPP_DISABLE_COPY(NodeParameters)
Expand All @@ -125,7 +136,11 @@ class NodeParameters : public NodeParametersInterface

ParametersCallbackFunction parameters_callback_ = nullptr;

std::map<std::string, rclcpp::Parameter> parameters_;
std::map<std::string, ParameterInfo> parameters_;

std::map<std::string, rclcpp::ParameterValue> initial_parameter_values_;

bool allow_undeclared_ = false;

Publisher<rcl_interfaces::msg::ParameterEvent>::SharedPtr events_publisher_;

Expand Down
19 changes: 19 additions & 0 deletions rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,25 @@ class NodeParametersInterface
public:
RCLCPP_SMART_PTR_ALIASES_ONLY(NodeParametersInterface)

/// Declare and initialize a parameter.
/* This method is used to declare that a parameter exists on this node.
* If a run-time user has provided an an initial value then it will be set in this method,
* otherwise the default_value will be set.
* \param[in] name the name of the parameter
* \param[in] default_value An initial value to be used if a run-time user did not override it.
* \param[in] read_only if True then this parameter may not be changed after initialization.
* \throws std::runtime_error if parameter has already been declared.
* \throws std::runtime_error if a parameter name is invalid.
* \throws rclcpp::exceptions::InvalidParameterValueException if initial value fails to be set.
*/
RCLCPP_PUBLIC
virtual
void
declare_parameter(
const std::string & name,
const rclcpp::ParameterValue & default_value = rclcpp::ParameterValue(),
bool read_only = false) = 0;

RCLCPP_PUBLIC
virtual
~NodeParametersInterface() = default;
Expand Down
33 changes: 25 additions & 8 deletions rclcpp/include/rclcpp/node_options.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class NodeOptions

/// Return a reference to the use_global_arguments flag.
Comment thread
wjwwood marked this conversation as resolved.
Outdated
RCLCPP_PUBLIC
const bool &
bool
use_global_arguments() const;

/// Set the use_global_arguments flag, return this for parameter idiom.
Expand All @@ -144,11 +144,11 @@ class NodeOptions
*/
RCLCPP_PUBLIC
NodeOptions &
use_global_arguments(const bool & use_global_arguments);
use_global_arguments(bool use_global_arguments);

/// Return a reference to the use_intra_process_comms flag
RCLCPP_PUBLIC
const bool &
bool
use_intra_process_comms() const;

/// Set the use_intra_process_comms flag, return this for parameter idiom.
Expand All @@ -163,11 +163,11 @@ class NodeOptions
*/
RCLCPP_PUBLIC
NodeOptions &
use_intra_process_comms(const bool & use_intra_process_comms);
use_intra_process_comms(bool use_intra_process_comms);

/// Return a reference to the start_parameter_services flag
RCLCPP_PUBLIC
const bool &
bool
start_parameter_services() const;

/// Set the start_parameter_services flag, return this for parameter idiom.
Expand All @@ -182,11 +182,11 @@ class NodeOptions
*/
RCLCPP_PUBLIC
NodeOptions &
start_parameter_services(const bool & start_parameter_services);
start_parameter_services(bool start_parameter_services);

/// Return a reference to the start_parameter_event_publisher flag.
RCLCPP_PUBLIC
const bool &
bool
start_parameter_event_publisher() const;

/// Set the start_parameter_event_publisher flag, return this for parameter idiom.
Expand All @@ -198,7 +198,7 @@ class NodeOptions
*/
RCLCPP_PUBLIC
NodeOptions &
start_parameter_event_publisher(const bool & start_parameter_event_publisher);
start_parameter_event_publisher(bool start_parameter_event_publisher);

/// Return a reference to the parameter_event_qos_profile QoS.
RCLCPP_PUBLIC
Expand All @@ -213,6 +213,21 @@ class NodeOptions
NodeOptions &
parameter_event_qos_profile(const rmw_qos_profile_t & parameter_event_qos_profile);

/// Return a reference to the allow_undeclared_parameters flag.
RCLCPP_PUBLIC
bool
allow_undeclared_parameters() const;

/// Set the allow_undeclared_parameters, return this for parameter idiom.
/**
* If true, allow any parameter name to be set on the node without first
* being declared.
* Otherwise, setting an undeclared parameter will raise an exception.
*/
RCLCPP_PUBLIC
NodeOptions &
allow_undeclared_parameters(bool allow_undeclared_parameters);

/// Return the rcl_allocator_t to be used.
RCLCPP_PUBLIC
const rcl_allocator_t &
Expand Down Expand Up @@ -256,6 +271,8 @@ class NodeOptions

rmw_qos_profile_t parameter_event_qos_profile_ {rmw_qos_profile_parameter_events};

bool allow_undeclared_parameters_ {false};
Comment thread
wjwwood marked this conversation as resolved.

rcl_allocator_t allocator_ {rcl_get_default_allocator()};
};

Expand Down
Loading