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

Let users access and modify low-level controller configuration #121

Closed
PeterBowman opened this issue Oct 28, 2017 · 12 comments
Closed

Let users access and modify low-level controller configuration #121

PeterBowman opened this issue Oct 28, 2017 · 12 comments
Assignees

Comments

@PeterBowman
Copy link
Member

PeterBowman commented Oct 28, 2017

As discussed with @jgvictores, we'd like to stop and avoid an increasing expansion of the list of commands at ICartesianControl (totalling 17 at the time of filing this issue). See background at #105.

Influenced by yarp::dev::ICartesianControl::tweakSet and yarp::dev::ICartesianControl::tweakGet, consider adding a pair of setter/getter RPC methods that would allow users to change a set of controller parameters on runtime, such as:

Iin parallel to this, new configuration options should be implemented in the corresponding YARP devices (e.g. yarpdev --device BasicCartesianControl --gain 0.15 ...).

Example signature:

// for doubles...
bool setParameter(int vocab, double value);
bool getParameter(int vocab, double *value);

// for bools...
bool setParameter(int vocab, bool value);
bool getParameter(int vocab, bool *value);

// other types?

Multiple overloads may seem like overkill and go against the initial movation of this issue, but C++ lacks a standard container similar to yarp::os::Value (at least not until C++17, see std::any).

@PeterBowman
Copy link
Member Author

ros::NodeHandle implements a similar solution for accessing the parameter server (ref1, ref2). ROS devs didn't hesitate to create overloads: ros::NodeHandle::getParam x 16, ros::NodeHandle::setParam x 16, plus their cached counterparts, and more, including generic methods (which I think we can't use in an abstract base class).

A remark on that: ROS does not place constraints on the parameter identifier (the key part, as opposed to the value). It's meant to be a plain std::string which may point at any type of data, hence such a high number of overloads. Should we make them accept vocabs instead, a decision should be taken whether to define a narrow, basic set of properties in ICartesianControl.h (e.g. VOCAB_CC_GAIN), or delegate this responsibility to the implementors (e.g. BasicCartesianControl and AmorCartesianControl might accept different sets of vocabs). The latter imposes a serious restriction: client code must know beforehand how is the vocab encoded and construct it with yarp::os::Vocab::encode prior to handling the vocab parameter. This is obviously true for stringified parameter keys as well, but what is the point of enforcing a four-character limit for these descriptors, then?

@PeterBowman
Copy link
Member Author

PeterBowman commented Dec 10, 2017

Idea: publish some or all of these configuration parameters along with controller state and FK solution in stat() (#102, #129). Discuss it here or at #118.

@jgvictores
Copy link
Member

Maybe just double and string for now?

@PeterBowman
Copy link
Member Author

I'm going to forget about strings for now... I can't see any valid use case for the time being.

@PeterBowman
Copy link
Member Author

Done at b0ccbf2. Currently enabled parameters:

  • controller gain
  • maximum joint velocity
  • trajectory duration
  • reference frame (implemented as vocabs):
    • base frame (inertial)
    • TCP frame (end effector)

No longer blocking #46.

@PeterBowman
Copy link
Member Author

PeterBowman commented Jan 20, 2018

Small follow-up at 944ec2a. Relevant changes:

  • new configuration parameter: cmcRateMs (only BasicCartesianControl)
  • encoded frame vocabs have been moved to ICartesianControl::reference_frame
    //! Lists supported reference frames.
    enum reference_frame
    {
    BASE_FRAME = VOCAB4('c','p','f','b'), //!< Base frame
    TCP_FRAME = VOCAB4('c','p','f','t') //!< End-effector frame (TCP)
    };
  • a few RPC commands now support the TCP frame, notably movl (we can issue linear offsets) and movv

@jgvictores
Copy link
Member

//! Lists supported reference frames.
enum reference_frame
{
BASE_FRAME = VOCAB4('c','p','f','b'), //!< Base frame
TCP_FRAME = VOCAB4('c','p','f','t') //!< End-effector frame (TCP)
};

Classy! Smooth! Nice!!

  • important: movl and pose will crash unless controllerGain is set to zero

Should that be happening? In simulation, zero gain should work, but a non-zero gain should be needed with real motors.

@PeterBowman
Copy link
Member Author

Classy! Smooth! Nice!!

Just a shameless copypasta from YARP 😄

Should that be happening? In simulation, zero gain should work, but a non-zero gain should be needed with real motors.

In fact, this is closely related to #136 because of a fwdKinError call that does not support (yet) the TCP frame. See:

iCartesianSolver->fwdKinError(desiredX, currentQ, commandXdot);
for (int i = 0; i < 6; i++)
{
commandXdot[i] *= gain * (1000.0 / cmcRateMs);
commandXdot[i] += desiredXdot[i];
}

As a workaround, I suggest setting gain to zero so that the buggy command does not influence subsequent operations.

@jgvictores
Copy link
Member

Okay, understood. Thanks!

@PeterBowman
Copy link
Member Author

PeterBowman commented Jan 23, 2018

Another follow-up: commit 5ad5eea added two new methods for setting and retrieving multiple parameters at once. Also, set/get vocabs have been standardized. Example invocations via yarp rpc:

  • get cpjv: retrieve maximum joint velocity
  • set cpjv 15: set maximum joint velocity to 15
  • get prms: retrieve all configuration parameters
  • set prms (cpjv 15) (cpcr 60): set maximum joint velocity to 15 and CMC rate to 60 ms

Vocabs may be passed either as plain strings or in brackets: cpjv or [cpjv].

/**
* @brief Set multiple configuration parameters.
*
* Ask the controller to store or update multiple parameters at once.
*
* @param params Dictionary of YARP-encoded vocabs as keys and their values.
*
* @return true on success, false otherwise
*/
virtual bool setParameters(const std::map<int, double> & params) = 0;
/**
* @brief Retrieve multiple configuration parameters.
*
* Ask the controller to retrieve all available parameters at once.
*
* @param params Dictionary of YARP-encoded vocabs as keys and their values.
*
* @return true on success, false otherwise
*/
virtual bool getParameters(std::map<int, double> & params) = 0;

@PeterBowman
Copy link
Member Author

PeterBowman commented Feb 3, 2018

Yet another follow-up (YAFU, which vaguely resembles YAFUD): 1f62c00. Now, we can configure the check period within the implementors of ICartesianControl::wait (#106):

addUsage("... [cpwp] value", "(config param) check period of 'wait' command [ms]");

@PeterBowman
Copy link
Member Author

PeterBowman commented Jul 19, 2018

#121 (comment)

The cpjv (maximum joint velocity) parameter shall be removed after roboticslab-uc3m/yarp-devices#188, and convenient calls to getVelLimits performed on controller init (similarly to getLimits).

Edit: see #159.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants