Skip to content

step 4/6: CoreS3メモリ設定整理とドライバ最適化#366

Open
meganetaaan wants to merge 2 commits intodev/v1.0from
step-pr/04-runtime-tuning
Open

step 4/6: CoreS3メモリ設定整理とドライバ最適化#366
meganetaaan wants to merge 2 commits intodev/v1.0from
step-pr/04-runtime-tuning

Conversation

@meganetaaan
Copy link
Copy Markdown
Collaborator

@meganetaaan meganetaaan commented Feb 28, 2026 •
edited by coderabbitai Bot
Loading

Summary

  • Move CoreS3 memory settings into manifest platform section
  • Remove sdkconfig overrides and the obsolete 8MB flash partition manifest
  • Reduce torque path allocation in dynamixel driver (Promise.all -> sequential await)

Scope

  • Base: dev/v1.0
  • Head: step-pr/04-runtime-tuning
  • This is step 4 of 6.

Verification

  • npm run build in firmware/ for default esp32/m5stack: pass
  • npm_config_target=esp32/m5stack_cores3 npm run build in firmware/: pass

Summary by CodeRabbit

  • Refactor

    • Optimized driver performance with improved execution strategy.

  • Documentation

    • Removed outdated documentation sections regarding MOD write partition configuration.

  • Chores

    • Updated platform-specific memory and flash configuration for ESP32 M5Stack CoreS3.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 28, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d9172dd-b4b2-47a8-91ef-7bf5166b1e0f

📥 Commits

Reviewing files that changed from the base of the PR and between 781e0f7 and ed6afa2.

📒 Files selected for processing (6)
  • firmware/docs/flashing-firmware.md
  • firmware/docs/flashing-firmware_ja.md
  • firmware/stackchan/drivers/dynamixel-driver.ts
  • firmware/stackchan/manifest.json
  • firmware/stackchan/manifest_8mb_flash.json
  • firmware/stackchan/sdkconfig/sdkconfig.defaults
💤 Files with no reviewable changes (4)
  • firmware/docs/flashing-firmware.md
  • firmware/stackchan/sdkconfig/sdkconfig.defaults
  • firmware/stackchan/manifest_8mb_flash.json
  • firmware/docs/flashing-firmware_ja.md
✅ Files skipped from review due to trivial changes (1)
  • firmware/stackchan/drivers/dynamixel-driver.ts
📝 Walkthrough

Walkthrough

Convert concurrent torque updates to sequential processing; move global creation/memory settings into an esp32/m5stack_cores3-specific manifest block; remove default 8MB flash-size setting and related docs/manifest_8mb references.

Changes

Cohort / File(s) Summary
Dynamixel Driver Control Flow
firmware/stackchan/drivers/dynamixel-driver.ts		 Replaced concurrent Promise.all + .map torque application with sequential for...of await calls; still updates this._torque and applies enable/disable to all _controls.
Platform Memory Manifest
firmware/stackchan/manifest.json		 Removed top-level global creation block; added platform-scoped creation settings for esp32/m5stack_cores3 with explicit static, chunk, heap, stack, and keys.initial/keys.incremental values.
Flash / SDK Config & Docs
firmware/stackchan/sdkconfig/sdkconfig.defaults, firmware/docs/flashing-firmware.md, firmware/docs/flashing-firmware_ja.md, firmware/stackchan/manifest_8mb_flash.json		 Deleted default SDK flash-size selection (8MB) and removed documentation and manifest file that instructed including an 8MB-flash manifest and custom partitions; partition-related build settings removed.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested reviewers

  • stc1988

Poem

🐰 I hopped through code with nimble paws,
One torque at a time — no cause for pause.
Memory tucked to platform soil,
Flash defaults gone — a lighter toil.
Firmware fluffs its tiny tail and cheers! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is partially related to the changeset; it refers to real aspects of the changes (CoreS3 memory configuration and driver optimization) but uses Japanese text and is vague about specific changes, making it less clear to English-speaking developers scanning history. Consider using an English title that more specifically describes the main changes, such as 'Move CoreS3 memory settings to manifest and optimize torque driver' for better clarity and accessibility.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch step-pr/04-runtime-tuning

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@meganetaaan meganetaaan force-pushed the step-pr/03-chat-ui-hotpath branch from 4061491 to b6843ad Compare March 11, 2026 23:34
@meganetaaan meganetaaan force-pushed the step-pr/04-runtime-tuning branch from 739bc9f to 2d9bbf2 Compare March 11, 2026 23:34
@meganetaaan
Copy link
Copy Markdown
Collaborator Author

meganetaaan commented Apr 26, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@meganetaaan meganetaaan force-pushed the step-pr/04-runtime-tuning branch from 781e0f7 to ed6afa2 Compare April 26, 2026 06:27
@meganetaaan meganetaaan changed the base branch from step-pr/03-chat-ui-hotpath to dev/v1.0 April 26, 2026 06:27
@meganetaaan
Copy link
Copy Markdown
Collaborator Author

meganetaaan commented Apr 26, 2026

@coderabbitai reviewして。Moddableのupstream最新リリースでビルド失敗に繋がる不具合が残存しているため、CI結果は一旦無視して

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

@meganetaaan レビューを開始します！Moddableのupstream最新リリースに起因するビルド失敗についてはCI結果を無視してレビューします。

✏️ Learnings added 
Learnt from: meganetaaan
URL: https://github.com/stack-chan/stack-chan/pull/366

Timestamp: 2026-04-26T06:50:28.878Z
Learning: In the stack-chan repository, CI failures caused by Moddable upstream latest release compatibility issues should be ignored during code review. This is a known, ongoing issue as of early 2026.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@meganetaaan
Copy link
Copy Markdown
Collaborator Author

meganetaaan commented Apr 26, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 26, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@meganetaaan meganetaaan mentioned this pull request Apr 26, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@meganetaaan