8000 Adds iLabs Challenger boards. by PontusO · Pull Request #9562 · adafruit/circuitpython · GitHub
[go: up one dir, main page]
Skip to content

Adds iLabs Challenger boards. #9562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service< 8000 /a> and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Sep 16, 2024
Merged

Adds iLabs Challenger boards. #9562

merged 10 commits into from
Sep 16, 2024

Conversation

PontusO
Copy link
@PontusO PontusO commented Aug 27, 2024

This PR adds the following boards:

  • Challenger+ RP2350 WiFi/BLE5
  • Challenger+ RP2350 BConnect

tannewt
tannewt requested changes Aug 27, 2024
View reviewed changes
Copy link
Member
@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

One pre-commit issue. Good otherwise!

@dhalbert dhalbert changed the title Adds iLabs Challener boards. Adds iLabs Challenger boards. Aug 28, 2024
@dhalbert
Copy link
Collaborator

If you are editing files through the GitHub web interface, it is sometimes hard to get the last lines correct. Pushing from a local copy will make sure that what you think is in the file is really there.

@dhalbert
Copy link
Collaborator

Don't worry about the doc build failure -- we know about that and it is unrelated.

@jepler
Copy link
jepler commented Aug 30, 2024

The good news is, your last push did resolve the pre-commit problem during CI.

The failure was in building the documentation, which is not due to your PR. It can be ignored. (should be fixed by #9580)

The other problem that needs to be fixed is that your latest change also changes the Pico-PIO-USB submodule. Using git add . or git commit -a (rather than individually choosing each file to commit in a program like git gui) is very prone to including more than you intended to change, especially in the case of submodules, due to other design choices in git (just doing a git switch or git checkout does not update submodules, meaning that they become marked as changed)

I made an attempt to fix this but I was not permitted by github to add my change to this PR:

remote: Resolving deltas: 100% (3/3), completed with 3 local objects.
remote: This repository moved. Please use the new location:
remote:   git@github.com:PontusO/circuitpython.git
remote: error: GH006: Protected branch update failed for refs/heads/main.
remote: error: Changes must be made through a pull request.
To github.com:pontuso/circuitpython
 ! [remote rejected]       pr9562 -> main (protected branch hook declined)

so, you will have to resolve this. The steps I followed were roughly:

  • open a shell on ports/raspberrypi/lib/Pico-PIO-USB
  • git fetch origin
  • git checkout fe9133f
  • cd ..
  • git commit -m"Pico-PIO-USB: undo unintended change to submodule" -- Pico-PIO-USB

once you've done these steps and pushed, you should be able to look at the list of files changed on github and see that Pico-PIO-USB is not listed.

I can't recommend strongly enough to use a tool (whether command-line or GUI) that individually selects which files to commit, to avoid irritations like this.

@bablokb
Copy link
bablokb commented Sep 2, 2024

One other thing maybe worth changing for the challenger_rp2350_wifi6_ble: board.ESP_RXD is actually the TX-pin (from the perspective of the MCU). Same with board.ESP_TXD, which is the RX-pin. The very similar challenger_rp2040_wifi_ble has this right (but using the pin-names board.ESP_RX, board.ESP_TX).

I would also keep the pin-names in sync. I assume there will be users upgrading from the rp2040_wifi_ble and they will certainly be more happy if they don't need to change their programs.

@PontusO PontusO requested a review from tannewt September 16, 2024 08:13
tannewt
tannewt approved these changes Sep 16, 2024
View reviewed changes
Copy link
Member
@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Thank you! Please also make a PR to https://github.com/raspberrypi/usb-pid to publicly document the PIDs.

@tannewt tannewt merged commit b383cbe into adafruit:main Sep 16, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@PontusO @dhalbert @jepler @bablokb @tannewt
0