Skip to content
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.

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

Add an abstract base to generated parameters structures. #37

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

artivis
Copy link
Collaborator

@artivis artivis commented Jan 10, 2018

Summary

  • Adds an abstract base to generated parameter classes enabling their use through a parameter base pointer.
  • Add copy initialization/assignment to Parameters objects.
  • Initialize variables with default value at construction so that they can be used prior to calling fromParamServer

Comes in replacement to #35 .

Details

Please consider the following example :

struct DummyBase
{
  DummyBase() = default;
  virtual ~DummyBase() = default;

  // A helper function to instantiate the params_ptr_
  template <typename T>
  void init()
  {
    params_ptr_ = boost::make_shared<T>(ros::NodeHandle("~"));
  }

  void fromParamServer()
  {
    params_ptr_->fromParamServer();
  }

  void toParamServer()
  {
    params_ptr_->toParamServer();
  }

  // Pointer to the xParameters object
  rosparam_handler::ParametersPtr params_ptr_;
};

struct Foo : public DummyBase
{
    Foo()
    {
       // Instantiate the params_ptr_ as a FooParameters object
       init<FooParameters>();

       params_ptr_->fromParamServer();
    }

    void doThingsWithParams()
    {
       auto foo_param_ptr = boost::dynamic_pointer_cast<FooParameters>(params_ptr_);

       // Assuming FooParameters has an int parameter called my_int_param
       foo_param_ptr->my_int_param = 42;

       params_ptr_->toParamServer();
    }
};

struct Bar : public DummyBase
{
  Bar()
  {
     // Instantiate the params_ptr_ as a BarParameters object
     init<BarParameters>();

     params_ptr_->fromParamServer();

     dynamic_reconfigure::Server<BarConfig>::CallbackType cb;
     cb = boost::bind(&Bar::configCallback, this, _1, _2);
     dr_srv_.setCallback(cb);
  }

  void configCallback(BarConfig &config, uint32_t level)
  {
     params_ptr_->fromConfig(config);
  }

  dynamic_reconfigure::Server<BarConfig> dr_srv_;
};

This PR does not change the default behavior while allowing one to use a unique rosparam_handler::ParameterPtr in a base class and instantiate it to a different parameter type. All functions remain available through the base pointer.

The main drawback here is that to access the actual members of the instantiated object one has to cast the pointer to the appropriate type (either statically or dynamically).
See helper functions :

using namespace boost;
using namespace rosparam_handler;
...
ParametersPtr my_params = make_shared<BarParameters>(ros::NodeHandle("~"));
...
shared_ptr<FooParameters> foo_cast = dynamic_parameters_cast<FooParameters>(my_params);

assert(foo_cast == nullptr);

shared_ptr<BarParameters> bar_cast = static_parameters_cast<BarParameters>(my_params);

assert(bar_cast != nullptr);

All former tests are passing and a I added one for this new scheme.
Their is also a test-case demonstrating how to test dynamic_reconfigure callback with rosparam_handler.

This PR also turns *Parameters into copyable objects :

FooParameters my_foo(ros::NodeHandle("~"));
my_foo.fromParamServer();

FooParameters my_foo_copy(my_foo); // Copy initialization

FooParameters my_foo_copy2 = my_foo; // Copy assignment

Please consider this PR as Work In Progress until I add some more tutorial to rosparam_handler_tutorial.

@artivis
Copy link
Collaborator Author

artivis commented Jan 23, 2018

Force pushed after a cleaning of this branch history.

@artivis
Copy link
Collaborator Author

artivis commented Jan 23, 2018

Since all tests are declared in the same rostest, they all share the same environment, thus the same param server.
This can be an issue since some tests set params while other retrieve them (or vice-versa) leading to collision. To avoid it each NodeHandle uses a sub-namespace (see 9956bf1).
The other alternative is to break down the unique rostest into several.

@artivis
Copy link
Collaborator Author

artivis commented Feb 4, 2018

Rebased on develop and fix tests compilation (#34).
Force pushed a broken history...

@artivis
Copy link
Collaborator Author

artivis commented Jun 6, 2018

ping @130s, @cbandera. Anyone to review this ?? :DD

@artivis artivis mentioned this pull request Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant