refactor/lekiwi robot #863
Conversation
imstevenpmwork
added
refactor
imstevenpmwork
force-pushed
the
refactor/lekiwi_robot
branch
6 times, most recently
from
8458616 to
2a04a2a
Compare
imstevenpmwork
force-pushed
the
refactor/lekiwi_robot
branch
from
435dee5 to
c9d7a88
Compare
|
@aliberts This PR has been rebased with the latest changes of user/aliberts/2025_02_25_refactor_robots |
imstevenpmwork
force-pushed
the
refactor/lekiwi_robot
branch
4 times, most recently
from
447496d to
4ec2ef5
Compare
imstevenpmwork
force-pushed
the
refactor/lekiwi_robot
branch
3 times, most recently
from
7719882 to
a0657ee
Compare
|
@aliberts This PR has been rebased onto the latest changes from the refactor_robots branch (2025-04-25), including all commit before: Fix calibration msg display (including this one). |
There was a problem hiding this comment.
I have left many comments regarding comments.
My general take is: Whenever there's a comment somewhere, this means the code its commenting isn't readable enough -> should be fixable 90% of the time by just better code/naming. I prefer reading 10 additional lines of intelligible code than 1 line of comment (exception made for configs and docstring, obv)
Moreover, if the comment is a TODO, then this means there's something wrong here that we didn't take the time to fix now, and we're adding debt for our future selves to solve. The goal of these refactoring PRs should be to remove the TODOs as much as possible, not add more of them :')
Don't hesitate to comment code on github directly if there's something about which you'd like to get the reviewer's attention.
Let's not use the codebase as a notepad and work our way through this now. We abused them in the past and I think it's time to get rid of them.
|
Awesome! Thanks for taking the time to do the in-depth review. I will target the comments asap so we can move forward 🦾 |
imstevenpmwork
force-pushed
the
refactor/lekiwi_robot
branch
4 times, most recently
from
291bd89 to
dffb69e
Compare
chore(teleop): Add missing abstract methods to keyboard implementation refactor(robots): update lekiwi client and host code for the new api chore(config): update host lekiwi ip in client config chore(examples): move application scripts to the examples directory fix(motors): missing type check condition in set_half_turn_homings fix(robots): fix assumption in calibrate() for robots with more than just an arm fix(robot): change Mode to Operating_Mode in configure write for lekiwi fix(robots): make sure message is display in calibrate() method lekiwi fix(robots): no need for .tolist() in lekiwi host app fix(teleop): fix is_connected in teleoperator keyboard fix(teleop): always display calibration message in so100 fix(robots): fix send_action in lekiwi_client debug(examples): configuration for lekiwi client app fix(robots): fix send_action in lekiwi client part 2 refactor(robots): use dicts in lekiwi for get_obs and send_action dbg(robots): check sent action wheels lekiwi debug(robots): fix overflow base commands debug(robots): fix how we deal with negative values lekiwi debug(robots): lekiwi sign degrees fix fix(robots): right motors id in lekiwi host chore(doc): update todos chore(doc): added todos
Co-authored-by: Simon Alibert <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Simon Alibert <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Simon Alibert <[email protected]>
for more information, see https://pre-commit.ci
Co-authored-by: Simon Alibert <[email protected]>
imstevenpmwork
force-pushed
the
refactor/lekiwi_robot
branch
from
dffb69e to
c882875
Compare
Co-authored-by: Simon Alibert <[email protected]>
Co-authored-by: Simon Alibert <[email protected]>
…eleop in favor of is_conected property
aliberts
merged commit 2e528a8
into
user/aliberts/2025_02_25_refactor_robots
What this does
Ports
LeKiwiRobotto the new robot architectureThis PR is meant to be
Rebase & MergeHow it was tested
(before the rebase against the latest motor API)
Tested with a S0100 as a leader arm + the Mac keyboard vs a Remote Lekiwi Robot
Dataset recorded (remotely): https://huggingface.co/datasets/imstevenpmwork/lekiwi