recomputer-rk35xx-common: drop bluez juggle vs fcs960k-aic-bluez - #10358
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughSeeed RK35xx configuration now provides default-on feature flags with independent controls. Package installation uses shared apt helpers and conditional package lists. AIC8800, Morse, USB gadget, and USB automount setup can be skipped independently. ChangesSeeed feature controls
Estimated code review effort: 2 (Simple) | ~15 minutes Merge Risk: ⚪ Minimal · up to The change updates vendor feature defaults and package handling without any identified merge-blocking risk at the current head. Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc`:
- Around line 299-303: Update the skip alert in the SEEED_AUTOMOUNT_ENABLE guard
to report that USB auto-mount is being skipped, replacing the incorrect “USB
gadget defaults” wording while preserving the existing condition and
display_alert behavior.
- Around line 62-64: Update the repository preparation sequence around
chroot_sdcard_apt_get_update and chroot_sdcard_apt_get_install_download_only to
retry transient failures before aborting. Reuse the existing retry helper or
pattern available in the surrounding build configuration, while preserving the
current ordering and final chroot_sdcard_apt_get_install "$@" invocation.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f91b50b9-e422-45b3-9992-cd1273b7598b
📒 Files selected for processing (2)
config/boards/recomputer-rk3576-devkit.confconfig/sources/vendors/seeed-studio/recomputer-rk35xx-common.inc
8acdf33 to
add4f20
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Hi @rpardini, Thanks a lot for the feedback! As a side note, we've actually been working on our own patch for this and it's currently being tested — but we'd prefer to go with your approach here. We'll run our tests on it as soon as possible and get back to you with the results. |
|
Hi @rpardini, We've tested this patch, and Bluetooth is working fine now. |
add4f20 to
76a8df8
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@rpardini can you merge this patch? |
Not until someone approves this PR -- I've no special merge powers... |
…install - so local cache is correctly used and doesn't leak into image - do `chroot_sdcard_apt_get_install_download_only` before actual install
- since Seeed-Studio/seeed_armbian_extension#4 landed, it can now simply replace bluez
- `SEEED_USB_GADGET_ENABLE` - `SEEED_MORSE_ENABLE` - `SEEED_AIC_BLUEZ_ENABLE` - `SEEED_MORSE_ENABLE` - `SEEED_AUTOMOUNT_ENABLE` - those all default to `SEEED_DEFAULT_ENABLE`, which defaults to `yes`
- `SEEED_AIC8800_ENABLE` - `SEEED_AIC_BLUEZ_ENABLE` (shared with common) - those all default to `SEEED_DEFAULT_ENABLE`, which defaults to `yes`
76a8df8 to
3d09600
Compare
|
✅ This PR has been reviewed and approved — all set for merge! |
recomputer-rk35xx-common: drop bluez juggle vs fcs960k-aic-bluez
chroot_sdcard_apt_get_install_download_onlybefore actual installSEEED_USB_GADGET_ENABLESEEED_MORSE_ENABLESEEED_AIC_BLUEZ_ENABLESEEED_MORSE_ENABLESEEED_AUTOMOUNT_ENABLESEEED_DEFAULT_ENABLE, which defaults toyesSEEED_AIC8800_ENABLESEEED_AIC_BLUEZ_ENABLE(shared with common)SEEED_DEFAULT_ENABLE, which defaults toyesSummary by CodeRabbit
New Features
Bug Fixes