For mobile robot with Isaac Sim (bug fixed version) #25
Conversation
[change] separate position/velocity/effort command [change] use mimic joint name to publish command
…bot starting state
…tion after being moved to the initial position.
src/topic_based_system.cpp
Outdated
| @@ -156,6 +160,15 @@ CallbackReturn TopicBasedSystem::on_init(const hardware_interface::HardwareInfo& | |||
| { | |||
| sum_wrapped_joint_states_ = true; | |||
| } | |||
| if (get_hardware_parameter("use_initial_states_as_initial_commands", "false") == "true") | |||
There was a problem hiding this comment.
Did you copy these changes from https://github.com/PickNikRobotics/topic_based_ros2_control/pull/24/files? Please use git cherry-pick to retain commit authorship attribution to the original author
There was a problem hiding this comment.
Thanks for the review.
I didn't think I would get a response, so I had to copy other pull requests on my own.
I have never used git cherry-pick, but I will give it a try.
There was a problem hiding this comment.
Sorry for the late review, I'm hoping to get it merged today :)
There was a problem hiding this comment.
I would like to merge the #24 pull request, is it merged into this repository? Or is there a procedure to merge forked repository?
There was a problem hiding this comment.
I'll review it now, but I still don't understand the use case, can you give me an example where it's needed?
There was a problem hiding this comment.
I misunderstood the cherry-pick in git. Does that mean I should delete the relevant part and re-commit?
There was a problem hiding this comment.
I deleted the relevant part and re-committed.
| @@ -89,6 +94,7 @@ class TopicBasedSystem : public hardware_interface::SystemInterface | |||
| /// The size of this vector is (standard_interfaces_.size() x nr_joints) | |||
| std::vector<std::vector<double>> joint_commands_; | |||
| std::vector<std::vector<double>> joint_states_; | |||
| std::vector<std::vector<double>> initial_joint_commands_; | |||
There was a problem hiding this comment.
This is used to store the initial value of each joint; assigning to joint_commands_ will overwrite the value.
There was a problem hiding this comment.
When using Isaac Sim and Move It 2, I have a different variable for joint_command because of the overwriting behavior when I set the initial value to joint_command. It seems that joint_command is overwritten with all zeros even when joint_command messages are not exchanged.
| @@ -91,11 +92,12 @@ CallbackReturn TopicBasedSystem::on_init(const hardware_interface::HardwareInfo& | |||
| // Check the initial_value param is used | |||
| if (!interface.initial_value.empty()) | |||
| { | |||
| joint_commands_[index][i] = std::stod(interface.initial_value); | |||
| initial_joint_commands_[index][i] = std::stod(interface.initial_value); | |||
There was a problem hiding this comment.
Any reason to have a different variable for the initial ones? We should use command_interfaces rather than state_interfaces when getting the initial value
There was a problem hiding this comment.
The following file appears to have initial values for state_interfaces. So I'm using state_interfaces.
Was this just because this file is out of date?
https://github.com/moveit/moveit_resources/blob/ros2/panda_moveit_config/config/panda.ros2_control.xacro
… with robot starting state" This reverts commit 2b23e6c.
hijimasa
force-pushed
the
for_mobile_robot_with_isaac_sim
branch
from
cc59e67
to
ab71778
Compare
|
I added initial integration test to the repo #28, do you mind helping me writing tests for this PR as well? Writing tests that would fail without this PR |
|
@JafarAbdi Do I just merge feature/integration_test and add the test? |
What do you think about creating a new branch on top of |
[add] check velocity command exists to publish command
Solved the problem of commands not being published even if velocity was given, since only the difference in position was checked.
[change] separate position/velocity/effort command
This is the form of command expected by Isaac Sim.
Reference: https://docs.omniverse.nvidia.com/isaacsim/latest/ros2_tutorials/tutorial_ros2_manipulation.html#position-and-velocity-control-modes
[change] use mimic joint name to publish command
When the commands were separated, the joint state index was no longer available, so it was changed to search by name.
Edit: fixed bug about mimic joint only publish position command