code-iai / iai_maps

The semantic and ground lab and environment maps for projects in the IAI group in the University of Bremen
12 stars 37 forks source link

Fix issues #113

Closed HoangGiang93 closed 1 year ago

HoangGiang93 commented 1 year ago

I fixed most of the issues that @daniel86 addressed. Can you check on that?

daniel86 commented 1 year ago

@HoangGiang93 could you maybe summarize your changes? especially what are the new link names? and did you refactor others? I would need to go through the whole file again without this summary.

btw. why is the diff so large? It is odd, e.g. the diff states you removed 1,787 lines from kitchen.xacro (you deleted the file) but in the same diff it is added again with 1855 lines. What is expected to see a diff with just the roughly 100 lines that changed. that would also be a bit more comprehensible for doing a review ;) maybe you messed up something?

HoangGiang93 commented 1 year ago

Sorry I @daniel86, I tried different ways to use git mv to rename the file but it didn't work properly. After I modify the file (the old one but with a new name), github always change it into deleting that file and creating a new one :/

image

You see, I tried to rename apartment.urdf to furnitures.xacro. Then I modified furnitures.xacro and pushed it. Github changed it into deleting apartment.urdf and creating furnitures.xacro, which removed all the old commits from the others.

HoangGiang93 commented 1 year ago

I think if you want to know which code I changed that actually fixed the issue, you can go through each commit. Before the commit https://github.com/code-iai/iai_maps/pull/113/commits/f41048749a4f2263d1fb3b4d02f3fceb6e2da0ba that fixes the issue #106, I didn't rename any files, so you would see the diff there better. We can discuss about this later.

daniel86 commented 1 year ago

Seems fine to me. I think one of the persons using the URDF on the CRAM side should approve as well. maybe @artnie ?

gaya- commented 1 year ago

I can't see if the coordinates are still the same or different. For example, wall_coloksu_wall1_joint changed coordinates. Was that on purpose or a copy paste mistake from a previous URDF? It would be great if somebody spawned the 2 kitchens half-transparent at the same time, then we'd see if the offsets are still fine. This can be done with the bullet world by having two different names for the kitchens. Or in Giskard PyBullet. Or maybe even in RViz. It would be great if indentation changes were committed separately, so one could look at apartment.urdf and see what changed. So the urdf -> xarco change should've been it's own commit. @HoangGiang93 where is the commit that changed the wall_coloksu_wall1_joint origin?

HoangGiang93 commented 1 year ago

It would be great if indentation changes were committed separately, so one could look at apartment.urdf and see what changed.

I changed the name of kitchen.xarco to kitchen.xacro in 1 commit and github still turned it into deleting and creating :(

@HoangGiang93 where is the commit that changed the wall_coloksu_wall1_joint origin?

I didn't change the wall_coloksu_wall1_joint origin. See below: https://github.com/code-iai/iai_maps/blob/master/iai_apartment/urdf/apartment.urdf#L53 https://github.com/code-iai/iai_maps/blob/FixIssues/iai_apartment/urdf/furnitures.xacro#L50

gaya- commented 1 year ago

I didn't change the wall_coloksu_wall1_joint origin. See below: https://github.com/code-iai/iai_maps/blob/master/iai_apartment/urdf/apartment.urdf#L53 https://github.com/code-iai/iai_maps/blob/FixIssues/iai_apartment/urdf/furnitures.xacro#L50

Indeed, you didn't. In the diff it looked like you did. Ok, so from what I can see the offsets are ok.

@sunava Maybe you could try to pull this PR onto pr2-ext and see if the demo still runs? Of course making sure to note the commit that we're using now in case you need to revert.

Asking @sunava because she is showing the demo to our visitors and will be very angry if anyone else breaks the demo. :)

gaya- commented 1 year ago

When I start the upload_kitchen.launch file

$ roslaunch iai_apartment apartment_bringup.launch 

I get the following errors:

process[iai_apartment_root_link_broadcaster-1]: started with pid [5726]
process[apartment_state_publisher-2]: started with pid [5727]
process[apartment_joint_state_publisher-3]: started with pid [5738]
[ERROR] [1666284384.401244446]: material 'oven_gray' is not unique.
[apartment_state_publisher-2] process has died [pid 5727, exit code 255, cmd /opt/ros/kinetic/lib/robot_state_publisher/robot_state_publisher robot_description:=apartment_description joint_states:=apartment_joint_states __name:=apartment_state_publisher __log:=/home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_state_publisher-2.log].
log file: /home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_state_publisher-2*.log
Traceback (most recent call last):
  File "/opt/ros/kinetic/lib/joint_state_publisher/joint_state_publisher", line 72, in <module>
    jsp.loop()
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/joint_state_publisher/__init__.py", line 274, in loop
    msg.position[i] = joint['position'] * factor + offset
IndexError: list assignment index out of range
[apartment_joint_state_publisher-3] process has died [pid 5738, exit code 1, cmd /opt/ros/kinetic/lib/joint_state_publisher/joint_state_publisher robot_description:=apartment_description /joint_states:=/apartment_joint_states __name:=apartment_joint_state_publisher __log:=/home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_joint_state_publisher-3.log].
log file: /home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_joint_state_publisher-3*.log

The main problem seems to be the following line:

[ERROR] [1666284384.401244446]: material 'oven_gray' is not unique.

Please fix.

gaya- commented 1 year ago

And please let me know if you don't have this problem. It's weird that two people approve of this PR whereby the basic launch already gives errors.

gaya- commented 1 year ago

Then when I use my own launch file, I get

Link `side_A_rb' seems to have two parents

How did you guys test this?

HoangGiang93 commented 1 year ago

When I start the upload_kitchen.launch file

$ roslaunch iai_apartment apartment_bringup.launch 

I get the following errors:

process[iai_apartment_root_link_broadcaster-1]: started with pid [5726]
process[apartment_state_publisher-2]: started with pid [5727]
process[apartment_joint_state_publisher-3]: started with pid [5738]
[ERROR] [1666284384.401244446]: material 'oven_gray' is not unique.
[apartment_state_publisher-2] process has died [pid 5727, exit code 255, cmd /opt/ros/kinetic/lib/robot_state_publisher/robot_state_publisher robot_description:=apartment_description joint_states:=apartment_joint_states __name:=apartment_state_publisher __log:=/home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_state_publisher-2.log].
log file: /home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_state_publisher-2*.log
Traceback (most recent call last):
  File "/opt/ros/kinetic/lib/joint_state_publisher/joint_state_publisher", line 72, in <module>
    jsp.loop()
  File "/opt/ros/kinetic/lib/python2.7/dist-packages/joint_state_publisher/__init__.py", line 274, in loop
    msg.position[i] = joint['position'] * factor + offset
IndexError: list assignment index out of range
[apartment_joint_state_publisher-3] process has died [pid 5738, exit code 1, cmd /opt/ros/kinetic/lib/joint_state_publisher/joint_state_publisher robot_description:=apartment_description /joint_states:=/apartment_joint_states __name:=apartment_joint_state_publisher __log:=/home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_joint_state_publisher-3.log].
log file: /home/gaya/.ros/log/51aff9c0-5096-11ed-abb3-e03f4977eaf0/apartment_joint_state_publisher-3*.log

The main problem seems to be the following line:

[ERROR] [1666284384.401244446]: material 'oven_gray' is not unique.

Please fix.

image

Sorry I don't have that error :(

HoangGiang93 commented 1 year ago

The main problem seems to be the following line:

[ERROR] [1666284384.401244446]: material 'oven_gray' is not unique.

And by the way the oven_gray is actually unique https://github.com/code-iai/iai_maps/blob/FixIssues/iai_apartment/urdf/apartment.urdf#L535

sunava commented 1 year ago

i can try Tuesday if that works @gaya-

daniel86 commented 1 year ago

i can try Tuesday if that works @gaya-

so?

sunava commented 1 year ago

@gaya- @HoangGiang93 Sadly the test did not went well, the robot is driving against the wall when trying to open the door. Maybe that has something to do with the orientation changes that u mentioned gaya? Idk if you guys want me to investigate this more or not

HoangGiang93 commented 1 year ago

@gaya- @HoangGiang93 Sadly the test did not went well, the robot is driving against the wall when trying to open the door. Maybe that has something to do with the orientation changes that u mentioned gaya? Idk if you guys want me to investigate this more or not

I will need more information to know where the problem is

ichumuh commented 1 year ago

@sunava are you sure you loaded the kitchen into giskard?

sunava commented 1 year ago

since this happened last week to Alina, im going to check it on Tuesday if that was the issue. I remember to try it 5 times tho would be a bigger if i failed to load it correctly all the time :D, will update u

sunava commented 1 year ago

@HoangGiang93 @daniel86 approved from myside