Skip to content
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.

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

Already on GitHub? Sign in to your account

Fix #11 - Onboard LED for pico_w not working #21

Merged
merged 4 commits into from
Jun 27, 2024
Merged

Conversation

Kintar
Copy link
Contributor

@Kintar Kintar commented Jun 26, 2024

This builds on the inclusion of the cygw43_arch_none library from the 0.2 release and makes the following changes:

  • hardware/rp2040/prebuild.cmake
    • Add USING_CYW43 compiler definition (rather than PICO_W of some sort, in case other pico hardware uses this chip)
  • hardware/rp2040/hardware.c
    • now initializes the cyw43 system if building for a pico_w
    • removes guard around init_onboard_led (see next point)
  • hardware/rp2040/onboard_led.c
    • add conditional guards to swap out the appropriate functions for onboard LED access depending on pico variant
  • cli/node_dev.c
    • remove pico_w specific guard against onboard LED

Built using both Windows 11 and MacOS Sonoma, tested on an official Raspberry Pi Pico W.

@Kintar
Copy link
Contributor Author

Kintar commented Jun 26, 2024

Oh, and since I use JetBrains tools for most of my development I committed the .idea directory to ease the effort of other folks who use the same tools. Happy to nuke it and add to .gitignore if there's a reason you don't want it.

Copy link
Owner

@mcknly mcknly left a comment

Choose a reason for hiding this comment

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

Looks good - just a couple minor changes:

  1. Yes let's add .idea/ to gitignore, I'd like to avoid bundling IDE settings for now (I use VScode and there's a lot of system-specific crap in there)
  2. Minor boot print add - see inline comment

Looks good to merge after those changes are made.

if(cyw43_arch_init()) {
uart_puts(UART_ID_CLI, timestamp());
uart_puts(UART_ID_CLI, "Failed to initialize CYW43 hardware.\r\n");
}
Copy link
Owner

Choose a reason for hiding this comment

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

Let's add

else {
    uart_puts(UART_ID_CLI, timestamp());
    uart_puts(UART_ID_CLI, "Initialized onboard wireless module\r\n");
}

to print a success status message at boot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

HAH! I thought I did. Must only be on my wifi branch. :D

@Kintar Kintar requested a review from mcknly June 27, 2024 16:45
@mcknly mcknly changed the base branch from main to v0.3-pre June 27, 2024 18:00
Copy link
Owner

@mcknly mcknly left a comment

Choose a reason for hiding this comment

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

Thanks! I am going to start using a pre-release branching strategy so that only release-up-revs go into main, so I'll merge this into v0.3-pre

@mcknly mcknly merged commit f08115a into mcknly:v0.3-pre Jun 27, 2024
mcknly pushed a commit that referenced this pull request Oct 31, 2024
* chore: add clion .idea directory

* fix(#11): conditional compilation flag to swap out onboard LED logic for pico_w

* fix(#11): remove conditional guard around LED for pico_w, now handled in hardware.c

* fix(#11): add .idea to .gitignore, add status print when module initializes
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