General cleanup: remove unused signals and dead code#2
Conversation
- Remove 17 unused signals from llander_top.vhd (vestigial RAM signals from Asteroids Deluxe base that were never connected) - Remove 10 unused signals from llander.vhd (unimplemented sound placeholders and unused declarations) - Remove dead noiserst_l assignment (signal was assigned but never read) - Fix missing clk_6k assignment in clock divider (was declared and read but never driven, causing tone6khz to always be zero) - Remove commented-out old ROM instantiation code from llander.vhd and llander_vg.vhd - Clean up formatting: consistent 2-space indentation, aligned port maps, removed excessive blank lines in llander_top.vhd - Mark 'General cleanup' TODO item as done https://claude.ai/code/session_01QmPirfEXJ1bsJHb9i98uGS
Replace Quartus-specific `work.dpram` shorthand with standard VHDL `entity work.dpram` syntax in all direct entity instantiations. This fixes GHDL analysis errors and makes the code portable across VHDL tools (Quartus, GHDL, ModelSim, etc). https://claude.ai/code/session_01QmPirfEXJ1bsJHb9i98uGS
Uses raetro/quartus-flow with the `pocket` Docker image to compile the core, reverse the bitstream, and package the release artifact. https://claude.ai/code/session_01QmPirfEXJ1bsJHb9i98uGS
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edc9c4a0aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
.github/workflows/build.yml
Outdated
| python3 -c " | ||
| with open('src/fpga/output_files/ap_core.rbf', 'rb') as f: |
There was a problem hiding this comment.
Unindent inline Python passed to
python3 -c
The "Reverse bitstream" step sends a multi-line script to python3 -c, but each statement is indented (with open(...) starts with leading spaces), which Python treats as top-level unexpected indentation and raises IndentationError before any packaging occurs. This makes the workflow fail consistently on GitHub Actions runners; use unindented code (or a here-doc) for the inline script.
Useful? React with 👍 / 👎.
The inline Python passed via `python3 -c` had indented top-level statements, causing an IndentationError on CI. Switch to a heredoc (`<< 'PYEOF'`) so the script is fed via stdin with correct indentation. Also rename `reversed` to `reversed_data` to avoid shadowing the builtin. https://claude.ai/code/session_01QmPirfEXJ1bsJHb9i98uGS
- Compute next patch version from the latest v* git tag - Stamp version and release date into core.json before packaging - Zip the release with Cores/, Platforms/, Assets/ at the root so it can be extracted directly onto an Analogue Pocket SD card - Create a GitHub Release with auto-generated notes (main only) - PR and branch builds still upload artifacts for testing https://claude.ai/code/session_01QmPirfEXJ1bsJHb9i98uGS
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| MINOR=$(echo "$VERSION" | cut -d. -f2) | ||
| PATCH=$(echo "$VERSION" | cut -d. -f3) | ||
| PATCH=$((PATCH + 1)) | ||
| NEXT_VERSION="${MAJOR}.${MINOR}.${PATCH}" |
There was a problem hiding this comment.
Version computation lacks semver validation
Medium Severity
The version computation extracts version components using cut without validating the tag format. If a git tag doesn't follow strict semver format (e.g., v1.0 instead of v1.0.0, or v1.0.0-beta), the PATCH variable could be empty or contain non-numeric characters, causing PATCH=$((PATCH + 1)) to either silently produce incorrect versions or fail with a bash arithmetic error, breaking the CI build and release process.


from Asteroids Deluxe base that were never connected)
placeholders and unused declarations)
but never driven, causing tone6khz to always be zero)
and llander_vg.vhd
maps, removed excessive blank lines in llander_top.vhd
https://claude.ai/code/session_01QmPirfEXJ1bsJHb9i98uGS
Note
Medium Risk
Mostly build/packaging and cleanup, but it also changes synthesizable clock/audio behavior (
clk_6k) and adds automated versioning/releases, which could affect outputs if misconfigured.Overview
Adds a new GitHub Actions workflow (
.github/workflows/build.yml) that compiles the Quartus project, bit-reverses the generated.rbf, auto-increments av*tag-based version, stampscore.json, packagesdist/assets into arelease/artifact, and (onmainpushes) creates a zip and GitHub Release.Cleans up the VHDL sources by removing unused/vestigial signals and commented-out ROM blocks, standardizing
dpraminstantiations toentity work.dpram, and fixing the clock divider inllander.vhdby actually drivingclk_6k(restoring the 6kHz tone path). Also updatessrc/fpga/.gitignoreto ignore*.cf.Written by Cursor Bugbot for commit 2a3b53f. This will update automatically on new commits. Configure here.