Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ROS 2 #4

Merged
merged 14 commits into from
Oct 11, 2023
Merged

Support ROS 2 #4

merged 14 commits into from
Oct 11, 2023

Conversation

Kuwamai
Copy link
Collaborator

@Kuwamai Kuwamai commented Sep 29, 2023

What does this implement/fix?

  • ROS 2環境でロボットモデルが表示できるようになりました。Gazeboやcontrollerの設定はまだです。
  • URDFのlinkやjoint名、jointのlimit、colorをpropertyで定義するようにしました
  • industrial_ciをhumble環境に変更しました

Does this close any currently open issues?

しません

How has this been tested?

ビルドしたのち、以下のコマンドでRViz上にロボットモデルが表示されることを確認しました。

ros2 launch sciurus17_description display.launch.py

Any other comments?

Checklists

@Kuwamai Kuwamai added the Type: Feature New Feature label Sep 29, 2023
@Kuwamai Kuwamai requested a review from ShotaAk September 29, 2023 10:22
@Kuwamai Kuwamai self-assigned this Sep 29, 2023
Copy link

@ShotaAk ShotaAk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

display.launchでモデルが表示されることを確認しました。

コメントの確認お願いします。

<license>NON-COMMERCIAL LICENSE AGREEMENT</license>

<buildtool_depend>catkin</buildtool_depend>
<author email="[email protected]">Hiroyuki Nomura</author>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kuwagataさんの名前もauthorに追加してください

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ユニットテストを追加してください

<xacro:property name="NAME_LINK_NECK_1" value="neck_yaw_link"/>
<xacro:property name="NAME_LINK_NECK_2" value="neck_pitch_link"/>

<xacro:property name="NAME_LINK_CAMERA" value="camera_link"/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHEST_CAMERAと区別するため、NAME_LINK_HEAD_CAMERAにしてください

<xacro:property name="NAME_JOINT_NECK_1" value="neck_yaw_joint"/>
<xacro:property name="NAME_JOINT_NECK_2" value="neck_pitch_joint"/>

<xacro:property name="NAME_JOINT_CAMERA" value="head_camera_joint"/>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらも*_HEAD_CAMERAにしてください

Comment on lines 190 to 191
waist_yaw_upper_limit="${NAME_JOINT_BODY_LOWER_LIMIT}"
waist_yaw_lower_limit="${NAME_JOINT_BODY_UPPER_LIMIT}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upperに対してlowerをセットしているので修正してください。

関連issue:#3

Comment on lines 200 to 203
neck_yaw_lower_limit="${JOINT_NECK_1_LOWER_LIMIT}"
neck_yaw_upper_limit="${JOINT_NECK_1_UPPER_LIMIT}"
neck_pitch_lower_limit="${JOINT_NECK_2_LOWER_LIMIT}"
neck_pitch_upper_limit="${JOINT_NECK_2_UPPER_LIMIT}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

マクロに対して引数でパラメータを入れているところと、マクロ内でpropertyを参照しているところが混在しています。

わかりやすくするため、引数を撤廃し、マクロから直接propertyを参照してください。

他のマクロも同様です。

@Kuwamai Kuwamai requested a review from ShotaAk October 10, 2023 06:25
Copy link

@ShotaAk ShotaAk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1か所コメントしました。

urdf/sciurus17.urdf.xacro Outdated Show resolved Hide resolved
Copy link

@ShotaAk ShotaAk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTMです

@ShotaAk ShotaAk merged commit ee688c1 into ros2 Oct 11, 2023
2 checks passed
@ShotaAk ShotaAk deleted the ros2-devel branch October 11, 2023 07:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants