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

Cwbollinger/mobile api wip #69

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

cwbollinger
Copy link
Contributor

No description provided.

Specifically, behavior is bad when waypoint velocities
are not constrained. Need to fix `buildTrajectory`
Now uses piecewise cartesian trajectories and maps to
joint space as you go. More similar to old approach
…hebi-cpp-examples into cwbollinger/mobile_api_wip
kits/base/omni_mobile_io_control.cpp Outdated Show resolved Hide resolved
mobile -> clearText();

// Display instructions on screen
mobile -> sendText(instructions);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these have bool return values; I'm not sure about the balance between making the example more complex vs. more robust by handling them

kits/base/omni_mobile_io_control.cpp Outdated Show resolved Hide resolved

std::cout << "Creating Omni Base" << std::endl;

OmniBase base(p);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just looking at the example here vs. the code below, it isn't clear what happens if it can't find these modules on the network. I'd probably match the arm and mobile IO and have a static create method that returns a unique_ptr, so we can have a invalid option.

(actually...if we are OK bumping to C++17, returning an optional might be a good way to go...but we'd want to update the MobileIO and Arm APIs too. Worth further discussion...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be addressed now, and be c++11 compatible


while(base.update())
{
auto state = mobile->getState();
Copy link
Contributor

Choose a reason for hiding this comment

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

This reminds me...I think we still need to fix the MobileIO::getState call to not block in C++, or accept a timeout. I'll track this as a separate issue.

Comment on lines 14 to 19
struct Params: public MobileBase::Params {
Params() : wheel_radius(0.0762), base_radius(0.220), sample_density(0.1) {}
double wheel_radius; // m
double base_radius; // m (center of omni to origin of base)
double sample_density; // time between sampled waypoints on cartesian trajectory
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the basic idea of the inherited parameter structures here.

I want to try to think of something clean for initialization -- you can always set each value after creating a default instance, or add a constructor whose first argument is an instance of the base class. I feel like both of those work, but are sort of clunky...I'm not sure.

util/omni_base.hpp Outdated Show resolved Hide resolved
util/base.hpp Outdated Show resolved Hide resolved
util/base.hpp Outdated
Comment on lines 177 to 180
const Eigen::VectorXd times_{0};
const Eigen::Matrix<double, 3, Eigen::Dynamic> positions_{0, 0};
const Eigen::Matrix<double, 3, Eigen::Dynamic> velocities_{0, 0};
const Eigen::Matrix<double, 3, Eigen::Dynamic> accelerations_{0, 0};
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder about just storing a list of Waypoint objects instead, and just converting to the Eigen types as needed in the getters? Probably just moves the complexity rather than reduces it

util/base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/group_manager.hpp Outdated Show resolved Hide resolved
util/group_manager.hpp Outdated Show resolved Hide resolved
util/group_manager.hpp Outdated Show resolved Hide resolved
util/group_manager.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/mobile_base.hpp Outdated Show resolved Hide resolved
util/omni_base.hpp Outdated Show resolved Hide resolved
util/omni_base.hpp Outdated Show resolved Hide resolved

std::unique_ptr<queue<shared_ptr<Trajectory>>> OmniBase::buildTrajectory(const CartesianGoal& g) {
auto num_waypoints = g.times().size();
auto num_wheels = g.positions().row(0).size();
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@iamtesch iamtesch Jul 20, 2021

Choose a reason for hiding this comment

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

maybe:

if (_past_trajectory && trajectory time is valid)
{
  // these are both in old trajectory frame.
  auto pose, vel = _past_trajectory.getState(time);
  auto local_vel = vel * pose.theta.inverse();
  // now we have a starting waypoint velocity.
}
else
{
  // first try last command velocity, then last feedback if it doesn't exist.
  auto v = group_manager.getLastCommand().getVelocity();
  auto cartesian_local_vel = WheelToSE2(nil, v);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

add this at the starting

util/mobile_base.hpp Outdated Show resolved Hide resolved
Minor refactoring to remove duplicated transform math

using arm::Goal;

class GroupTrajectoryManager: GroupMananger {
Copy link
Contributor

Choose a reason for hiding this comment

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

Banananananananana

Comment on lines 141 to 151
double t = get_current_time_s_();

// Time must be monotonically increasing!
if (t < last_time_)
return false;

dt_ = t - last_time_;
last_time_ = t;

if (!group_->getNextFeedback(feedback_))
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

GroupManager::update()

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

Successfully merging this pull request may close these issues.

2 participants