UbiquityRobotics / move_basic

A minimal navigation node
BSD 3-Clause "New" or "Revised" License
69 stars 21 forks source link

Navigation features and lots more stuff #76

Closed dorkamotorka closed 3 years ago

dorkamotorka commented 3 years ago

This add features that were also included in the breadcrumb/move_smooth. Besides that it fixes a bug - Robot was not turning toward the requested final orientation, because the final rotation was calculated as (initial robot orientation - final wanted robot orientation (both in odom)), but the robot arrived to the goal with different orientation as initial, therefore it didn't turned in place the way it should.

JanezCim commented 3 years ago

@tp4348 where have the min_lateral_velocity and min_turning_velocity gone? is that not implemented?

JanezCim commented 3 years ago

and what about params like robot_front_length and robot_width, will they not be in dynamic reconfigure?

dorkamotorka commented 3 years ago

@tp4348 where have the min_lateral_velocity and min_turning_velocity gone? is that not implemented?

If you have min_lateral velocity specified theoretically you cannot even drive the robot straight, because there will also be this lateral component and also for the turning velocity you cannot bring the error as close to 0 as possible. Velocities are now limited like [-max_lateral_velocity, max_lateral_velocity] and also for the turning velocity,.. This ranges are just below what robot is able to perform in the firmware.

dorkamotorka commented 3 years ago

and what about params like robot_front_length and robot_width, will they not be in dynamic reconfigure?

This values are the lengths of the robot mechanical parts. They shouldn't be dynamically reconfigured. Changing them would change the behavior of collision avoidance(stopping before collision) but for this you have forwardObstacleThreshold

dorkamotorka commented 3 years ago

Just to make it clear - I will make sure the READ.md and ROS Wiki is updated

dorkamotorka commented 3 years ago

@AGregorc This PR will be open at least till the end of this week. If you have time, it would be useful for you to understand it, because I will be away in January and that directly relates to navigation which we will be using for Q... .

dorkamotorka commented 3 years ago

Mistakenly closed it.

dorkamotorka commented 3 years ago

I tested with web routes for 5 hours. No error.

JanezCim commented 3 years ago

Tested it multiple times on a real robot in a small room and it seems to be working alright. I need to test in TP still in a large area still and with different speeds. Im testing with this launch, can you confirm all parameters are ok?

<?xml version="1.0"?>
<!-- This launch file is the collection of the move_basic params relevant for Qualcomm robot -->

<launch>
  <node name="move_basic" pkg="move_basic" type="move_basic"  output="screen">    
    <param name="base_frame" value="base_link"/>
    <param name="reverse_without_turning_threshold" value="0.1"/>
    <param name="angular_tolerance" value="0.08"/>
    <param name="linear_tolerance" value="0.1"/>
    <param name="velocity_threshold" value="0.2"/>
    <param name="max_linear_velocity" value="0.3"/>
    <param name="max_turning_velocity" value="0.5"/>
    <param name="lateral_kp" value="0.3"/>
    <param name="lateral_kd" value="0.5"/>
    <param name="linear_gain" value="0.5"/>
    <param name="rotational_gain" value="3.5"/>
    <param name="linear_acceleration" value="0.4"/>
    <param name="abort_timeout" value="0.5"/>    

    <param name="forward_obstacle_threshold" value="0.2"/>
    <param name="robot_back_length" value="0.33"/>
    <param name="min_side_dist" value="0.2"/>
    <param name="robot_width" value="0.2"/>
    <param name="robot_front_length" value="0.2"/>

    <remap from="/move_base/goal" to="move_base/goal"/>
    <remap from="/move_base_simple/goal" to="move_base_simple/goal"/>
    <remap from="/plan" to="plan"/>
    <remap from="/obstacle_viz" to="obstacle_viz"/>
    <remap from="/cmd_vel" to="cmd_vel"/>
    <remap from="/lateral_error" to="lateral_error"/>
    <remap from="/obstacle_distance" to="obstacle_distance"/>
    <remap from="/plan" to="plan"/>
    <remap from="/scan" to="scan_filtered"/>

  </node>
</launch>
dorkamotorka commented 3 years ago

image The parameter I tested with are on the image. In my opinion the parameters should be tuned a bit more, since it still happens on some goal that the robot triggers abort: Moving away from goal. So thread with cautious. Nevertheless, if anyone finds suitable parameters write the down and publish it here or something :)

JanezCim commented 3 years ago

I guess it would be of great value if original author of move_basic would take a detailed look at this before we merge right? @rohbotics @jim2v

jim-v commented 3 years ago

I guess it would be of great value if original author of move_basic would take a detailed look at this before we merge right? @rohbotics @jim2v

I could take a look, but won't be able to get to it for a few days.

dorkamotorka commented 3 years ago

This would be great if Jim and Rohan can take a look. We can hold it off, no problem.

rohbotics commented 3 years ago

I don't have time to properly review the code, but I like the overall changes and there are already approvals so you can go ahead and merge @tp4348