muriqui用のコードを移植する#38
Open
hideyoshi-k wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
muriqui で利用していた実装を rt_manipulators_lib 側へ移植し、Dynamixel モデル追加(XC330/PH54)や座標変換の切替、外部ポート(アナログ入力)読み取り等を拡張するPRです。
Changes:
- 関節グループの sync_read 対象として external_port1-4 を追加し、読み取り・保持・取得APIを追加
- XC330 の indirect 非対応を考慮した direct アドレス経路(sync_read/sync_write)の導入
- PH54 / XC330 のモデル追加と、それに伴うテスト・FakeIt モック導入
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| rt_manipulators_lib/test/test_hardware.cpp | Hardware の read/write_data 等のユニットテスト追加(FakeIt モック使用) |
| rt_manipulators_lib/test/test_dynamixel_ph54.cpp | PH54 の変換系テスト追加 |
| rt_manipulators_lib/test/test_dynamixel_p.cpp | P 系の external port 間接アドレス設定テスト追加 |
| rt_manipulators_lib/test/test_config_file_parser.cpp | external port 読み取り設定のパース検証テスト追加 |
| rt_manipulators_lib/test/config/ok_read_external_port.yaml | external_port1-4 を含む設定追加 |
| rt_manipulators_lib/test/CMakeLists.txt | FakeIt FetchContent とテスト追加 |
| rt_manipulators_lib/src/kinematics_utils.cpp | モデルにより座標変換を切り替える分岐追加 |
| rt_manipulators_lib/src/joint.cpp | XC330/PH54 の生成追加、external port 電圧保持を追加 |
| rt_manipulators_lib/src/hardware.cpp | direct/indirect 切替、external port 読み取り、注入用コンストラクタ等を追加 |
| rt_manipulators_lib/src/hardware_joints.cpp | external port 電圧取得APIを追加 |
| rt_manipulators_lib/src/hardware_communicator.cpp | PacketHandler 取得方法変更、呼び出し箇所更新 |
| rt_manipulators_lib/src/dynamixel_xc330.cpp | XC330 モデル実装追加 |
| rt_manipulators_lib/src/dynamixel_x.cpp | external port / direct read-write 用の定義・抽出・書き込み追加 |
| rt_manipulators_lib/src/dynamixel_ph54.cpp | PH54 モデル実装追加 |
| rt_manipulators_lib/src/dynamixel_p.cpp | external port(間接アドレス・モード設定・抽出)追加、間接アドレス設定の冪等化 |
| rt_manipulators_lib/src/config_file_parser.cpp | parse() 前に parsed_joints をリセット |
| rt_manipulators_lib/src/CMakeLists.txt | PH54 ビルド対象追加 |
| rt_manipulators_lib/include/joint.hpp | external port API / name() 追加 |
| rt_manipulators_lib/include/hardware.hpp | 注入用コンストラクタ、external port API、read/write_data テンプレ追加 |
| rt_manipulators_lib/include/hardware_joints.hpp | external port 電圧取得API追加 |
| rt_manipulators_lib/include/hardware_communicator.hpp | Communicator をモック可能にするため virtual 化 |
| rt_manipulators_lib/include/dynamixel_xc330.hpp | XC330 ヘッダ追加 |
| rt_manipulators_lib/include/dynamixel_x.hpp | external port / direct read-write API・状態追加 |
| rt_manipulators_lib/include/dynamixel_ph54.hpp | PH54 ヘッダ追加 |
| rt_manipulators_lib/include/dynamixel_p.hpp | external port API 追加 |
| rt_manipulators_lib/include/dynamixel_base.hpp | external port / direct read-write 用の仮想API追加 |
| rt_manipulators_lib/CMakeLists.txt | XC330/PH54 ソース追加 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
38
to
40
| explicit Hardware(const std::string device_name); | ||
| explicit Hardware(std::unique_ptr<hardware_communicator::Communicator> comm); | ||
| ~Hardware(); |
| std::map<JointGroupName, uint16_t> addr_sync_read_temperature_; | ||
| bool thread_enable_; | ||
| std::shared_ptr<std::thread> read_write_thread_; | ||
| bool use_direct_addr_enabled_; |
Comment on lines
24
to
+31
| Hardware::Hardware(const std::string device_name) : | ||
| thread_enable_(false) { | ||
| comm_ = std::make_shared<hardware_communicator::Communicator>(device_name); | ||
| comm_ = std::make_unique<hardware_communicator::Communicator>(device_name); | ||
| } | ||
|
|
||
| Hardware::Hardware(std::unique_ptr<hardware_communicator::Communicator> comm) : | ||
| thread_enable_(false) { | ||
| comm_ = std::move(comm); |
Comment on lines
+326
to
340
| if (joints_.group(group_name)->sync_read_external_port1_enabled()) { | ||
| retval = read_external_port(1); | ||
| } | ||
|
|
||
| if (joints_.group(group_name)->sync_read_external_port2_enabled()) { | ||
| retval = read_external_port(2); | ||
| } | ||
|
|
||
| if (joints_.group(group_name)->sync_read_external_port3_enabled()) { | ||
| retval = read_external_port(3); | ||
| } | ||
|
|
||
| if (joints_.group(group_name)->sync_read_external_port4_enabled()) { | ||
| retval = read_external_port(4); | ||
| } |
Comment on lines
+863
to
+873
| bool Hardware::search_unsupport_indirect_addr_hw_type(const std::string& group_name) { | ||
| bool retval = true; | ||
| for (const auto & joint_name : joints_.group(group_name)->joint_names()) { | ||
| if (joints_.joint(joint_name)->dxl->get_name() == "XC330") { | ||
| use_direct_addr_enabled_ = true; | ||
| retval = false; | ||
| } | ||
| } | ||
|
|
||
| return retval; | ||
| } |
Comment on lines
+344
to
+354
| unsigned int DynamixelX::indirect_addr_of_external_port(const int number) { | ||
| if (number == 1) { | ||
| return indirect_addr_of_external_port1_; | ||
| } else if (number == 2) { | ||
| return indirect_addr_of_external_port2_; | ||
| } else if (number == 3) { | ||
| return indirect_addr_of_external_port3_; | ||
| } | ||
|
|
||
| return indirect_addr_of_external_port1_; | ||
| } |
Comment on lines
+586
to
+593
| uint16_t target_addr = 0; | ||
| if (number == 1) { | ||
| target_addr = ADDR_EXTERNAL_PORT_MODE1; | ||
| } else if (number == 2) { | ||
| target_addr = ADDR_EXTERNAL_PORT_MODE2; | ||
| } else if (number == 3) { | ||
| target_addr = ADDR_EXTERNAL_PORT_MODE3; | ||
| } |
Comment on lines
+53
to
+55
| rt_manipulators_cpp::Hardware hardware( | ||
| std::unique_ptr<hardware_communicator::Communicator>(&mock.get())); | ||
|
|
Comment on lines
+69
to
+73
| bool no_coordinate_transformation = false; | ||
| if(file_path.find("bonobo") != std::string::npos){ | ||
| std::cout << "bonobo_links" << std::endl; | ||
| no_coordinate_transformation = true; | ||
| } |
Comment on lines
31
to
+33
| uint8_t id() const; | ||
| uint8_t operating_mode() const; | ||
| std::string name() const; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
muriquiで使用していたコードを移植します
座標変換を切り替える処理の他、Dynamixelモデル(xc330, ph54)の追加や、処理の追加・変更があります
Does this close any currently open issues?
How has this been tested?
humble、jazzy環境でビルドが通ることを確認しました
humble環境でcrane-x7の重力補償が動作することを確認しました
Any other comments?
Checklists