Fix bag pick + add singular launch w/ bag orientation randomizer - #16
Fix bag pick + add singular launch w/ bag orientation randomizer#16mtobia-nasa wants to merge 5 commits into
Conversation
…ner tissue box on HW. CTB reorientation occurs before vertical liftkit movement now.
…zing ctb orientation
|
Haven't tested it but looks neat! Although I do wonder how we can get away from basically duplicating like the entire clr_mujoco.launch.py here... I wonder if maybe we need some python helpers to basically provide the functionality where we can just sprinkle some stuff on? I think this a great use case for us to think about how we do this in the future, because this functionality is going to be super necessary! |
…h, added support to specify ctb z rotation at launch
ndunkelb-nasa
left a comment
There was a problem hiding this comment.
So I mentioned it in the other comments, but I think having this file here that includes all the contents isn't the right solution. I think you are trying to accomplish two things here. Let me know if these are incorrect
- Add a way to move the CTB however you want
- Make a single launch file that can run everything
I would prefer to see potentially a separate launch file that is maybe control.launch.py (see pb cylinder demo on gitlab - I won't link bc its internal). You can make that so it if you pass use_sim_time:=true, it launches similar to the main mujoco launch file, but with extra parameters for the ctb location placement, and if you run it with use_sim_time:=false it will launch the main hardware launch.
Then you can make a separate launch file that is just like launch_all.launch.py use_sim_time:=xxxxx where you can propagate that arg.
I like having some kind of structure that is something like below, which remains consistent regardless of what platform you are running on.
# launch control either in mujoco or on hw
ros2 launch clr_pick_and_place_demo control.launch.py use_sim_time:=true/false ctb_x_location:=0.5 ...
# launch visualization with appropriate args
ros2 launch clr_pick_and_place_demo visualization.launch.py use_sim_time:=true/false
# launch planning application layer with appropriate args
ros2 launch clr_pick_and_place_demo planning.launch.py use_sim_time:=true/false
# OR, you can run your single launch file
ros2 launch clr_pick_and_place_demo launch_all.launch.py use_sim_time:=true/false ctb_x_location:=0.5 ...
Open to suggestions on this, but this is how I would prefer to see it.
|
|
||
| def generate_mjcf_description_and_node(context): | ||
|
|
||
| ctb_orientation_randomized = randomize_ctb_orientation.perform(context) |
There was a problem hiding this comment.
I actually think that it might be best to leave the randomization logic out of this launch file. I think if this goes into some testing framework, it will be the responsibility of that testing framework to decide how exactly to apply the randomization to get the desired results, and I think the exposing of the positions/orientations does enough to accomplish that.
I assume the purpose of this was to support some kind of testing?
There was a problem hiding this comment.
Absolutely it was for testing. I wanted to verify the new demo order worked for other bag positions without having to go run a bunch of different picks on hw. Now that it works I'll break it down into separate launches like you described in the parent comment above.
| ], | ||
| ) | ||
|
|
||
| moveit_config = ( |
There was a problem hiding this comment.
I'm not the biggest fan of dumping all of the contents of the related launch files into a single file. I like that there are a couple of separate launch files, and that they can be run independently and brought up/down as necessary. I think if there was a goal of running all of them together, you could just make a single launch file that was like full_demo_setup.launch.py or something which just includes the launch descriptions of the other components. That lets you parameterize each of them as much as you want, then those changes get automatically pulled into your conglomerated launch file.
|
@mtobia-nasa if/when we want to pick this up we can merge into the updated repo in clr_ws. |
This PR fixes hardware issues where order of operations caused the CTB to collide with the trainer tissue box.