Add: KyungHo's study folder and files - #2
Conversation
- Updated PKG-INFO to reflect correct metadata version and added scipy dependency. - Modified SOURCES.txt to include additional source files. - Updated requires.txt to include scipy and ensure correct versions for dependencies. - Adjusted setup.py to remove gym as a mandatory dependency and include find_packages. - Added week01 and week02 markdown files for curriculum documentation.
study
Added detailed setup instructions for WSL and PowerShell, including package installations and virtual environment setup for the SpotMicroJetson project.
Document unresolved issue with PyBullet simulation errors.
…tation for keyboard control methods
add_study_shin_eunji
study: iru-han 폴더 및 초기 파일 추가
Added detailed explanations and parameters for robot movement and behavior during ground and aerial experiments.
파이뷸렛 GUI 올리기
Added images to the week04.md document and made minor adjustments.
Removed duplicate episode reward entries and added images.
Add week 12 documentation and code examples
add humanoid file
Fix code block formatting in week12.md
- Add Kinematics/__init__.py so Python recognizes it as a package - Fix spotmicroai.py: use __file__-based sys.path to resolve import regardless of CWD - Fix spotmicroai.py: os.chdir to Simulation/ dir on Robot init so relative paths (URDF, textures) resolve correctly when run from project root in VS Code - Add traceback.print_exc() in main exception handler for better error visibility - Add SIMULATION_ANALYSIS.md: Korean documentation of simulation code structure Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Week01: servo motor comparison (MG996R, CLS6336HV, DS3218), BOM, wiring Week02: right leg assembly with mixed servos, ESP32 motor test, PyBullet gait simulation analysis Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Converted w02_RightLeg_Test.MOV (56MB) to mp4 to stay under GitHub's 50MB limit. Updated work02.md video reference accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Minho/week01 02
Add week03 pybullet practice
- work03.md: Isaac Sim 4.5 + Isaac Lab 2.3.2 installation guide with Windows patches - work04.md: SpotMicro body stand fabrication (aluminum profile + 3D print) - stl/: Stand STL file, drawing, BOM, and assembly photos - Simulation/: Isaac Sim hello world and SpotMicro URDF import test scripts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add minho study notes: week03-04 Isaac Lab setup and SpotMicro stand
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR updates repository configuration and Simulation core scripts (gait loop, spotmicroai.py, packaging), adds Isaac Sim scripts and simulation analysis docs, and introduces extensive independent per-contributor study materials (archer, iru-han, kim, kyungho, minho, robert) covering PyBullet/MuJoCo simulation, kinematics/IK, gait generation, RL training pipelines, and hardware documentation. ChangesCore project configuration and Simulation updates
Estimated code review effort: 4 (Complex) | ~75 minutes archer study contributions
iru-han study contributions
kim study contributions
kyungho study contributions
minho study contributions
robert study contributions
Estimated code review effort: 4 (Complex) | ~90 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
study/iru-han/week08/week08.md (1)
39-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win"1,000 Steps" doesn't match the benchmark script's actual invocation.
engine_benchmark.py'srun_benchmarkdefault and its__main__calls both usesteps=500, not 1,000. See corresponding comment in that file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/week08.md` at line 39, The benchmark heading is inconsistent with the actual script invocation, which uses 500 steps rather than 1,000. Update the “벤치마크 결과” label in the markdown to match the real default and __main__ usage from engine_benchmark.py, and keep the wording aligned with the run_benchmark symbol so the documented result reflects the executed step count.
🟡 Minor comments (21)
study/kim/week03/week03_pybullet_api.py-44-113 (1)
44-113: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMode cycles through 4 states but only 3 are handled.
mode = (mode + 1) % 4produces values 0-3, but onlymode == 0, 1, 2are handled by the if/elif chain (Lines 58, 76, 92). Every 4th 5-second interval, no motor control command is issued at all — the joint just coasts under whatever control mode/torque was last applied. This contradicts the "3 control modes" experiment described in the README.🐛 Proposed fix
- mode = (mode + 1) % 4 + mode = (mode + 1) % 3🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kim/week03/week03_pybullet_api.py` around lines 44 - 113, The mode logic in the main control loop cycles through 4 values, but the `if/elif` chain in `week03_pybullet_api.py` only handles three control modes (`mode == 0`, `1`, `2`), so the 4th state leaves the joint without a fresh command. Update the `mode = (mode + 1) % 4` logic or add an explicit handling branch in the loop so the control flow matches the intended 3-mode experiment; use the existing `mode` variable and the control blocks around `p.setJointMotorControl2` to keep behavior consistent.study/kim/week03/week03_pybullet_api.py-1-2 (1)
1-2: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
pybulletto the declared dependencies —study/kim/week03/week03_pybullet_api.pyimportspybullet/pybullet_data, butpyproject.tomldoesn’t listpybullet. If this example is meant to run from the repo root, add it there; otherwise document the separate install step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kim/week03/week03_pybullet_api.py` around lines 1 - 2, The week03_pybullet_api example imports pybullet and pybullet_data but the project dependency declaration is missing pybullet; update the dependency list in the package config so this module can run from the repo root, or otherwise add a clear install note for the example. Use the imports in week03_pybullet_api and the dependency declaration in pyproject.toml to locate the change and keep the setup consistent with the example’s runtime requirements.study/iru-han/week11_1.md-7-7 (1)
7-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced block.
```pythonwill satisfy markdownlint and make the embedded snippet easier to read/copy.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week11_1.md` at line 7, The fenced code block is missing a language tag, so update the markdown snippet to use a Python fence for the block identified in the review comment; add the language identifier to the opening fence so markdownlint passes and the snippet is easier to read and copy.Source: Linters/SAST tools
study/archer/pybullet_automatic_gait_test.py-173-178 (1)
173-178: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDuplicate
robot.step()/consoleClear()call per loop iteration.Lines 173-174 and 177-178 are identical — the simulation is stepped and the console cleared twice per iteration. This doubles the effective step rate compared to every other script in this cohort and skews the torque-logging measurements this script is meant to produce.
🐛 Proposed fix
log_joint_torques_every_0_5s(pose_label) robot.step() consoleClear() - - robot.step() - consoleClear()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/archer/pybullet_automatic_gait_test.py` around lines 173 - 178, The loop in pybullet_automatic_gait_test.py is calling robot.step() and consoleClear() twice per iteration, which doubles the simulation advance and skews the torque logs. Remove the duplicated pair so each loop iteration performs only one robot.step() followed by one consoleClear(), keeping the behavior aligned with the other gait test scripts.Simulation/SIMULATION_ANALYSIS.md-19-40 (1)
19-40: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language tags to the fenced examples.
The bare fences will keep tripping MD040, and the same omission repeats throughout this doc. Please label them (
text,python,bash, etc.) so markdownlint passes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Simulation/SIMULATION_ANALYSIS.md` around lines 19 - 40, The fenced examples in SIMULATION_ANALYSIS.md use unlabeled code fences, which triggers markdownlint MD040. Update each fenced block in the document to include an appropriate language tag such as text, python, or bash, and make sure the same fenced-listing pattern is consistently labeled wherever it appears so the markdown lint passes.Source: Linters/SAST tools
study/iru-han/week08/Simulation/mujoco_automatic_gait.py-26-27 (1)
26-27: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid blocking on
command_status.get()
KeyInterrupt.__init__seeds the queue, so the first loop won’t hang immediately. The remaining problem is that this token handoff can still freezemain()if the keyboard process stalls or exits, or if an exception drops the item betweenget()andput(). A shared dict or a timeout-backed read would make this loop resilient.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/Simulation/mujoco_automatic_gait.py` around lines 26 - 27, The token handoff in main() using command_status.get() and command_status.put() can still block forever if the keyboard process stalls, exits, or the item is lost between reads. Update the loop around command_status so it does not depend on an unconditional blocking get(); use a timeout-backed read or replace the queue handoff with a shared state mechanism, and keep the behavior localized to the main() loop and KeyInterrupt-driven status flow.study/iru-han/week08/Simulation/engine_benchmark.py-9-9 (1)
9-9: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBenchmark default (
steps=500) doesn't match the reported "1,000 Steps" inweek08.md.
week08.mdLine 39 reports results for "1,000 Steps" but the default (and only usage in__main__) runssteps=500. Either the doc or the invocation used to produce the report differs from what's in this file; please align them so the report is reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/Simulation/engine_benchmark.py` at line 9, The benchmark configuration is inconsistent between `run_benchmark()` and the reported “1,000 Steps” in the writeup. Update either the default `steps` value in `run_benchmark`, the `__main__` invocation, or the documentation so they all use the same step count, and make sure the reported results can be reproduced from `engine_benchmark.py` without relying on an unstated override.study/iru-han/week08/urdf/stairs_mujoco.xml-1-7 (1)
1-7: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the missing stairs geometry to
stairs_mujoco.xml.scene.xmlincludes this file, but it still only defines the ground plane/light setup, so the MuJoCo path renders no stairs. If this is only a converter output, the stairs asset needs to be checked in or wired in elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/urdf/stairs_mujoco.xml` around lines 1 - 7, The MuJoCo scene setup in stairs_mujoco.xml only defines the base worldbody with the light and ground_plane, so the stairs are missing from the included asset. Update the same XML to add the stairs geometry under mujoco/worldbody (or otherwise reference the generated stairs body used by scene.xml) so the included file actually renders the staircase instead of just the floor. Keep the existing model/compiler/worldbody structure intact and add the missing stairs elements alongside the current scene setup.study/iru-han/week08/urdf/scene.xml-1-5 (1)
1-5: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove the duplicate ground plane/light include
ground_and_light_template.xmlandstairs_mujoco.xmlboth add a plane atz=0and a top-down light, soscene.xmlends up with overlapping ground geoms and duplicate lighting. Drop one of the includes or remove the shared ground/light from one file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/urdf/scene.xml` around lines 1 - 5, The scene setup in mujoco model SpotMicro_World is including ground_and_light_template.xml and stairs_mujoco.xml together, which duplicates the ground plane and top-down light. Update scene.xml by removing one of those includes or by moving the shared ground/light definitions out of one source so only a single ground geom and light are created.study/iru-han/week09/train_spot.py-101-105 (1)
101-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTermination height doesn't match the report.
week09.mddocuments the fall-termination threshold as height < 0.2m, but this code terminates atheight < 0.25. Please reconcile the doc and implementation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week09/train_spot.py` around lines 101 - 105, The fall-termination threshold in `_is_done` does not match the documented value. Update the termination check in `train_spot.py` so the `height` cutoff used by `_is_done` matches the report’s 0.2m threshold, and keep the implementation and documentation consistent for the spot training task.study/kyungho/week01_03/Mujoco_quadruped_RL-main/custom_callback.py-24-24 (1)
24-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCheckpoint directory name diverges from
train.py's save path.This saves periodic checkpoints under
"PPO_pretrained_model/", whiletrain.py(Lines 14, 35) uses"PPO_trained_model/"for the model directory. If this isn't intentional, checkpoints will land in an unexpected folder.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/custom_callback.py` at line 24, The checkpoint path in the callback is inconsistent with the training script’s model directory, so periodic saves may go to the wrong folder. Update the directory construction in `custom_callback.py` where `log_dir` is built inside the callback to match the save path used by `train.py`, and keep the naming consistent across the callback and training setup so checkpoints land in the intended model directory.study/iru-han/week06_kinematicMotion.py-113-144 (1)
113-144: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a fallback return in
calcLeg.positions()feeds it modulo-based phase times, but the strict<chain has noelse, so a boundary value can still fall through and returnNone, breaking the leg array/IK path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week06_kinematicMotion.py` around lines 113 - 144, The calcLeg method can fall through without returning a value because the current t-based branches end with no default case; add a final fallback return for the last phase in calcLeg so it always returns a leg position array. Make sure the new fallback matches the existing startLp/endLp trajectory logic used by positions() and preserves the return type expected by the IK/array code path.study/minho/work02_SIMULATION_ANALYSIS.md-21-33 (1)
21-33: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse one canonical kinematics path.
The tree mixes
kinematics.pyandKinematics/kinematics.py; on case-sensitive filesystems those are different locations, and readers will follow the wrong path. Align the diagram with the import path used byspotmicroai.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/minho/work02_SIMULATION_ANALYSIS.md` around lines 21 - 33, The project tree is showing two different kinematics locations, which can mislead readers on case-sensitive systems. Update the diagram to use a single canonical path that matches the import used by spotmicroai.py, and remove the alternate kinematics.py reference so only the real source location is documented. Keep the naming consistent with the Kinematics/kinematics.py entry and the spotmicroai.py import path.study/minho/work01.md-122-126 (1)
122-126: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stray spaces in the CLS6336HV image paths.
The current filenames look like
CLS6336HV .jpg/CLS6336HV _specs.jpg/CLS6336HV _drawings.jpg, which will 404 unless the assets were intentionally named that way.🧹 Suggested correction
- - - + + +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/minho/work01.md` around lines 122 - 126, The CLS6336HV image links contain stray spaces in the filenames, which will break the asset paths. Update the image references in the markdown to match the actual filenames by removing the spaces in the paths used by the three CLS6336HV image entries.study/minho/work01.md-83-86 (1)
83-86: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the sourcing queries in the servo comparison table.
Rows 83-86 reuse
DS3218 PROas the query for unrelated models, so the table no longer points to the actual part names. That makes the purchase guidance misleading.♻️ Suggested correction
-| DS3230 Pro | DS3218 PRO 180 degree | ₩15,000 | DSServo | -| YP3235MG | DS3218 PRO 180 degree | ₩28,000 | QYRC Servo | -| YPinervo | DS3218 PRO 180 degree | ₩21,000 | GXServo | -| SPT5435LV-180 | DS3218 PRO 180 degree | ₩25,000 | SPT Servo | +| DS3230 Pro | DS3230 Pro | ₩15,000 | DSServo | +| YP3235MG | YP3235MG | ₩28,000 | QYRC Servo | +| YPinervo | YPinervo | ₩21,000 | GXServo | +| SPT5435LV-180 | SPT5435LV-180 | ₩25,000 | SPT Servo |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/minho/work01.md` around lines 83 - 86, The servo comparison table is using the same DS3218 PRO sourcing query for multiple unrelated models, which makes the entries inaccurate. Update each row in the table so the query/search term matches the actual model name in the first column (for example, the source text tied to DS3230 Pro, YP3235MG, YPinervo, and SPT5435LV-180 in the markdown table). Keep the rest of the pricing/vendor columns unchanged and verify the corrected queries are consistent throughout the comparison section.study/robert/6-4.py-352-386 (1)
352-386: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBare
except:instep()can swallowKeyboardInterrupt, breaking graceful shutdown.
run()relies onexcept KeyboardInterrupt(Line 416) to exit gracefully, but the bareexcept:blocks here catchBaseException(includingKeyboardInterrupt), so a Ctrl+C duringstep()gets silently absorbed instead of propagating torun()'s handler.🔧 Suggested fix
try: # 슬라이더에서 파라미터 읽기 self.trotting.t0 = p.readUserDebugParameter(self.slider_t0) self.trotting.t1 = p.readUserDebugParameter(self.slider_t1) self.trotting.t2 = p.readUserDebugParameter(self.slider_t2) self.trotting.t3 = p.readUserDebugParameter(self.slider_t3) self.trotting.Sh = p.readUserDebugParameter(self.slider_sh) height = p.readUserDebugParameter(self.slider_height) walking = p.readUserDebugParameter(self.slider_start) - except: + except p.error: self.connected = False return elapsed ... angles = self.kinematics.legIK(foot_pos_adjusted) self.set_leg_angles(leg_name, angles) - except: - pass + except (ValueError, ZeroDivisionError) as e: + print(f"IK error for {leg_name}: {e}") try: p.stepSimulation() - except: + except p.error: self.connected = False🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/6-4.py` around lines 352 - 386, The step() method uses bare except blocks around parameter reads, leg IK updates, and p.stepSimulation(), which can swallow KeyboardInterrupt and prevent run() from handling shutdown cleanly. Update the exception handling in step() to catch only expected exceptions (for example, standard runtime errors from debug parameter reads or IK/physics calls) and let KeyboardInterrupt propagate, using the step(), run(), and set_leg_angles()/kinematics.legIK() paths to locate the affected spots.Source: Linters/SAST tools
study/robert/8-1.py-12-38 (1)
12-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBroken fallback path construction.
model_pathpassed in ("../../urdf/spotmicroai_gen.urdf.xml") already contains the"../../"prefix; prepending another"../../"viaos.path.join("../../", model_path)produces"../../../../urdf/spotmicroai_gen.urdf.xml", going up 4 directories instead of the intended 2. This fallback branch will not resolve to the intended path. Since there's a further fallback to a hardcoded box model, this fails gracefully but likely never finds the real model.Resolving relative to the script's own directory would make this robust to the current working directory as well:
🩹 Suggested fix
+from pathlib import Path + def run_simulation(model_path="../../urdf/spot_micro.xml"): + script_dir = Path(__file__).parent # Attempt to find the model file if os.path.exists(model_path): print(f"Loading model from: {model_path}") model = mujoco.MjModel.from_xml_path(model_path) else: - # Check if the path is relative to the project root - project_root_path = os.path.join("../../", model_path) - if os.path.exists(project_root_path): + project_root_path = script_dir / model_path + if project_root_path.exists(): print(f"Loading model from: {project_root_path}") - model = mujoco.MjModel.from_xml_path(project_root_path) + model = mujoco.MjModel.from_xml_path(str(project_root_path))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/8-1.py` around lines 12 - 38, The fallback path logic in run_simulation is double-prefixing an already relative model_path, so the second existence check can point to the wrong location and miss the real URDF/MJCF. Update the path resolution in run_simulation to resolve the candidate model path relative to the script’s own directory (or otherwise normalize it) instead of blindly joining another "../../" prefix, and keep the final xml_content box fallback unchanged.study/robert/10-2.py-92-97 (1)
92-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIMU gate uses the wrong MuJoCo field.
self.model.nsensorcounts sensor definitions, not scalar values, so IMU data can stay zero even whensensordataalready has 6 entries. Switch this check toself.model.nsensordata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/10-2.py` around lines 92 - 97, The IMU read gate is using the wrong MuJoCo model field, so the code may skip valid IMU values even when sensordata contains them. In the state निर्माण method that builds qpos/qvel/imu and returns the concatenated array, replace the sensor-count check on self.model.nsensor with the scalar sensor-data count on self.model.nsensordata, and keep the existing self.data.sensordata[:6] read unchanged.study/robert/9-1.py-125-136 (1)
125-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
nsensordatafor the IMU guard
nsensorcounts sensor definitions, not scalar outputs, so a model with two 3-axis sensors (or any 6-value IMU layout) will fall back to zeros even whendata.sensordata[:6]is populated.🩹 Suggested fix
- if self.model.nsensor >= 6: + if self.model.nsensordata >= 6:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/9-1.py` around lines 125 - 136, The IMU branch in the code that reads `self.data.sensordata` is guarding on `self.model.nsensor`, which counts sensor objects rather than scalar channels, so valid 6-value IMU data can be skipped. Update the `imu_acc`/`imu_gyro` check to use `self.model.nsensordata` (or another scalar-output count) in the `if` condition, and keep the fallback zero/estimated path only when there are fewer than 6 sensor values available.study/robert/7-5.py-511-543 (1)
511-543: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winBare
except: passswallows all errors silently on disconnect.Static analysis flags this: bare except with no logging makes debugging shutdown issues impossible.
🔧 Suggested fix
finally: try: p.disconnect() - except: - pass + except Exception as exc: + print(f"[WARN] Error during disconnect: {exc}") print("[INFO] Simulation ended")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/7-5.py` around lines 511 - 543, The shutdown handling in run swallows every exception during p.disconnect() with a bare except: pass, hiding disconnect failures. Replace that blanket handler with a specific exception catch in the p.disconnect cleanup path, and log the failure using a clear message in run so shutdown issues are visible while preserving the existing KeyboardInterrupt and p.error handling.Source: Linters/SAST tools
study/robert/7-1.py-227-241 (1)
227-241: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThread the computed body pose into
calcIKor remove the unused path.body_rot/body_posare derived from the sliders inrun(), butapply_ik()still callscalcIK(..., (0, 0, 0), (0, 0, 0)), so the body-frame IK branch never runs. Ifset_body_pose()is the intended source of truth, drop the dead pose plumbing; otherwise pass the computed pose through and remove the duplicated offset logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/7-1.py` around lines 227 - 241, The IK path is still ignoring the computed body pose, so the body-frame branch in calcIK never gets used. Update apply_ik() and the call site in run() so the slider-derived body_rot/body_pos are passed through consistently, or remove the unused body pose plumbing entirely if set_body_pose() remains the single source of truth. Make sure the duplicated offset handling is not applied twice and that calcIK receives the intended pose from the active path.
🧹 Nitpick comments (21)
Simulation/pybullet_automatic_gait.py (1)
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
idparameter — shadows builtin.Static analysis flags this: the parameter shadows the built-in
id. It also does not appear to be used anywhere in the function body shown.♻️ Suggested rename
-def main(id, command_status): +def main(_id, command_status):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Simulation/pybullet_automatic_gait.py` at line 53, The main function currently uses a parameter named id, which shadows Python’s built-in id and is not used in the function body shown. Rename the parameter in main to a non-builtin name and update any corresponding call sites or references in pybullet_automatic_gait.py so the function signature and usage stay consistent.Source: Linters/SAST tools
Simulation/isaac_hello.py (1)
14-17: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap the render loop so
app.close()always runs.If
world.step(...)raises (or the process receivesKeyboardInterrupt),app.close()is skipped, leaving the simulation app resources unclosed.♻️ Suggested fix
-while app.is_running(): - world.step(render=True) - -app.close() +try: + while app.is_running(): + world.step(render=True) +finally: + app.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Simulation/isaac_hello.py` around lines 14 - 17, The main simulation loop in isaac_hello.py can exit via an exception before app.close() runs, so wrap the app.is_running()/world.step(render=True) loop in a try/finally and move app.close() into the finally block. Keep the fix centered on the existing app, world, and app.close() flow so cleanup always happens even if world.step raises or KeyboardInterrupt occurs.Simulation/isaac_spotmicro.py (1)
40-43: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWrap the render loop so
app.close()always runs.Same concern as
isaac_hello.py: ifworld.step(...)raises,app.close()at line 43 is never reached.♻️ Suggested fix
-while app.is_running(): - world.step(render=True) - -app.close() +try: + while app.is_running(): + world.step(render=True) +finally: + app.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Simulation/isaac_spotmicro.py` around lines 40 - 43, The render loop in the main simulation flow can skip cleanup if world.step() throws, so app.close() may never run. Update the loop around app.is_running() and world.step(render=True) to use a cleanup-safe structure in the same pattern as isaac_hello.py, ensuring app.close() is always reached even when an exception occurs. Keep the fix localized to the app/world stepping block so the shutdown path is guaranteed.study/iru-han/week08/Simulation/spotmicroai.py (3)
20-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBare
except: passsilently swallows import failures.Catching all exceptions (including
ImportErroras well as unrelated errors likeKeyboardInterrupt) and discarding them makes debugging a brokenpybullet/mujocoinstall very difficult — the script will simply fail later with a confusingNameErrorinstead of a clear import error.As per static analysis hints: "Do not use bare `except`" (E722) and "consider logging the exception" (S110).🛠️ Narrow the exception and log it
-try: - import pybullet as p - import pybullet_data -except: - pass +try: + import pybullet as p + import pybullet_data +except ImportError as e: + print(f"pybullet not available: {e}") -try: - import mujoco -except: - pass +try: + import mujoco +except ImportError as e: + print(f"mujoco not available: {e}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/Simulation/spotmicroai.py` around lines 20 - 29, The top-level import handling in spotmicroai.py is too broad because the bare except blocks silently hide pybullet and mujoco import failures, leading to later NameErrors. Update the import guards around the pybullet/pybullet_data and mujoco imports to catch only the expected import-related exception, and add a clear log or message that includes the exception details so missing or broken installs are easy to diagnose.Source: Linters/SAST tools
216-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
bodyPos/bodyOrnin PyBullet control path.These are unpacked but never referenced in the
step()PyBullet branch (handleCamera/addInfoTextare no-op stubs now).As per static analysis hints flagging
RUF059unused unpacked variables.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/Simulation/spotmicroai.py` around lines 216 - 217, In the PyBullet branch of step(), the values returned by getBasePositionAndOrientation are unpacked into bodyPos and bodyOrn but never used. Remove the unused unpacking in that control path or replace it with a direct call if the result is only needed for side effects, and keep the change localized around step(), handleCamera, and addInfoText so the RUF059 warning is eliminated.Source: Linters/SAST tools
42-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefault
xml_pathis relative but not resolved against the script's own directory.Lines 8-11 already show awareness that relative paths break depending on CWD (fixed via
os.path.dirname(os.path.abspath(__file__))forsys.path), but the MuJoCoxml_pathdefault ("../urdf/spot_micro_mujoco.xml") still relies on the process's current working directory rather thancurrent_dir. This will fail to load when the script is invoked from a different working directory.🛠️ Resolve relative to the module location
- def __init__(self, useFixedBase=False, useStairs=True, resetFunc=None, use_mujoco=False, - xml_path="../urdf/spot_micro_mujoco.xml"): + def __init__(self, useFixedBase=False, useStairs=True, resetFunc=None, use_mujoco=False, + xml_path=None): + if xml_path is None: + xml_path = os.path.join(parent_dir, "urdf", "spot_micro_mujoco.xml")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/Simulation/spotmicroai.py` around lines 42 - 59, The MuJoCo initializer in SpotMicroAI.__init__ still uses a relative xml_path default that depends on the current working directory, so resolve it against the module’s directory instead. Update the xml_path handling near the use_mujoco / mujoco.MjModel.from_xml_path path to build an absolute path from the script location (similar to the existing current_dir pattern used elsewhere) before loading the XML, and keep the default behavior working regardless of where the script is launched.study/iru-han/week08/urdf/spot_micro_mujoco.xml (1)
97-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLarge block of commented-out legacy XML.
Roughly 200 lines (two prior model iterations) are left commented out at the end of the file. Consider removing them (or moving to git history) to keep the active model easy to read.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/urdf/spot_micro_mujoco.xml` around lines 97 - 295, The file contains a large commented-out legacy MuJoCo model block that should be removed from the active XML. Delete the unused commented sections around the older model iterations in spot_micro_mujoco.xml, or move them to version control history/notes, so the current model definition remains easy to read and maintain.study/iru-han/week08/Simulation/engine_benchmark.py (1)
63-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo engine teardown between successive benchmark runs.
Running the PyBullet benchmark opens a
p.connect(...)session (viaRobot.__init__) that is never disconnected before the MuJoCo run starts. This is fine for a one-off study script but can leave stray GUI/physics-server state if re-run in the same process.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week08/Simulation/engine_benchmark.py` around lines 63 - 67, The benchmark entry point in engine_benchmark.py starts a PyBullet session via run_benchmark("pybullet") and then immediately runs MuJoCo without cleanup. Add explicit engine teardown between successive benchmark runs by disconnecting/closing the PyBullet session after the pybullet benchmark completes, using the relevant cleanup path in Robot.__init__ / run_benchmark, so reruns in the same process do not leave stray state.study/iru-han/week09/train_spot.py (1)
116-129: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo cleanup on training failure.
If
model.learn(...)raises mid-training,env.close()(Line 129) is skipped, leaving the passive viewer process/window open.♻️ Suggested fix
model = PPO("MlpPolicy", env, verbose=1, tensorboard_log="./ppo_spot_tensorboard/") - - print("-------------- 학습 시작 (화면이 표시됩니다) --------------") - model.learn(total_timesteps=50000) - - print("-------------- 학습 완료 --------------") - model.save("ppo_spot_micro") - env.close() + try: + print("-------------- 학습 시작 (화면이 표시됩니다) --------------") + model.learn(total_timesteps=50000) + print("-------------- 학습 완료 --------------") + model.save("ppo_spot_micro") + finally: + env.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week09/train_spot.py` around lines 116 - 129, The training flow in main() can leak the SpotMicroEnv viewer because env.close() is only reached after model.learn() succeeds. Wrap the model.learn(...) and model.save(...) sequence in a try/finally so the cleanup always runs, and keep env.close() in the finally block. Use the main() function and the SpotMicroEnv instance to locate the change.study/iru-han/week09/test_spot.py (1)
27-46: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider closing the env/viewer on exit.
The test loop is designed to be interrupted with Ctrl+C, but
env.close()is never called, so the MuJoCo passive viewer window/GL context isn't cleaned up on exit.♻️ Suggested cleanup
print("테스트 시작! (Ctrl+C로 종료)") - while True: - action, _ = model.predict(obs, deterministic=True) - obs, reward, done, truncated, info = env.step(action) - if done: - print("넘어짐! 다시 시작합니다.") - obs, _ = env.reset() - time.sleep(1.0) + try: + while True: + action, _ = model.predict(obs, deterministic=True) + obs, reward, done, truncated, info = env.step(action) + if done: + print("넘어짐! 다시 시작합니다.") + obs, _ = env.reset() + time.sleep(1.0) + finally: + env.close()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week09/test_spot.py` around lines 27 - 46, The test loop in test_spot.py does not clean up the MuJoCo environment when interrupted, so add exit cleanup around the main while True loop in the test loop section and ensure env.close() is called on termination. Use the existing env variable from env.reset()/env.step() and wrap the loop so Ctrl+C or any early exit closes the viewer/GL context reliably.study/iru-han/week09/spot_micro_mujoco.xml (1)
25-27: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftUnrealistic mass/actuator scale for a hobby-size quadruped.
Total robot mass here is ~34 kg (base 16.25 kg + 12 leg links × 1.5 kg, Lines 76/81/87/93 etc.), and each joint actuator uses
kp="1000",forcerange="-100 100",ctrlrange="-6.28 6.28"(Line 26) — well beyond both a small quadruped's typical torque budget and the joints' own motionrange(-1.57..1.57/-3.14..3.14). This mismatch (over-powered, over-ranged position actuators driving an overly heavy body) is a likely root cause of the stiff/awkward gait behavior called out inweek09.md's "개선할 점" section.Consider scaling mass/inertia down to the physical SpotMicro robot's actual specs and tightening
ctrlrange/forcerangeto match realistic servo torque and the joint's own range.Also applies to: 76-76, 81-81, 87-87, 93-93
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week09/spot_micro_mujoco.xml` around lines 25 - 27, The actuator and body parameters in the affine default setup are scaled far beyond a hobby quadruped’s realistic limits. Update the robot’s mass/inertia values and the default joint actuator settings in the affine class so the base and leg links better match SpotMicro-scale hardware, and tighten the position actuator kp, ctrlrange, and forcerange to stay within each joint’s actual motion range and plausible servo torque budget. Use the existing affine/default class and the leg link definitions to make the changes consistently across the model.study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py (1)
98-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffMuJoCo state is module-global, not per-instance.
model,data,cam,window,scene,contextare module-level globals rather than attributes ofQuadruped_robot. Multiple instances (e.g.,n_envs>1in-process, or re-instantiating the env) would share and corrupt the same simulation state. Given only a single instance is used acrosstest.py/train.pyin this PR, this doesn't manifest today, but it's a fragile pattern if the training scripts scale to more parallel envs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py` around lines 98 - 134, Move the MuJoCo simulation state out of module globals and into the Quadruped_robot instance so each env owns its own model, data, camera, window, scene, and context. Update the setup currently performed before the class definition to initialize these inside Quadruped_robot (for example in __init__ or a dedicated reset/setup method), and make controller/callback wiring use the instance-owned state instead of shared module-level variables. Ensure any references in init_controller, controller, keyboard, mouse_move, mouse_button, and scroll still resolve against the correct env instance.study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.py (1)
1-476: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused alternate environment variant, not wired to the registered entry point.
This file implements the (correct) Gymnasium-style 5-tuple
step/reset(seed, options)API, butenvs/__init__.pyonly re-exportsQuadruped_robotfromquadruped_robot_env.py, so this variant is currently dead code from the package's perspective. If it's meant as a reference/experiment, consider a comment noting that, or renaming the class/module to avoid confusion with the actively-registeredQuadruped_robot.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.py` around lines 1 - 476, The alternate Quadruped_robot environment variant is not currently used by the package entry point, so it reads as dead/duplicative code. In the Quadruped_robot class, either add a clear module-level note that this implementation is an experimental/reference variant, or rename this class/module so it cannot be confused with the registered environment; also make sure the active export in the package points to the intended implementation. Keep the Gymnasium-style step and reset(seed, options) behavior here only if this variant is meant to stay as the canonical environment.study/kyungho/week01_03/Mujoco_quadruped_RL-main/custom_callback.py (1)
7-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
self.num_timestepscollides withBaseCallback's own counter.
BaseCallback.on_step()already setsself.num_timesteps = self.model.num_timestepsbefore calling_on_step(). Re-initializingself.num_timesteps = 0in__init__and then doingself.num_timesteps += 1here means the manual counter is silently overwritten every call and never actually counts from 0 — the checkpoint cadence ends up tied tomodel.num_timesteps + 1instead of an independent counter, which is confusing and fragile ifnum_envs > 1is ever used.♻️ Use `self.n_calls` (SB3's built-in step counter) instead
def __init__(self, model, env, env_name, verbose: int = 0): super().__init__(verbose=verbose) - self.num_timesteps = 0 self.env_name = env_name self.model = model self.env = env @@ def _on_step(self) -> bool: - self.num_timesteps += 1 - if self.num_timesteps % 10000 == 0: + if self.n_calls % 10000 == 0:Also applies to: 21-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/custom_callback.py` around lines 7 - 13, `CustomCallback` is using `self.num_timesteps` as a manual counter, but that name is already managed by `BaseCallback.on_step()` and gets overwritten each step. Update `CustomCallback.__init__` and `_on_step()` to stop initializing/incrementing `self.num_timesteps`, and use the built-in `self.n_calls` counter for checkpoint cadence instead so the callback’s step tracking stays consistent with `BaseCallback` and works correctly in `custom_callback.py` for the checkpoint logic.study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.py (3)
102-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffModel/data/window state is module-level, not per-instance.
model,data,cam,scene,context, andwindow(Lines 106-129) are global singletons shared by everyQuadruped_robot()instance. This works only becausetrain.py/test.pycreate a singleDummyVecEnvwith one env; any future move to parallel/vectorized envs (e.g.SubprocVecEnv) would have multiple instances silently corrupt each other's simulation state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.py` around lines 102 - 148, The MuJoCo/G GLFW runtime state is created at module scope and shared across all Quadruped_robot instances, which makes the environment non-reentrant and unsafe for multiple envs. Move the initialization of model, data, cam, scene, context, and window into Quadruped_robot.__init__ or a dedicated setup method, and store them on self so each instance owns its own simulation/rendering state. Keep the existing controller hookup and callbacks tied to the instance-owned objects rather than module-level singletons.
146-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffSensor indices are hardcoded and tightly coupled to
a1.xmlsensor ordering.
self.sensorlist = [0, 1, 5, 8]implicitly relies on the exact<sensor>block order inunitree_a1/a1.xml(framexaxis, framepos, framezaxis). Any reordering/addition of sensors in the XML would silently shift these indices and corrupt velocity/height/fall-detection computations without raising an error.Also applies to: 158-158, 161-165, 189-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.py` at line 146, The sensor selection in the environment is hardcoded to the current `a1.xml` sensor order, so update `quadruped_robot_env_origin.py` to resolve sensors by name or another stable identifier instead of fixed indices. Refactor `self.sensorlist` and the related sensor reads used in the observation/termination logic so the code in the environment class still finds the correct `framexaxis`, `framepos`, and `framezaxis` data even if the XML sensor block changes order or new sensors are added.
106-129: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftGLFW window is created unconditionally at import time, regardless of
rend.
glfw.init()/glfw.create_window(...)(Lines 114-125) run at module import, beforesettings(rend, train)is ever called. Even when a caller wants headless training (rend=False), the module still requires a working GLFW/OpenGL context to import successfully, which will fail on displayless training servers/CI.settings()only tears the window down after-the-fact viaglfw.terminate()(Line 202) rather than avoiding creation in the first place.Also applies to: 198-203
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.py` around lines 106 - 129, The GLFW setup in the quadruped environment is happening at module import instead of being gated by the rendering flag, so headless training still tries to create a window. Move the `glfw.init()`, `glfw.create_window(...)`, callback registration, and related OpenGL context setup out of the top-level initialization path and into a render-only branch controlled by `settings(rend, train)` (or the appropriate setup function), using the existing `rend` check to skip window creation when false. Keep `glfw.terminate()` in the teardown path, but make sure the `settings`/environment initialization flow only creates `window`, `scene`, `context`, and camera state when rendering is enabled.study/iru-han/week06_kinematicMotion.py (1)
118-137: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove debug
print()calls from the hot control loop.
print("stay")/print("drag")/print("lift")execute on every leg, every call tocalcLeg, which runs every simulation tick per the real-time control loop documented in week06.md/week07.md. This adds unnecessary I/O overhead to the hot path.♻️ Proposed fix
if(t<self.t0): # stay on ground - print("stay") return startLp elif(t<self.t0+self.t1): # drag foot over ground - print("drag") td=t-self.t0 ... elif(t<self.t0+self.t1+self.t2): # stay on ground again - print("stay") return endLp elif(t<self.t0+self.t1+self.t2+self.t3): # Lift foot - print("lift") td=t-(self.t0+self.t1+self.t2)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week06_kinematicMotion.py` around lines 118 - 137, The calcLeg control path is still doing debug console output on every tick via the print calls in the stay/drag/lift branches. Remove those prints from this hot loop in kinematicMotion.py, or replace them with a gated logger that can be disabled by default, and keep the behavior inside calcLeg unchanged.study/iru-han/week05.py (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid wildcard import.
from math import *pollutes the namespace and Ruff correctly flags this as unable to detect undefined names (F403). All the functions actually used (sqrt,atan2,acos,sin,cos,pi) are known — import them explicitly.♻️ Proposed fix
-from math import * +from math import acos, atan2, cos, pi, sin, sqrt🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/iru-han/week05.py` at line 5, Replace the wildcard math import with explicit imports in the module that defines the math helpers, since `from math import *` in `week05.py` triggers Ruff F403 and hides the real dependencies. Update the top-level import to bring in only the symbols actually used by the code (`sqrt`, `atan2`, `acos`, `sin`, `cos`, `pi`) so the names remain clear and undefined-name detection works properly.Source: Linters/SAST tools
study/minho/work03.md (1)
170-177: 📐 Maintainability & Code Quality | 🔵 TrivialAvoid patching
site-packagesin place.This workaround will disappear on the next venv rebuild or package upgrade, so the setup won't be reproducible. Capture it as a checked-in patch or script the override instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/minho/work03.md` around lines 170 - 177, The workaround in the rsl_rl distribution update should not live as an in-place edit under site-packages. Move the nan_to_num change from the update() path in distribution.py into a checked-in patch or a scripted override that is applied during environment setup, so the fix is reproducible across venv rebuilds and package upgrades.study/robert/6-2.py (1)
359-366: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDon't silently swallow control-loop errors.
The per-leg
except Exception: pass(with unusede) hides IK/setJointMotorControl2failures during walking, making misbehavior hard to diagnose. At minimum log the failure. The bareexcept:blocks at Lines 347, 370, and 405 additionally swallowKeyboardInterrupt/SystemExit; prefer catchingException.♻️ Log instead of discarding
angles = self.foot_to_angles(foot_pos_adjusted) self.set_leg_angles(leg_name, angles) - except Exception as e: - pass + except Exception as e: + print(f"제어 오류 ({leg_name}): {e}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/robert/6-2.py` around lines 359 - 366, The walking control loop is swallowing failures in the per-leg try/except and the other bare except blocks, which hides IK and motor-control errors and also catches KeyboardInterrupt/SystemExit. Update the exception handling around foot_pos_adjusted, foot_to_angles, and set_leg_angles to catch Exception only, remove the unused e where appropriate, and log the failure with enough context to identify the leg and operation instead of using pass.
| # Python 3.12 호환 버전 | ||
| # gym==0.17.2 # 빌드 문제로 주석 처리, 필요시 gymnasium 사용 | ||
| matplotlib>=3.5.0 | ||
| numpy>=1.21.0 | ||
| pybullet==3.0.6 | ||
| keyboard>=0.13.5 | ||
| scipy>=1.7.0 No newline at end of file |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'FILES:\n'
git ls-files Simulation | sed -n '1,120p'
printf '\nREQUIREMENTS:\n'
cat -n Simulation/requirements.txt
printf '\nPYTHON VERSION REFERENCES:\n'
rg -n "3\.12|python_requires|requires-python|Python 3\.12|cp312|numpy>=|matplotlib>=|scipy>=" Simulation -S || trueRepository: robertchoi/SpotMicroJetson
Length of output: 2088
🌐 Web query:
numpy 1.21 Python 3.12 support release notes
💡 Result:
NumPy 1.21 does not support Python 3.12 [1]. The NumPy 1.21.x series officially supports Python versions up to 3.10 [2][3]. Support for newer Python versions, including 3.12, was introduced in subsequent NumPy releases, as NumPy requires updates to its build system and compatibility layers to support newer Python versions [1].
Citations:
- 1: NumPy does not install on Python 3.12.0b1 numpy/numpy#23808
- 2: https://numpy.org/devdocs/release/1.21.3-notes.html
- 3: https://numpy.org/doc/2.1/release/1.21.3-notes.html
🌐 Web query:
matplotlib 3.5 Python 3.12 support
💡 Result:
Matplotlib 3.5 does not officially support Python 3.12 [1]. Matplotlib 3.5 was released in 2021-2022, well before the release of Python 3.12 in October 2023 [1][2]. Because Python 3.12 introduced breaking changes to the Python C API and build process, attempting to use Matplotlib 3.5 with Python 3.12 is not supported and typically fails during installation or compilation [3]. Official support for Python 3.12 was introduced in later versions of Matplotlib (starting with Matplotlib 3.8) [4]. If you must use an environment with Python 3.12, you should upgrade to a newer version of Matplotlib that explicitly supports it [4][2]. If you are forced to use Matplotlib 3.5, you must use a compatible Python version (Python 3.7 through 3.11 are generally associated with the Matplotlib 3.5 era) [1].
Citations:
- 1: https://pypi.org/project/matplotlib/3.5.2/
- 2: https://matplotlib.org/stable/users/release_notes
- 3: [MNT]: install on Python 3.12.0b3 matplotlib/matplotlib#26161
- 4: MNT: Enable wheels for Python 3.12 matplotlib/matplotlib#26582
🌐 Web query:
scipy 1.7 Python 3.12 support
💡 Result:
SciPy 1.7 does not support Python 3.12 [1][2][3]. The SciPy 1.7.x series is compatible with Python 3.7 up to Python 3.10 (for versions 1.7.0/1.7.1) and Python 3.11 (for later patch versions such as 1.7.2+) [1][3][4]. Support for newer Python versions was introduced in subsequent major and minor releases of SciPy [1][3]. For example, support for Python 3.12 was added in later versions of SciPy, starting with SciPy 1.12 [1][2][3].
Citations:
- 1: https://scipy.github.io/devdocs/dev/toolchain.html
- 2: https://docs.scipy.org/doc/scipy-1.18.0/dev/toolchain.html
- 3: https://github.com/scipy/scipy/blob/main/doc/source/dev/toolchain.rst
- 4: DOC: toolchain updates and max Python scipy/scipy#14585
Bump the Python 3.12 dependency floors. numpy>=1.21.0, matplotlib>=3.5.0, and scipy>=1.7.0 do not support Python 3.12, so a clean install can fail on the declared target interpreter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Simulation/requirements.txt` around lines 1 - 7, Update the dependency
minimums in requirements.txt for Python 3.12 compatibility, since the current
floors for numpy, matplotlib, and scipy are too old and can break installs on
the target interpreter. Raise the version constraints for those packages to
Python 3.12-supported releases while keeping the rest of the requirements
unchanged, and make sure the entries in the requirements file reflect the new
floors consistently.
| setup( | ||
| name='gym_spotmicroai', | ||
| version='0.1', | ||
| packages=find_packages(include=['gym_spotmicroai', 'gym_spotmicroai.*']), | ||
| install_requires=['numpy', 'pybullet>=3.0.6', 'matplotlib>=3.5.0', 'scipy>=1.7.0'] | ||
| # gym 제거: 빌드 문제로 인해 선택적 의존성으로 변경 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files ==\n'
git ls-files 'Simulation/setup.py' 'Simulation/pybullet_automatic_gait.py' 'Simulation/*requirements*' 'Simulation/*.egg-info/*' 'requirements*' | sed 's#^`#-` #'
printf '\n== setup.py ==\n'
cat -n Simulation/setup.py
printf '\n== pybullet_automatic_gait imports ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('Simulation/pybullet_automatic_gait.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 'import ' in line or 'from ' in line:
print(f"{i}: {line}")
PY
printf '\n== search for keyboard usage/declaration ==\n'
rg -n --hidden --glob '!**/.git/**' '\bkeyboard\b' Simulation . || trueRepository: robertchoi/SpotMicroJetson
Length of output: 11028
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== egg-info requires ==\n'
cat -n Simulation/gym_spotmicroai.egg-info/requires.txt
printf '\n== egg-info PKG-INFO dependency lines ==\n'
python3 - <<'PY'
from pathlib import Path
p = Path('Simulation/gym_spotmicroai.egg-info/PKG-INFO')
for i, line in enumerate(p.read_text().splitlines(), 1):
if line.startswith('Requires-Dist:') or line.startswith('Requires:'):
print(f"{i}: {line}")
PY
printf '\n== setup.py packaging surface ==\n'
python3 - <<'PY'
from pathlib import Path
text = Path('Simulation/setup.py').read_text()
print('has install_requires:', 'install_requires' in text)
print('has keyboard in setup.py:', 'keyboard' in text)
PYRepository: robertchoi/SpotMicroJetson
Length of output: 530
Add keyboard to the package dependencies.
Simulation/pybullet_automatic_gait.py imports keyboard at module load time, but Simulation/setup.py and the committed egg-info omit it. A fresh pip install . won’t install the dependency, so the simulation can fail on startup. Add keyboard>=0.13.5 to install_requires and refresh the tracked egg-info.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Simulation/setup.py` around lines 3 - 8, The package dependency list in
setup() is missing keyboard, but pybullet_automatic_gait imports it at module
load time. Update the install_requires in setup.py to include keyboard>=0.13.5,
and regenerate the committed egg-info metadata so the dependency is reflected
consistently for fresh installs.
| # 상위 디렉토리를 경로에 추가 | ||
| script_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| project_root = os.path.join(script_dir, "..", "..") | ||
| sys.path.insert(0, project_root) | ||
| sys.path.insert(0, os.path.join(project_root, "Kinematics")) | ||
| sys.path.insert(0, os.path.join(project_root, "Simulation")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify directory depth: locate top-level Kinematics/Simulation/urdf dirs relative to repo root
fd -t d '^(Kinematics|Simulation|urdf)$' --max-depth 2
echo "---"
fd . study/archer/week07 --max-depth 1Repository: robertchoi/SpotMicroJetson
Length of output: 482
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== run_test.py: path setup =="
sed -n '15,28p' study/archer/week07/run_test.py
echo
echo "== run_test.py: URDF path =="
sed -n '154,166p' study/archer/week07/run_test.pyRepository: robertchoi/SpotMicroJetson
Length of output: 1041
project_root needs to go up one more level
study/archer/week07/run_test.py is three directories below the repo root, but project_root = os.path.join(script_dir, "..", "..") only reaches study/. That makes the inserted Kinematics/, Simulation/, and urdf/ paths point at study/*, so the import and URDF load fail. Use os.path.join(script_dir, "..", "..", "..").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@study/archer/week07/run_test.py` around lines 19 - 24, `run_test.py` is
resolving `project_root` too shallow, so the `sys.path` entries for `Kinematics`
and `Simulation` (and the URDF lookup that depends on them) point at `study/`
instead of the repo root. Update the `project_root` assignment in `run_test.py`
to go up one more directory from `script_dir`, and keep the existing
`sys.path.insert` calls so they resolve against the corrected root.
| import mujoco_automatic_gait | ||
|
|
||
| # 1. URDF 파일 읽기 | ||
| # stairs_gen | ||
| try: | ||
| model = mujoco.MjModel.from_xml_path("../urdf/spotmicroai_gen.urdf.xml") | ||
|
|
||
| # 2. MJCF(MuJoCo용 XML)로 저장하기 | ||
| mujoco.mj_saveLastXML("../urdf/spot_micro_mujoco.xml", model) | ||
| print("성공! spot_micro_mujoco.xml 파일이 생성되었습니다.") | ||
| except Exception as e: | ||
| print(f"변환 실패: {e}") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Script crashes: mujoco is never imported.
mujoco.MjModel.from_xml_path(...) (Line 6) and mujoco.mj_saveLastXML(...) (Line 9) reference the mujoco module, but only mujoco_automatic_gait is imported. This raises NameError: name 'mujoco' is not defined on every run — confirmed by Ruff's F821 hints. The import also pulls in unrelated multiprocessing/keyboard-input machinery from mujoco_automatic_gait for no benefit.
🐛 Proposed fix
-import mujoco_automatic_gait
+import mujoco📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import mujoco_automatic_gait | |
| # 1. URDF 파일 읽기 | |
| # stairs_gen | |
| try: | |
| model = mujoco.MjModel.from_xml_path("../urdf/spotmicroai_gen.urdf.xml") | |
| # 2. MJCF(MuJoCo용 XML)로 저장하기 | |
| mujoco.mj_saveLastXML("../urdf/spot_micro_mujoco.xml", model) | |
| print("성공! spot_micro_mujoco.xml 파일이 생성되었습니다.") | |
| except Exception as e: | |
| print(f"변환 실패: {e}") | |
| import mujoco | |
| # 1. URDF 파일 읽기 | |
| # stairs_gen | |
| try: | |
| model = mujoco.MjModel.from_xml_path("../urdf/spotmicroai_gen.urdf.xml") | |
| # 2. MJCF(MuJoCo용 XML)로 저장하기 | |
| mujoco.mj_saveLastXML("../urdf/spot_micro_mujoco.xml", model) | |
| print("성공! spot_micro_mujoco.xml 파일이 생성되었습니다.") | |
| except Exception as e: | |
| print(f"변환 실패: {e}") |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 6-6: Undefined name mujoco
(F821)
[error] 9-9: Undefined name mujoco
(F821)
[warning] 11-11: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@study/iru-han/week08/Simulation/convert_spotmicroai_gent.py` around lines 1 -
12, The script uses the mujoco module in convert_spotmicroai_gent.py without
importing it, so fix the NameError by adding the proper mujoco import and
removing the unnecessary mujoco_automatic_gait import. Update the top-level
imports so the calls in the try block using mujoco.MjModel.from_xml_path and
mujoco.mj_saveLastXML resolve correctly, while avoiding unrelated side effects
from mujoco_automatic_gait.
Source: Linters/SAST tools
| import mujoco_automatic_gait | ||
|
|
||
| # 계단 URDF 변환 | ||
| try: | ||
| stairs_model = mujoco.MjModel.from_xml_path("../urdf/stairs_gen.urdf.xml") | ||
| mujoco.mj_saveLastXML("../urdf/stairs_mujoco.xml", stairs_model) | ||
| print("성공! stairs_mujoco.xml 파일이 생성되었습니다.") | ||
| except Exception as e: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Same missing-import bug as convert_spotmicroai_gent.py.
mujoco.MjModel.from_xml_path(...) (Line 5) / mujoco.mj_saveLastXML(...) (Line 6) will raise NameError since only mujoco_automatic_gait is imported, not mujoco itself — confirmed by Ruff F821.
🐛 Proposed fix
-import mujoco_automatic_gait
+import mujoco📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import mujoco_automatic_gait | |
| # 계단 URDF 변환 | |
| try: | |
| stairs_model = mujoco.MjModel.from_xml_path("../urdf/stairs_gen.urdf.xml") | |
| mujoco.mj_saveLastXML("../urdf/stairs_mujoco.xml", stairs_model) | |
| print("성공! stairs_mujoco.xml 파일이 생성되었습니다.") | |
| except Exception as e: | |
| import mujoco | |
| # 계단 URDF 변환 | |
| try: | |
| stairs_model = mujoco.MjModel.from_xml_path("../urdf/stairs_gen.urdf.xml") | |
| mujoco.mj_saveLastXML("../urdf/stairs_mujoco.xml", stairs_model) | |
| print("성공! stairs_mujoco.xml 파일이 생성되었습니다.") | |
| except Exception as e: |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 5-5: Undefined name mujoco
(F821)
[error] 6-6: Undefined name mujoco
(F821)
[warning] 8-8: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@study/iru-han/week08/Simulation/convert_stairs_gen.py` around lines 1 - 8,
The script has a missing-import issue in the Mujoco conversion flow:
`convert_stairs_gen.py` uses `mujoco.MjModel.from_xml_path` and
`mujoco.mj_saveLastXML`, but only `mujoco_automatic_gait` is imported, so
`mujoco` is undefined. Add the proper `mujoco` import near the top of the file,
matching the fix used in `convert_spotmicroai_gent.py`, so the conversion code
can run without `NameError`.
Source: Linters/SAST tools
| def reset(self): | ||
| self.total_return = 0 | ||
| self.done = False | ||
| mj.mj_resetData(model, data) | ||
| mj.mj_forward(model, data) | ||
| for i in range(12): | ||
| data.ctrl[i] = self.res[i] | ||
| for i in range(10): | ||
| time_prev = data.time | ||
| while (data.time - time_prev < 1.0/60.0): | ||
| mj.mj_step(model, data) | ||
| if self.rend == True: | ||
| create_overlay(model,data, self.episode, self.total_return) | ||
| viewport_width, viewport_height = glfw.get_framebuffer_size(window) | ||
| viewport = mj.MjrRect(0, 0, viewport_width, viewport_height) | ||
| mj.mjv_updateScene(model, data, opt, None, cam, | ||
| mj.mjtCatBit.mjCAT_ALL.value, scene) | ||
| mj.mjr_render(viewport, scene, context) | ||
| for gridpos, [t1, t2] in _overlay.items(): | ||
| mj.mjr_overlay( | ||
| mj.mjtFontScale.mjFONTSCALE_150, gridpos, viewport, t1, t2, context) | ||
| glfw.swap_buffers(window) | ||
| glfw.poll_events() | ||
| _overlay.clear() | ||
| state1 = [data.qpos[i] for i in range(7, 19)] | ||
| state2 = [data.qvel[i] for i in range(18)] | ||
| state3 = [data.sensordata[i] for i in self.sensorlist] | ||
| state = state1 + state2 + state3 | ||
| return state |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
self.last_qvel is never reset at episode start.
reset() reinitializes total_return and done, but not self.last_qvel. On the first step() of a new episode, the energy penalty term (renergy, Line 169) compares fresh qvel against self.last_qvel left over from the end of the previous episode (often post-fall, high-velocity values), producing a spurious/incorrect penalty at every episode boundary. This corrupts the reward signal used for training on the very first step of each episode.
🐛 Reset the velocity history on episode start
def reset(self):
self.total_return = 0
self.done = False
mj.mj_resetData(model, data)
mj.mj_forward(model, data)
+ self.last_qvel = [0 for i in range(18)]
for i in range(12):
data.ctrl[i] = self.res[i]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def reset(self): | |
| self.total_return = 0 | |
| self.done = False | |
| mj.mj_resetData(model, data) | |
| mj.mj_forward(model, data) | |
| for i in range(12): | |
| data.ctrl[i] = self.res[i] | |
| for i in range(10): | |
| time_prev = data.time | |
| while (data.time - time_prev < 1.0/60.0): | |
| mj.mj_step(model, data) | |
| if self.rend == True: | |
| create_overlay(model,data, self.episode, self.total_return) | |
| viewport_width, viewport_height = glfw.get_framebuffer_size(window) | |
| viewport = mj.MjrRect(0, 0, viewport_width, viewport_height) | |
| mj.mjv_updateScene(model, data, opt, None, cam, | |
| mj.mjtCatBit.mjCAT_ALL.value, scene) | |
| mj.mjr_render(viewport, scene, context) | |
| for gridpos, [t1, t2] in _overlay.items(): | |
| mj.mjr_overlay( | |
| mj.mjtFontScale.mjFONTSCALE_150, gridpos, viewport, t1, t2, context) | |
| glfw.swap_buffers(window) | |
| glfw.poll_events() | |
| _overlay.clear() | |
| state1 = [data.qpos[i] for i in range(7, 19)] | |
| state2 = [data.qvel[i] for i in range(18)] | |
| state3 = [data.sensordata[i] for i in self.sensorlist] | |
| state = state1 + state2 + state3 | |
| return state | |
| def reset(self): | |
| self.total_return = 0 | |
| self.done = False | |
| mj.mj_resetData(model, data) | |
| mj.mj_forward(model, data) | |
| self.last_qvel = [0 for i in range(18)] | |
| for i in range(12): | |
| data.ctrl[i] = self.res[i] | |
| for i in range(10): | |
| time_prev = data.time | |
| while (data.time - time_prev < 1.0/60.0): | |
| mj.mj_step(model, data) | |
| if self.rend == True: | |
| create_overlay(model,data, self.episode, self.total_return) | |
| viewport_width, viewport_height = glfw.get_framebuffer_size(window) | |
| viewport = mj.MjrRect(0, 0, viewport_width, viewport_height) | |
| mj.mjv_updateScene(model, data, opt, None, cam, | |
| mj.mjtCatBit.mjCAT_ALL.value, scene) | |
| mj.mjr_render(viewport, scene, context) | |
| for gridpos, [t1, t2] in _overlay.items(): | |
| mj.mjr_overlay( | |
| mj.mjtFontScale.mjFONTSCALE_150, gridpos, viewport, t1, t2, context) | |
| glfw.swap_buffers(window) | |
| glfw.poll_events() | |
| _overlay.clear() | |
| state1 = [data.qpos[i] for i in range(7, 19)] | |
| state2 = [data.qvel[i] for i in range(18)] | |
| state3 = [data.sensordata[i] for i in self.sensorlist] | |
| state = state1 + state2 + state3 | |
| return state |
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 211-211: Loop control variable i not used within loop body
(B007)
[error] 215-215: Avoid equality comparisons to True; use self.rend: for truth checks
Replace with self.rend
(E712)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.py`
around lines 204 - 232, The episode reset in reset() does not clear the velocity
history used by step(), so the first reward after a new episode can inherit
stale qvel values from the previous run. Update reset() in
quadruped_robot_env_origin.py to reinitialize self.last_qvel alongside
total_return and done, using the same state shape expected by step() and the
energy penalty logic (renergy) so the next episode starts with a clean velocity
baseline.
| dirname = os.path.dirname(__file__) | ||
| xml_path = os.path.join(dirname, xml_path) | ||
|
|
||
| model = mj.MjModel.from_xml_path(xml_path) | ||
| data = mj.MjData(model) | ||
| cam = mj.MjvCamera() | ||
| opt = mj.MjvOption() | ||
|
|
||
| init_controller(model, data) | ||
| mj.set_mjcb_control(controller) | ||
|
|
||
| glfw.init() | ||
| window = glfw.create_window(1200, 900, env_name, None, None) | ||
| glfw.make_context_current(window) | ||
| glfw.swap_interval(1) | ||
|
|
||
| mj.mjv_defaultCamera(cam) | ||
| mj.mjv_defaultOption(opt) | ||
| glfw.set_key_callback(window, keyboard) | ||
| glfw.set_cursor_pos_callback(window, mouse_move) | ||
| glfw.set_mouse_button_callback(window, mouse_button) | ||
| glfw.set_scroll_callback(window, scroll) | ||
|
|
||
| scene = mj.MjvScene(model, maxgeom=10000) | ||
| context = mj.MjrContext(model, mj.mjtFontScale.mjFONTSCALE_150.value) | ||
|
|
||
| cam.azimuth = 90 | ||
| cam.elevation = -30 | ||
| cam.distance = 3 | ||
| cam.lookat = np.array([0.0, 0.0, 0.0]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the surrounding code
printf '\n## quadruped_robot_env.py outline\n'
ast-grep outline study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py --view expanded || true
printf '\n## Relevant line ranges\n'
sed -n '1,220p' study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py | cat -n
printf '\n## Search for environment registration / entry points\n'
rg -n "quadruped_robot-v0|register\\(|entry_point|gym\\.make|gymnasium\\.make" study/kyungho/week01_03/Mujoco_quadruped_RL-main -S || true
printf '\n## Search for headless / GUI notes\n'
rg -n "GUI|headless|X11|display|macOS" study/kyungho/week01_03/Mujoco_quadruped_RL-main -S || trueRepository: robertchoi/SpotMicroJetson
Length of output: 12629
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## quadruped_robot/__init__.py\n'
sed -n '1,120p' study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/__init__.py | cat -n
printf '\n## quadruped_robot/envs/__init__.py\n'
sed -n '1,120p' study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/__init__.py | cat -n
printf '\n## package file list\n'
git ls-files study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envsRepository: robertchoi/SpotMicroJetson
Length of output: 2004
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## train.py\n'
sed -n '1,120p' study/kyungho/week01_03/Mujoco_quadruped_RL-main/train.py | cat -n
printf '\n## test.py\n'
sed -n '1,120p' study/kyungho/week01_03/Mujoco_quadruped_RL-main/test.py | cat -nRepository: robertchoi/SpotMicroJetson
Length of output: 3771
Move MuJoCo/GLFW setup out of module import
glfw.init(), glfw.create_window(...), and the MuJoCo model/context setup run as soon as quadruped_robot.envs.quadruped_robot_env is imported. Since quadruped_robot.envs.__init__ imports this module and gym.make('quadruped_robot-v0') resolves through that path, headless runs can fail before Quadruped_robot is ever instantiated. Defer this setup behind the env’s render/train settings.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py`
around lines 98 - 127, The MuJoCo/GLFW initialization is happening at module
import time in quadruped_robot_env.py, which can break headless imports before
Quadruped_robot is instantiated. Move the glfw.init(), glfw.create_window(),
mj.MjModel.from_xml_path(), mj.MjData(), MjrContext, and related camera/scene
setup out of the top-level import path and into a render/setup method or guarded
initialization inside Quadruped_robot so it only runs when rendering is enabled.
Keep the existing controller and callback wiring, but trigger it from the
environment’s runtime flow rather than module import.
| def step(self, action: np.ndarray): | ||
| """ | ||
| action: residual joint position offsets (shape 12, range [-0.5, 0.5]) | ||
| 실제 제어값 = Q_HOMING + action | ||
| 반환: (obs, reward, done, info) | ||
| """ | ||
| action = np.asarray(action, dtype=np.float32) | ||
|
|
||
| # 제어 입력 적용 | ||
| data.ctrl[:] = self.Q_HOMING + action | ||
|
|
||
| # 시뮬레이션 1 제어 주기 진행 | ||
| time_prev = data.time | ||
| while data.time - time_prev < self.dt: | ||
| mj.mj_step(model, data) | ||
|
|
||
| self.curr_step += 1 | ||
|
|
||
| # 관측 / 보상 / 종료 | ||
| obs = self._get_obs(action) | ||
| reward = self._compute_reward(action) | ||
|
|
||
| self.total_return += reward | ||
| self.last_action = action.copy() | ||
|
|
||
| roll, pitch = self._get_rpy() | ||
| done = self._is_done(roll, pitch) | ||
|
|
||
| if done: | ||
| self.plot(self.train) | ||
| self.episode += 1 | ||
|
|
||
| if self.rend: | ||
| self._render_frame() | ||
|
|
||
| return obs, reward, done, {} | ||
|
|
||
| # ========================================================================= | ||
| # reset (Argo-Robot 3.5) | ||
| # ========================================================================= | ||
|
|
||
| def reset(self) -> np.ndarray: | ||
| """ | ||
| 초기 위치/속도에 균일 노이즈를 추가하여 재시작. | ||
| yaw = 0 고정 → body x축이 항상 전진 방향. | ||
| """ | ||
| self.total_return = 0.0 | ||
| self.curr_step = 0 | ||
| self.last_action = np.zeros(12, dtype=np.float32) | ||
|
|
||
| mj.mj_resetData(model, data) | ||
|
|
||
| # 균일 노이즈로 초기 상태 다양화 | ||
| data.qpos[:] = self.qpos_init + np.random.uniform( | ||
| -0.05, 0.05, size=model.nq | ||
| ) | ||
| data.qvel[:] = self.qvel_init + np.random.uniform( | ||
| -0.05, 0.05, size=model.nv | ||
| ) | ||
|
|
||
| # yaw = 0 고정: quaternion [w, x, y, z] = [1, 0, 0, 0] | ||
| data.qpos[3] = 1.0 | ||
| data.qpos[4] = 0.0 | ||
| data.qpos[5] = 0.0 | ||
| data.qpos[6] = 0.0 | ||
|
|
||
| # 초기 제어값 = homing 자세 | ||
| data.ctrl[:] = self.Q_HOMING | ||
|
|
||
| mj.mj_forward(model, data) | ||
|
|
||
| return self._get_obs(self.last_action) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files of interest =="
git ls-files | rg '(^|/)(quadruped_robot_env\.py|quadruped_robot_env__\.py|__init__\.py|test\.py|requirements.*|pyproject\.toml|setup\.py|README\.md)$' || true
echo
echo "== env registration and imports =="
rg -n --no-heading "quadruped_robot-v0|Quadruped_robot|quadruped_robot_env" study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot -g '!**/__pycache__/**'
echo
echo "== file sizes =="
for f in \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/__init__.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/test.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/README.md; do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
fi
done
echo
echo "== relevant snippets =="
for f in \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/__init__.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/test.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/README.md; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '1,220p'
fi
done
echo
echo "== step/reset definitions in both env files =="
for f in \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.py; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n --no-heading "def step|def reset|return .*obs|return .*reward|return .*done|return .*terminated|return .*truncated" "$f"
fi
done
echo
echo "== dependency declarations =="
for f in \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/requirements.txt \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/pyproject.toml \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/setup.py; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: robertchoi/SpotMicroJetson
Length of output: 5066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
root = Path("study/kyungho/week01_03/Mujoco_quadruped_RL-main")
targets = [
root / "quadruped_robot" / "__init__.py",
root / "quadruped_robot" / "envs" / "quadruped_robot_env.py",
root / "quadruped_robot" / "envs" / "quadruped_robot_env__.py",
root / "test.py",
]
for p in targets:
print(f"\n=== {p} ===")
if p.exists():
text = p.read_text(encoding="utf-8", errors="replace")
for needle in ["register(", "quadruped_robot-v0", "DummyVecEnv", "VecNormalize", "def step", "def reset", "return obs, reward", "return self._get_obs", "super().reset"]:
if needle in text:
print(f"-- contains: {needle}")
else:
print("missing")
PYRepository: robertchoi/SpotMicroJetson
Length of output: 939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# A small read-only probe for the SB3 contract mentioned in the review comment.
python3 - <<'PY'
import importlib.util
import sys
from pathlib import Path
spec = importlib.util.find_spec("stable_baselines3")
print("stable_baselines3_spec:", bool(spec))
if spec:
import stable_baselines3
print("stable_baselines3_file:", Path(stable_baselines3.__file__).as_posix())
try:
print("stable_baselines3_version:", stable_baselines3.__version__)
except Exception as e:
print("stable_baselines3_version_error:", e)
PYRepository: robertchoi/SpotMicroJetson
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
pyproject.toml \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/requirements.txt \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/__init__.py \
study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.py; do
echo "=== $f ==="
if [ -f "$f" ]; then
cat -n "$f" | sed -n '1,220p'
else
echo "(missing)"
fi
echo
doneRepository: robertchoi/SpotMicroJetson
Length of output: 9446
Switch the registered env to the Gymnasium API
quadruped_robot/__init__.py still registers quadruped_robot.envs:Quadruped_robot, and quadruped_robot/envs/quadruped_robot_env.py still uses the legacy Gym contract (step() returns 4 values, reset() returns only obs). With gymnasium>=1.2.3 and stable-baselines3>=2.7.1, test.py will fail when DummyVecEnv calls reset(seed=...) and unpacks the 5-value step result. Wire the registry to the Gymnasium-compliant implementation in quadruped_robot/envs/quadruped_robot_env__.py or port this file to the same API.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@study/kyungho/week01_03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py`
around lines 404 - 475, The registered quadruped environment is still using the
legacy Gym API, which is incompatible with the Gymnasium/SB3 path used by the
test harness. Update the environment referenced by Quadruped_robot in
quadruped_robot.envs so it follows the Gymnasium contract: make reset() accept
seed and options and return (obs, info), and make step() return (obs, reward,
terminated, truncated, info) instead of 4 values. If you keep this file, align
it with the Gymnasium-compliant implementation in Quadruped_robot_env__.py and
ensure the registration in __init__.py points to the updated env class.
| # obs = vec_env.reset() | ||
| for _ in range(1000): | ||
| action, _states = model.predict(obs) | ||
| obs, rewards, dones, info = vec_env.step(action) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
obs is undefined — evaluation loop will crash with NameError.
Line 63 comments out # obs = vec_env.reset(), but Line 65 immediately uses obs in model.predict(obs). This is confirmed by the static analysis hint (Ruff F821 "Undefined name obs"). Running this script as-is will raise NameError before any evaluation step executes.
🐛 Uncomment the reset call
-# obs = vec_env.reset()
+obs = vec_env.reset()
for _ in range(1000):
action, _states = model.predict(obs)
obs, rewards, dones, info = vec_env.step(action)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # obs = vec_env.reset() | |
| for _ in range(1000): | |
| action, _states = model.predict(obs) | |
| obs, rewards, dones, info = vec_env.step(action) | |
| obs = vec_env.reset() | |
| for _ in range(1000): | |
| action, _states = model.predict(obs) | |
| obs, rewards, dones, info = vec_env.step(action) |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 65-65: Undefined name obs
(F821)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@study/kyungho/week01_03/Mujoco_quadruped_RL-main/train.py` around lines 63 -
66, The evaluation loop in train.py uses `obs` in `model.predict(obs)` before it
is initialized, which will trigger a `NameError`. Restore the environment reset
in the evaluation setup by initializing `obs` from `vec_env.reset()` before the
`for` loop, and keep the `model.predict`, `vec_env.step`, and loop logic in
place so the rollout starts from a valid observation.
Source: Linters/SAST tools
| Servo servo_armA, servo_armB, servo_armC, servo_armD; | ||
| Servo servo_footA, servo_footB, servo_footC, servo_footD; | ||
|
|
||
| // 서보 배열: index = (type==foot ? 4:0) + (ch-'a') | ||
| // [0]=armA [1]=armB [2]=armC [3]=armD [4]=footA [5]=footB [6]=footC [7]=footD | ||
| Servo* servos[8] = {&servo_armA, &servo_armB, &servo_armC, &servo_armD, | ||
| &servo_footA, &servo_footB, &servo_footC, &servo_footD}; | ||
| int pins[8] = {26, 32, 15, 17, 12, 33, 2, 5}; | ||
| const char* names[8] = {"armA(26)", "armB(32)", "armC(15)", "armD(17)", | ||
| "footA(12)", "footB(33)", "footC(2)", "footD(5)"}; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Expand the sketch to cover the full 12-servo robot.
test_motor.cpp only instantiates 8 channels (arm/foot A-D), but the study docs describe 12 servos total (3 joints × 4 legs). As written, this firmware cannot validate the middle joints or the full wiring contract.
Either add the missing joint channels or explicitly scope this file as an 8-channel subset.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@study/minho/src/test_motor.cpp` around lines 20 - 29, The servo setup in
test_motor.cpp only covers the current arm/foot A-D channels, but the robot is
documented as having 12 servos, so the test sketch is incomplete. Update the
servo declarations, servos/pins/names arrays, and any related indexing logic to
include the missing middle-joint channels for all four legs, using the existing
servo array setup as the place to extend. If this file is intentionally limited
to 8 channels, make that scope explicit in the sketch comments and naming so it
is clear this is not the full 12-servo validation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
study/kyungho/week06/quadruped_infer.py (2)
120-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
int()call.
round()returnsintin Python 3, so the outerint()is unnecessary (RUF046).🧹 Proposed fix
- self.n_substeps = max(1, int(round(CONTROL_DT / self.model.opt.timestep))) + self.n_substeps = max(1, round(CONTROL_DT / self.model.opt.timestep))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week06/quadruped_infer.py` at line 120, The initialization in quadruped_infer.py for `self.n_substeps` has a redundant outer `int()` around `round()`. Update the assignment in the `quadruped_infer` class initialization logic to rely on `round(CONTROL_DT / self.model.opt.timestep)` directly inside `max(1, ...)`, keeping the same behavior while removing the unnecessary conversion.Source: Linters/SAST tools
117-118: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winJoint range indexing assumes joint order matches actuator order — fragile.
self.lowers/self.uppersare indexed fromjnt_range[1:1+nu](joint definition order: FR, FL, HR, HL), butdata.ctrl[:nu]instep()follows actuator definition order (FL, FR, HL, HR). This works correctly only because all legs share identical joint ranges per joint type. If per-leg ranges ever diverge, clipping and control targets will be silently misaligned.Consider indexing limits from
actuator_trnidor documenting the assumption explicitly.♻️ Optional: derive limits from actuator→joint mapping
- self.lowers = self.model.jnt_range[1:1 + self.nu, 0].astype(np.float32) - self.uppers = self.model.jnt_range[1:1 + self.nu, 1].astype(np.float32) + actuator_jnt_ids = self.model.actuator_trnid[:, 0] + self.lowers = self.model.jnt_range[actuator_jnt_ids, 0].astype(np.float32) + self.uppers = self.model.jnt_range[actuator_jnt_ids, 1].astype(np.float32)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@study/kyungho/week06/quadruped_infer.py` around lines 117 - 118, The joint limit lookup in quadruped_infer.py is assuming joint definition order matches actuator order, which makes `self.lowers` and `self.uppers` fragile. Update the limit extraction in the initialization logic near `self.model.jnt_range` to derive per-actuator joint bounds using the actuator-to-joint mapping (for example via `actuator_trnid`) so the limits align with `data.ctrl[:nu]` in `step()`. If you keep the current indexing, add an explicit note in the `self.lowers`/`self.uppers` setup documenting the shared-range assumption.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@study/kyungho/week06/urdf/mini_cheetah.urdf`:
- Line 8: The URDF/MJCF markup contains a stray trailing character after the
closing </mujoco> tag, so clean up the top-level XML in mini_cheetah.urdf to
leave the closing tag exact and valid. Use the </mujoco> element as the anchor,
remove the extra character immediately following it, and verify the file remains
well-formed for strict URDF-to-MJCF parsers.
---
Nitpick comments:
In `@study/kyungho/week06/quadruped_infer.py`:
- Line 120: The initialization in quadruped_infer.py for `self.n_substeps` has a
redundant outer `int()` around `round()`. Update the assignment in the
`quadruped_infer` class initialization logic to rely on `round(CONTROL_DT /
self.model.opt.timestep)` directly inside `max(1, ...)`, keeping the same
behavior while removing the unnecessary conversion.
- Around line 117-118: The joint limit lookup in quadruped_infer.py is assuming
joint definition order matches actuator order, which makes `self.lowers` and
`self.uppers` fragile. Update the limit extraction in the initialization logic
near `self.model.jnt_range` to derive per-actuator joint bounds using the
actuator-to-joint mapping (for example via `actuator_trnid`) so the limits align
with `data.ctrl[:nu]` in `step()`. If you keep the current indexing, add an
explicit note in the `self.lowers`/`self.uppers` setup documenting the
shared-range assumption.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35729dfd-f4d0-4070-935d-3b8f9b2dacac
⛔ Files ignored due to path filters (48)
study/kyungho/week03/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week03/Mujoco_quadruped_RL-main/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_10000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_10000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_100000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_100000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_20000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_20000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_30000/quadruped_robot-v0/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_30000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_30000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_40000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_40000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_50000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_50000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_60000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_60000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_70000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_70000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_80000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_80000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_90000/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_pretrained_model/time_steps_90000/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_reward.pngis excluded by!**/*.pngstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_trained_model/quadruped_robot-v0/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_trained_model/quadruped_robot-v0/ppo_quadruped_robot-v0.zipis excluded by!**/*.zipstudy/kyungho/week03/Mujoco_quadruped_RL-main/PPO_trained_model/quadruped_robot-v0/vec_normalize.pklis excluded by!**/*.pklstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/a1.pngis excluded by!**/*.pngstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/assets/calf.objis excluded by!**/*.objstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/assets/hip.objis excluded by!**/*.objstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/assets/thigh.objis excluded by!**/*.objstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/assets/thigh_mirror.objis excluded by!**/*.objstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/assets/trunk.objis excluded by!**/*.objstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/assets/trunk_A1.pngis excluded by!**/*.pngstudy/kyungho/week06/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week06/meshes/assets/hfield.pngis excluded by!**/*.pngstudy/kyungho/week06/meshes/mini_abad.daeis excluded by!**/*.daestudy/kyungho/week06/meshes/mini_abad.objis excluded by!**/*.objstudy/kyungho/week06/meshes/mini_body.daeis excluded by!**/*.daestudy/kyungho/week06/meshes/mini_body.objis excluded by!**/*.objstudy/kyungho/week06/meshes/mini_lower_link.daeis excluded by!**/*.daestudy/kyungho/week06/meshes/mini_lower_link.objis excluded by!**/*.objstudy/kyungho/week06/meshes/mini_upper_link.daeis excluded by!**/*.daestudy/kyungho/week06/meshes/mini_upper_link.objis excluded by!**/*.objstudy/kyungho/week06/xml/.DS_Storeis excluded by!**/.DS_Storestudy/kyungho/week06/xml/assets/hfield.pngis excluded by!**/*.png
📒 Files selected for processing (23)
study/kyungho/week03/Mujoco_quadruped_RL-main/.vscode/settings.jsonstudy/kyungho/week03/Mujoco_quadruped_RL-main/custom_callback.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/__init__.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/__init__.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/LICENSEstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/README.mdstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/a1.xmlstudy/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/scene.xmlstudy/kyungho/week03/Mujoco_quadruped_RL-main/test.pystudy/kyungho/week03/Mujoco_quadruped_RL-main/train.pystudy/kyungho/week03/README.mdstudy/kyungho/week06/meshes/mini_abad.stlstudy/kyungho/week06/meshes/mini_body.stlstudy/kyungho/week06/meshes/mini_lower_link.stlstudy/kyungho/week06/meshes/mini_upper_link.stlstudy/kyungho/week06/quadruped_infer.pystudy/kyungho/week06/urdf/mini_cheetah.urdfstudy/kyungho/week06/xml/mini_cheetah.xmlstudy/kyungho/week06/xml/scene_hfield_mjx.xmlstudy/kyungho/week06/xml/scene_mjx.xml
💤 Files with no reviewable changes (14)
- study/kyungho/week03/Mujoco_quadruped_RL-main/.vscode/settings.json
- study/kyungho/week03/README.md
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/LICENSE
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/README.md
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/init.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/init.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/test.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/a1.xml
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/unitree_a1/scene.xml
- study/kyungho/week03/Mujoco_quadruped_RL-main/train.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/custom_callback.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env_origin.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env__.py
- study/kyungho/week03/Mujoco_quadruped_RL-main/quadruped_robot/envs/quadruped_robot_env.py
| meshdir="../meshes/" | ||
| balanceinertia="true" | ||
| discardvisual="false" /> | ||
| </mujoco>s |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Stray s after </mujoco> closing tag.
</mujoco>s has an extraneous s character. While lenient XML parsers may ignore it, strict URDF-to-MJCF converters could fail or emit warnings.
🧹 Proposed fix
- </mujoco>s
+ </mujoco>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| </mujoco>s | |
| </mujoco> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@study/kyungho/week06/urdf/mini_cheetah.urdf` at line 8, The URDF/MJCF markup
contains a stray trailing character after the closing </mujoco> tag, so clean up
the top-level XML in mini_cheetah.urdf to leave the closing tag exact and valid.
Use the </mujoco> element as the anchor, remove the extra character immediately
following it, and verify the file remains well-formed for strict URDF-to-MJCF
parsers.
…h calibration servo_controller.py addressed kit2 with the raw servo index (6-11) instead of wrapping to that board's own channels (0-5), which didn't match the simpler wiring test_servos_cali.py already assumed. Both files now wire PCA9685 robertchoi#2 the same way as robertchoi#1: channels 0-5. Also apply the actual DS3230/DS3235 pulse width spec (500-2500usec) instead of ServoKit's default range, which was compressing commanded angle deltas to roughly half the real servo travel. Add RPi5 40-pin GPIO pinout diagram to work05.md, and a per-servo calibration checklist (existing _servo_offsets values vs expected physical pose) to work06.md for verifying the RPi5 port. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
7 participants
Summary by CodeRabbit