osrf / ros2_serial_example

72 stars 16 forks source link

Adding node options to allow params from command line #71

Closed ahcorde closed 5 years ago

ahcorde commented 5 years ago

The behavior of parameters changed for Dashing and newer.

ahcorde commented 5 years ago

Hi @clalancette,

I have used allow_undeclared_parameters because when we compile the code we don't know the number of topics which the node needs to publish or subscribe

The idea should be something like this:

    rcl_interfaces::msg::ParameterDescriptor descriptor;
    descriptor.read_only = true;
    declare_parameter("backend_comms", "udp", descriptor);
    declare_parameter("device", "/dev/pts/6", descriptor);
    declare_parameter("baudrate", 0, descriptor);
    declare_parameter("udp_recv_port", 2020, descriptor);
    declare_parameter("udp_send_port", 2019, descriptor);
    declare_parameter("read_poll_ms", 250, descriptor);
    declare_parameter("ring_buffer_size", 8192, descriptor);
    declare_parameter("write_sleep_ms", 4, descriptor);
    declare_parameter("dynamic_serial_mapping_ms", 0, descriptor);
    declare_parameter("backend_protocol", "px4", descriptor);
    declare_parameter("topics");

but the last one should be like :

    declare_parameter("topics.battery_status.serial_mapping");
    declare_parameter("topics.battery_status.type");
    declare_parameter("topics.battery_status.direction");
    declare_parameter("topics.air_speed.serial_mapping");
    declare_parameter("topics.air_speed.type");
    declare_parameter("topics.air_speed.direction");
    ...

I don't know if there is a way to indicate that only one parameter is undeclare.

clalancette commented 5 years ago

I have used allow_undeclared_parameters because when we compile the code we don't know the number of topics which the node needs to publish or subscribe

Haha, of course, you are totally right. Sorry about that. You alluded to this above, but I think we should declare_parameter on all of the "required" parameters (baudrate, etc), and also mark allow_undeclared_parameters as true to deal with the unknown topic mapping. Does that make sense?

ahcorde commented 5 years ago

Fixed @clalancette