Skip to content

Conversation

@mangelajo
Copy link
Member

@mangelajo mangelajo commented Sep 24, 2025

Summary by CodeRabbit

  • Refactor

    • Replaced the in-device interactive shell/CLI with a unified USB serial interface.
    • Removed the interactive console and its command set (power, storage, meter, monitor, console, config, status).
    • All host/device data paths now use USB serial; background tasks simplified and mode-dependent logic removed.
  • Chores

    • CI workflow: updated artifact upload action to a newer version.

@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2025

Walkthrough

Shell-based CLI was removed and replaced by a unified USB serial interface. application/src/main.rs was refactored to use USBSerialType (shared as usb_serial) and to remove ShellType/ShellStatus. application/src/shell.rs was deleted, removing all CLI commands and helpers.

Changes

Cohort / File(s) Summary
Shell module removal
application/src/shell.rs
Deleted entire shell module: removed ShellType, ShellStatus, prompt/help constants, constructor, command dispatcher, all command handlers and helper utilities.
RTIC main refactor to USB serial
application/src/main.rs
Replaced shell integration with USBSerialType (usb_serial); removed shell/shell_status fields and initializations; renamed serial1usb_serial; updated task signatures/resources (e.g., usart_task, console_monitor_task, usb_task, idle) and routed all I/O through usb_serial; removed escape/console-mode logic.
CI workflow minor updates
.github/workflows/build-firmware.yaml
Updated artifact upload steps to use actions/upload-artifact@v4 (was v3).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Host
  participant USBStack as "USB Device Stack"
  participant USBSer as "USBSerialType\n(usb_serial)"
  participant USBTask as usb_task
  participant Console as console_monitor_task
  participant USART as usart_task

  rect rgb(235,245,255)
  note over Host,USBSer: Unified USB serial path (shell removed)
  Host->>USBStack: USB CDC data (host → device)
  USBStack->>USBSer: RX bytes delivered
  USBSer->>Console: Console reads/forwards host data
  Console-->>USBSer: Device responses written
  USBSer-->>USBStack: TX bytes (device → host)
  USBStack-->>Host: USB CDC data
  end

  rect rgb(245,235,245)
  note over USART,USBSer: DUT ↔ Host bridged via usb_serial
  USART->>USBSer: DUT TX bytes (to host)
  USBSer-->>USBStack: Forward to host
  USBStack-->>Host: Deliver to host
  Host->>USBStack: Host-to-device input
  USBStack->>USBSer: Deliver to usb_serial
  USBSer-->>USART: Optional forwarding to DUT
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I tossed my shell, a tidy hop,
Now USB hums along the crop.
Bytes hop lanes with tidy grace,
Tasks blink on in their new place.
A rabbit rails the serial track—swift, light-footed, ace. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Remove shell support" succinctly and accurately summarizes the primary change: the shell module and its public API were removed and code paths were refactored to use usb_serial instead, with only minor unrelated changes (workflow artifact action updates). It is concise and specific enough for a reviewer to understand the main intent.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch no-shell

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

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/build-firmware.yaml (1)

39-43: Fix trailing space in artifact name; tighten glob; add fail-fast/retention

  • Trailing space in the artifact name (Line 41) will produce a confusing artifact name.
  • The broad glob may upload unintended files; narrow to the expected .cab(s).
  • Add if-no-files-found and retention for reliability.

Apply:

       - name: Archive resulting application
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v4
         with:
-          name: jumpstarter-main.cab 
-          path: application/jumpstarter*
+          name: jumpstarter-main.cab
+          path: application/jumpstarter*.cab
+          if-no-files-found: error
+          retention-days: 30
🧹 Nitpick comments (1)
.github/workflows/build-firmware.yaml (1)

19-23: V4 upgrade LGTM; add fail-fast and retention for robustness

Good move to actions/upload-artifact@v4. Recommend making uploads fail if files are missing and setting explicit retention.

Apply:

       - name: Archive resulting bootloader
-        uses: actions/upload-artifact@v4
+        uses: actions/upload-artifact@v4
         with:
           name: jumpstarter-bootloader-dfu.bin
           path: bootloader/dfu-bootloader.bin
+          if-no-files-found: error
+          retention-days: 30
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd942c7 and cd992bb.

📒 Files selected for processing (1)
  • .github/workflows/build-firmware.yaml (2 hunks)

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.

2 participants