locusrobotics / robot_navigation

Spiritual successor to ros-planning/navigation.
444 stars 149 forks source link

get the proper exception code #39

Closed cf-zhang closed 5 years ago

cf-zhang commented 5 years ago

when get the exception code through e.getResultCode() always be the base class's code.

because the "resultcode" is hard coded in the exceptions.h. So i change the result code with Member variables in base class.

And add a set of enumeration constants for exceptions.

DLu commented 5 years ago

Hey @cf-zhang thanks for bringing this to my attention.

I'm not quite sure where you're seeing the wrong result code though. I wrote this quick test and it seems to pass, at least on my machine. Does it pass for you? Or am I misunderstanding what the problem is?

TEST(Exceptions, test_ex_code)
{
  try
  {
    throw nav_core2::GlobalPlannerTimeoutException("test case");
  }
  catch (nav_core2::GlobalPlannerTimeoutException& x)
  {
    EXPECT_EQ(x.getResultCode(), 12);
  }

  try
  {
    throw nav_core2::GlobalPlannerTimeoutException("test case");
  }
  catch (nav_core2::PlannerException& x)
  {
    EXPECT_EQ(x.getResultCode(), 12);
  }
}
cf-zhang commented 5 years ago

sorry for my unclear expression,and the quick test you written is ok,

But it's not the scenario I'm talking about.

the problem is: when I throw nav_core2::OccupiedGoalException from Locomotor::makeGlobalPlan's makePlan, and the catch block pass the exception to fail_cb through nav_core2::PlannerException& e

void Locomotor::makeGlobalPlan(Executor& result_ex, GlobalPlanCallback cb, PlannerExceptionCallback fail_cb)
{
  ros::WallTime start_t = ros::WallTime::now();
  try
  {
    state_.global_pose = getGlobalRobotPose();

    {
      boost::unique_lock<boost::recursive_mutex> lock(*(global_costmap_->getMutex()));
      state_.global_plan = global_planner_mux_.getCurrentPlugin().makePlan(state_.global_pose, state_.goal);
    }
    if (cb) result_ex.addCallback(std::bind(cb, state_.global_plan, getTimeDiffFromNow(start_t)));
  }
  // if we didn't get a plan and we are in the planning state (the robot isn't moving)
  catch (const nav_core2::PlannerException& e)
  {
    if (fail_cb) result_ex.addCallback(std::bind(fail_cb, e, getTimeDiffFromNow(start_t)));
  }
}

when I deal the exception in SingleThreadLocomotor::onGlobalPlanningException, I want to use e.getResultCode() to distinguish the specific anomalies.

e.getResultCode() should have been 10. But because the parameter is nav_core 2:: PlannerException e, I get the e.getResultCode() is 3.

  void onGlobalPlanningException(nav_core2::PlannerException e, const ros::Duration& planning_time)
  {
    ROS_INFO_NAMED("LocoMoveBase", "e.getResultCode() : %d", e.getResultCode());
    ROS_ERROR_NAMED("Locomotor", "Global planning error. Giving up.");
    requestNavigationFailure(makeResultCode(locomotor_msgs::ResultCode::GLOBAL_PLANNER, e.getResultCode(),
                                            "Global Planning Failure."));
  }

So I made the exceptions.h file such a change, for the proper result code.

I'm not sure if it's because I'm not using it correctly.

thanks.

DLu commented 5 years ago

Hey @cf-zhang - I finally got a chance to dig deeper on this, and have replicated the problem. Working on a PR now.

DLu commented 5 years ago

Closed via #47. Thanks for bringing this to my attention.