Skip to content

tests/led での設定例の追加とTS strict対応#394

Open
stc1988 wants to merge 5 commits intostack-chan:dev/v1.0from
stc1988:strict/led
Open

tests/led での設定例の追加とTS strict対応#394
stc1988 wants to merge 5 commits intostack-chan:dev/v1.0from
stc1988:strict/led

Conversation

@stc1988
Copy link
Copy Markdown
Contributor

@stc1988 stc1988 commented Apr 19, 2026

  • NekomimiLED での動作を確認したので、設定例をコメントとして追記
  • 上記を tests/ledテストアプリで動作確認時に型チェックエラーが発生したので、修正

Summary by CodeRabbit

  • Bug Fixes

    • More robust preference and configuration handling: invalid or non-object values are safely ignored and sensible defaults are used.
    • Safer type handling for engine selection and TTS volume to prevent unexpected values from affecting behavior.
    • Improved LED timing behavior to avoid passing unexpected values to effect scheduling.
  • Tests

    • Added alternative LED configuration presets (commented) for different device models.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 63a63601-9630-4297-a360-21f26fce8ff4

📥 Commits

Reviewing files that changed from the base of the PR and between 1bf46fc and feebca5.

📒 Files selected for processing (2)
  • firmware/stackchan/main.ts
  • firmware/tests/led/main.ts
✅ Files skipped from review due to trivial changes (2)
  • firmware/stackchan/main.ts
  • firmware/tests/led/main.ts

📝 Walkthrough

Walkthrough

Updated preference handling and configuration validation; changed one LED Blink timeline call to pass undefined instead of null; added commented alternative LED test presets for multiple M5 device models.

Changes

Cohort / File(s) Summary
Preference helpers & usage
firmware/stackchan/main.ts
Added asString and asNumber coercion helpers and replaced direct preference access with these helpers when computing engine keys and TTS volume.
Config loading and validation
firmware/stackchan/utilities/loadPreference.ts
Replaced loose any-typed fallback with Record<string, unknown> and added isRecord type guard; ensure structuredClone receives validated record objects or {}.
LED behavior tweak
firmware/stackchan/led/led.ts
Changed Blink.activate(...) to pass undefined (4th arg) to effect.timeline.on(...) instead of null.
Test presets
firmware/tests/led/main.ts
Added commented alternative ledConfig presets for M5Stack Core/Core2/CoreS3 and M5Atom S3; active config unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • meganetaaan

Poem

🐰 I hopped through prefs with careful paws,
Turning odd values into well-typed laws.
I nudged a null to gentle undefined,
And left test LEDs for every kind—
A tiny hop for code, a joyful whisker-twist.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references adding configuration examples to tests/led and TypeScript strict mode compliance, which directly aligns with the changeset covering test configurations, type safety improvements, and strict mode fixes.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@firmware/stackchan/main.ts`:
- Around line 48-50: The asNumber function currently accepts NaN and Infinity
because typeof returns "number"; update asNumber to return the fallback whenever
the input is not a finite number (use Number.isFinite) so corrupted prefs can't
be passed into the Tone constructor (where Tone is created); specifically change
the guard in asNumber to check Number.isFinite(value) and only return value when
true, otherwise return fallback, ensuring callers that build Tone receive only
finite numeric values.

In `@firmware/tests/led/main.ts`:
- Around line 6-9: Fix the typo in the commented board presets: replace
"M5AStack" with "M5Stack" in the two comment lines that precede the ledConfig
presets (the lines containing the commented presets for ledConfig like "// const
ledConfig = { pin: 26, length: 18 }" and "// const ledConfig = { pin: 9, length:
18 }") so the board name matches the existing "M5Stack" comment.
🪄 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: a013b3ee-47ac-4734-8026-d1e405f63303

📥 Commits

Reviewing files that changed from the base of the PR and between ec17d1c and 1bf46fc.

📒 Files selected for processing (4)
  • firmware/stackchan/led/led.ts
  • firmware/stackchan/main.ts
  • firmware/stackchan/utilities/loadPreference.ts
  • firmware/tests/led/main.ts

Comment thread firmware/stackchan/main.ts
Comment thread firmware/tests/led/main.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant