-
Notifications
You must be signed in to change notification settings - Fork 1
moteus arm control #71
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
Conversation
| } | ||
| } | ||
|
|
||
| void ArmSerial::send_position_command(float pos[NUM_JOINTS]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how does pos[] get to target positions and the can frame? looks like we are missing something?
|
|
||
| add_executable(arm_serial_driver src/ArmSerialInterface.cpp include/ArmSerialInterface.h include/arm_hardware_interface/ArmSerialProtocol.h) | ||
| # For old and new arm | ||
| # add_executable(arm_serial_driver src/ArmSerialInterface.cpp include/ArmSerialInterface.h include/arm_hardware_interface/ArmSerialProtocol.h) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like the idea of using the same class/header, but we should be able to compile both binaries so we don't need to edit a cmake file if we want to run the new arm vs the old arm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so yeah try to add both targets, maybe moteus_arm_driver and serial_arm_driver
|
|
||
| #define PI 3.14159 | ||
|
|
||
| ArmSerial::ArmSerial() : Node("ArmSerialDriver") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what would be best here is to make a base class that both the old arm driver and new arm driver use.
although this should still be fine because each node is it's own cmake target/binary and have seperate main()
| [submodule "src/external_pkgs/Catch2"] | ||
| path = src/external_pkgs/Catch2 | ||
| url = https://github.com/catchorg/Catch2.git | ||
| [submodule "src/arm_hardware_interface/moteus"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, did you use git submodule add for this (like git commands) or manually edit the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manually editing this doesn't work for me, afaik there are more files git changes when you add via the command line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats this for?
|
|
||
| target_compile_definitions(moteus_arm_driver | ||
| PRIVATE | ||
| BUILDING_NEW_DRIVER=1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooo compile definitions. nice! this is better than just throwing a definition in a header
| serial::Timeout timeout_uart = serial::Timeout::simpleTimeout(1000); // E.g., 1000 ms or 1 second | ||
|
|
||
| #ifdef BUILDING_NEW_DRIVER | ||
| std::map<int, std::shared_ptr<moteus::Controller>> controllers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should moteus stuff be in the serial driver? i thought serial driver was purely for old arm
| import numpy as np | ||
|
|
||
|
|
||
| def quat_mul(q, r): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this something you added @jodhsinghnahal ? arman may be working on the same branch by accident..?
|
Closing because duplicate with #94 |
No description provided.