Skip to content

Unify system tick: single entry point for harness and firmware#10

Merged
kaidokert merged 1 commit into
mainfrom
pr3_1
Apr 30, 2026
Merged

Unify system tick: single entry point for harness and firmware#10
kaidokert merged 1 commit into
mainfrom
pr3_1

Conversation

@kaidokert
Copy link
Copy Markdown
Owner

@kaidokert kaidokert commented Apr 30, 2026

New rm32::system::SystemTick provides tick_input() and tick_main() that both harness.rs and firmware main.rs call. Eliminates the harness-vs-firmware divergence that caused repeated regressions.

SystemTick owns InputState (previously duplicated). The ISR tick still runs separately (inline in harness, actual ISR on hardware).

Summary by Sourcery

Unify the main-loop system tick between the firmware and harness by introducing a shared SystemTick entry point used in both binaries.

New Features:

  • Introduce a SystemTick abstraction that encapsulates input processing and main-loop ticking as a shared pipeline for firmware and harness.

Enhancements:

  • Refactor harness and firmware mains to delegate input processing and main-loop execution to SystemTick, removing duplicated InputState handling and reducing divergence between environments.

Summary by CodeRabbit

  • Refactor
    • Introduced a unified system tick abstraction that consolidates input processing and main-loop execution into a single entry point, eliminating code duplication between firmware and test harness implementations.

New rm32::system::SystemTick provides tick_input() and tick_main()
that both harness.rs and firmware main.rs call. Eliminates the
harness-vs-firmware divergence that caused repeated regressions.

SystemTick owns InputState (previously duplicated). The ISR tick
still runs separately (inline in harness, actual ISR on hardware).
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 30, 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: 5af88b62-687e-4884-b386-f04ce2f5bddb

📥 Commits

Reviewing files that changed from the base of the PR and between eaf9af8 and 666e241.

📒 Files selected for processing (4)
  • rm32/src/bin/harness.rs
  • rm32/src/lib.rs
  • rm32/src/system.rs
  • rm32_stm32/src/bin/main.rs

📝 Walkthrough

Walkthrough

Introduces a unified SystemTick abstraction that consolidates input processing and main-loop execution by replacing direct calls to input::process_input and MainState::tick in both harness and firmware. The abstraction owns a shared InputState and provides delegating tick_input and tick_main methods for streamlined control flow.

Changes

Cohort / File(s) Summary
System Tick Abstraction
rm32/src/system.rs, rm32/src/lib.rs
Introduces new SystemTick struct with owned InputState and methods to delegate input processing (tick_input) and main-loop execution (tick_main). Library exports the new system module publicly.
Harness Refactoring
rm32/src/bin/harness.rs
Replaces standalone input_state field and separate process_input/MainState::tick calls with SystemTick instance, now using system.tick_input(...) and system.tick_main(...).
Firmware Main Refactoring
rm32_stm32/src/bin/main.rs
Mirrors harness changes: removes standalone InputState and direct input/main tick invocations, delegating to SystemTick instance methods.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A tick unified, two paths made one,
Input and main now dance as one dance,
No more separate calls—just system.tick run,
Cleaner abstraction, a code-flow romance!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main objective: introducing a unified SystemTick abstraction as a single entry point for both harness and firmware, eliminating code divergence.
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
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 pr3_1

Review rate limit: 4/5 reviews remaining, refill in 12 minutes.

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

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a unified system tick mechanism by adding a new SystemTick struct in rm32/src/system.rs. This change centralizes the input processing and main-loop execution logic, ensuring that both the firmware and the test harness use the same control pipeline. The InputState is now owned by SystemTick, and the harness and firmware have been updated to call tick_input and tick_main respectively. These changes effectively eliminate logic divergence between the simulation and the actual hardware execution. I have no feedback to provide.

@kaidokert kaidokert merged commit 2e5ea91 into main Apr 30, 2026
19 checks passed
@kaidokert kaidokert deleted the pr3_1 branch April 30, 2026 16:46
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