Skip to content

Conversation

@KumarADITHYA123
Copy link

@KumarADITHYA123 KumarADITHYA123 commented Nov 29, 2025

Hello!
I added a new Clock Divider component to help with issue #587.

Why I did this: Making slow clocks for projects was hard because users had to connect many flip-flops by hand. This new component makes it much easier and faster to use.

What I added:
A new
Clock Divider
component in the "Sequential Elements" menu.

You can change the speed (divide by 2, 4, 8, etc.) in the settings.
It shows the number (like "÷2") on the component so you can see the speed.

Testing: I checked that it shows up in the menu, connects to wires, and correctly slows down the clock speed when I change the settings.

Closes #587

Summary by CodeRabbit

  • New Features
    • Added a Clock Divider element under Sequential Elements to divide an input clock's frequency.
    • Configurable divide factor (user-settable, 2–100) with persistent counting and live output updates.
    • Visual representation shows a labeled divider indicator for quick identification and placement.

✏️ Tip: You can customize this high-level summary in your review settings.

- Create ClockDivider module for dividing clock frequencies by a configurable factor
- Support divide factors: 2, 4, 8, 16, and custom values
- Register component in module system for simulator availability
- Add Clock Divider to Sequential Elements menu in UI
- Component displays division factor (N) on canvas for visual clarity
- Uses internal counter-based logic for clock division
- Output frequency = Input frequency / divideFactor
- Simplifies timing visualization for educational projects
@netlify
Copy link

netlify bot commented Nov 29, 2025

Deploy Preview for circuitverse ready!

Name Link
🔨 Latest commit ceee37e
🔍 Latest deploy log https://app.netlify.com/projects/circuitverse/deploys/692ad6e61bce34000849e7fe
😎 Deploy Preview https://deploy-preview-680--circuitverse.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 34 (🔴 down 10 from production)
Accessibility: 73 (no change from production)
Best Practices: 92 (no change from production)
SEO: 82 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 29, 2025

Walkthrough

Adds a new ClockDivider circuit element: metadata entries to expose it in the UI, module registration, and a new ClockDivider class implementing divideFactor, edge-detection counter logic, serialization, and drawing.

Changes

Cohort / File(s) Summary
Metadata updates
src/simulator/src/metadata.ts
Inserted ClockDivider into circuitElementList, inputList, and subCircuitInputList; added { name: "ClockDivider", label: "Clock Divider" } under "Sequential Elements" in elementHierarchy.
Module registration
src/simulator/src/moduleSetup.js
Imported ClockDivider from ./modules/ClockDivider and registered it in the exported moduleSet.
Clock Divider implementation
src/simulator/src/modules/ClockDivider.js
New default-exported ClockDivider class extending CircuitElement with divideFactor (default 2), counter, prevClockState, clock and output nodes; methods constructor, setDivideFactor, customSave, resolve, and customDraw; prototype metadata (tooltipText, helplink, objectType).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

  • Inspect rising-edge detection and counter wrap logic in resolve().
  • Verify when simulationQueue updates are enqueued and output timing relative to input clock.
  • Validate setDivideFactor bounds/clamping and counter reset behavior.
  • Check customSave() for correct serialization of constructor parameters and node references.
  • Review customDraw() for correct sizing/labeling and any canvas state interactions.

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding a Clock Divider component. It is specific, directly related to the changeset, and follows conventional commit format.
Linked Issues check ✅ Passed All coding requirements from issue #587 are met: Clock Divider component added to Sequential Elements menu, accepts Clock input with configurable divide factor (default 2, range 2-100), outputs slower clock signal, displays divide factor on component face.
Out of Scope Changes check ✅ Passed All changes are directly within scope: metadata registration, module setup, and Clock Divider implementation. No unrelated modifications detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
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: 1

🧹 Nitpick comments (1)
src/simulator/src/modules/ClockDivider.js (1)

89-107: Minor text rendering nit: rely on fillText’s fontSize parameter

fillText already sets ctx.font based on a fontSize argument, so the manual ctx.font = '14px Raleway' is overridden and effectively unused. You can simplify and ensure consistent sizing by passing the desired font size directly:

-        ctx.font = '14px Raleway'
         ctx.fillStyle = colors['input_text']
         ctx.textAlign = 'center'
-        fillText(ctx, '÷' + this.divideFactor, xx, yy + 5)
+        fillText(ctx, '÷' + this.divideFactor, xx, yy + 5, 14)

This keeps the API usage consistent with other drawing helpers.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11b4883 and b13f1cd.

📒 Files selected for processing (3)
  • src/simulator/src/metadata.ts (4 hunks)
  • src/simulator/src/moduleSetup.js (2 hunks)
  • src/simulator/src/modules/ClockDivider.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/simulator/src/modules/ClockDivider.js (3)
src/simulator/src/node.js (2)
  • Node (150-1034)
  • findNode (87-89)
src/simulator/src/themer/themer.ts (1)
  • colors (47-47)
src/simulator/src/canvasApi.js (3)
  • correctWidth (401-403)
  • rect2 (370-386)
  • fillText (513-522)
🔇 Additional comments (6)
src/simulator/src/moduleSetup.js (1)

10-10: ClockDivider import and registration look consistent

The new import path and moduleSet registration are aligned with existing patterns and metadata names; this will correctly expose ClockDivider to the simulator.

Also applies to: 127-127

src/simulator/src/metadata.ts (1)

41-41: Metadata wiring for ClockDivider is coherent

ClockDivider is added consistently to circuitElementList, inputList, subCircuitInputList, and the "Sequential Elements" hierarchy, matching the module name and desired palette placement.

Also applies to: 89-89, 106-106, 164-164

src/simulator/src/modules/ClockDivider.js (4)

17-37: Constructor and node setup follow existing CircuitElement patterns

The constructor wiring (direction RIGHT, bitWidth 1, rectangle geometry, input clock and output output) and mutableProperties.divideFactor configuration look consistent with other modules and should integrate cleanly with the property panel.


47-61: customSave shape matches standard module persistence

Persisting only divideFactor in constructorParamaters and serializing the clock/output nodes via findNode aligns with how other stateful elements are saved and reconstructed. No issues here.


63-83: Edge detection and division logic are sound for the intended use

  • Rising‑edge detection via this.clock.value !== this.prevClockState && this.clock.value === 1 avoids double‑counting during high/low phases.
  • Counter wrap at this.divideFactor and the subsequent high/low computation give a divided clock with period scaled by divideFactor. Even factors yield a clean 50% duty cycle; odd factors produce a slightly asymmetric duty cycle but still the correct period.

Given the educational/visual focus, this is a reasonable and efficient implementation.


110-112: Prototype metadata is well‑formed

tooltipText, helplink, and objectType are set as expected. Once dedicated docs exist, you may want to point helplink to the specific Clock Divider section, but the current placeholder is fine for now.

Copy link
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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b13f1cd and 713ef21.

📒 Files selected for processing (1)
  • src/simulator/src/modules/ClockDivider.js (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/simulator/src/modules/ClockDivider.js (3)
src/simulator/src/node.js (2)
  • Node (150-1034)
  • findNode (87-89)
src/simulator/src/themer/themer.ts (1)
  • colors (47-47)
src/simulator/src/canvasApi.js (3)
  • correctWidth (401-403)
  • rect2 (370-386)
  • fillText (513-522)
🔇 Additional comments (2)
src/simulator/src/modules/ClockDivider.js (2)

72-88: Clock division logic looks correct.

The rising-edge detection, counter wrap, and 50% duty-cycle output calculation correctly implement frequency division. Output toggles at divideFactor/2 boundaries producing the expected f/N waveform.


115-117: Metadata looks good; consider updating helplink when documentation is available.

The tooltip and objectType are appropriate. The placeholder help link is fine for now but should eventually point to component-specific documentation.

Copy link
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: 0

🧹 Nitpick comments (1)
src/simulator/src/modules/ClockDivider.js (1)

108-111: Redundant font assignment on line 108.

Since fillText on line 111 receives the font size (14) as its fourth argument and sets ctx.font internally, line 108 has no effect. Consider removing it for clarity.

-        ctx.font = '14px Raleway'
         ctx.fillStyle = colors['input_text']
         ctx.textAlign = 'center'
         fillText(ctx, '÷' + this.divideFactor, xx, yy + 5, 14)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 713ef21 and ceee37e.

📒 Files selected for processing (1)
  • src/simulator/src/modules/ClockDivider.js (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-24T05:25:39.773Z
Learnt from: 092vk
Repo: CircuitVerse/cv-frontend-vue PR: 644
File: src/simulator/src/utils.ts:54-61
Timestamp: 2025-08-24T05:25:39.773Z
Learning: In CircuitVerse simulator, clearing the clock interval (simulationArea.ClockInterval) during pause can cause unexpected results. The safer approach is to let the clock continue ticking and use flag checks in clockTick() to prevent simulation updates.

Applied to files:

  • src/simulator/src/modules/ClockDivider.js
🔇 Additional comments (6)
src/simulator/src/modules/ClockDivider.js (6)

1-5: Imports are correctly utilized.

All imported modules and functions are used appropriately throughout the class.


17-37: Constructor implementation looks good.

The initialization properly sets up dimensions, nodes, and mutable properties. The divide factor bounds (min: 2, max: 100) are correctly configured per past review feedback.


39-50: Setter implementation is robust.

The clamping logic (2–100), NaN check, and counter reset on change are all correctly implemented.


57-66: Serialization format is correct.

The spelling fix (constructorParameters) has been applied, and the save structure follows the expected pattern.


72-88: Core clock division logic is correct.

The rising-edge detection, counter wrap, and 50% duty-cycle output computation are all properly implemented. The output is only pushed to the simulation queue when it changes, which is efficient.

Minor note: The modulo on line 82 is technically redundant since counter is already bounded by lines 78–80, but it doesn't affect correctness.


115-117: Prototype metadata is properly defined.

The objectType matches the class name, and the tooltip is descriptive. The help link is a placeholder pointing to the docs root—consider updating it to a specific page (e.g., /sequential-elements/clock-divider) once documentation is available.

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.

Feature: Add Clock Divider Component

1 participant