search

mcknly / breadboard-os

A firmware platform aimed at quick prototyping, built around FreeRTOS and a feature-packed CLI
MIT License
500 stars 19 forks source link

feat: add logic to hardware/rp2040/prebuild.cmake to properly add cyw… #9

Closed Kintar closed 1 week ago

Kintar commented 4 weeks ago

Setting the board to "pico_w" in project.cmake as directed in the README does not add the necessary libraries to utilize the cyw43 wifi chip on the pico_w. This PR updates the hardware/rp2040/prebuild.cmake to conditionally add pico_cyw43_arch_none to the hardware_libs CMAKE variable, and to add a project define "WITH_CYW43" to enable conditional compilation of -- for example -- onboard LED code, which does not function on the pico_w due to the change in GPIO pinouts for the wireless module.

Kintar commented 4 weeks ago

Forgot to rebase to your latest changes before opening the PR. 🤦 Fixed that.

Kintar commented 3 weeks ago

You can actually just add pico_cyw43_arch_none to hardware_libs, which at this point is a LIST variable. I found this page helpful.

Yes...that's what this pull does, it automatically adds that library to the hardware_libs list automatically if the board is set to pico_w.

mcknly commented 2 weeks ago

@Kintar would you still like to get this merged in? Just check my comments above. Would love to see what else you come up with for the wireless module on Pico in the future too.

Kintar commented 1 week ago

@Kintar would you still like to get this merged in? Just check my comments above. Would love to see what else you come up with for the wireless module on Pico in the future too.

@mcknly , yes I would, but I don't see any comments from you other than the automated GitHub notification that you self-requested a review, and this comment that I'm replying to. Admittedly, I've been using BitBucket for the past 8 years at work, so I might be out of practice with GitHub, but I didn't think I'd forgotten THAT much! What am I missing? :)

mcknly commented 1 week ago

Ah, maybe it looks different from my dashboard, I thought the comments were visible inline in this thread but maybe they are "review comments" in a different view. Does this work? I just requested a couple of minimal changes before I merge.

mcknly commented 1 week ago

Sorry the link disappeared

Kintar commented 1 week ago

Sorry the link disappeared

@mcknly : That link just takes me back to this comment page. I at least see a "view reviewed changes" button for biot's comment, above, but I don't see any links to show me the files you've reviewed or any recommended changes. In fact, the final part of the page after "add more commets by..." is showing you as a PENDING reviewer. Maybe you need to take an additional action before I can see the changes you requested, since I'm not an actual member of this project?

mcknly commented 1 week ago

Okay seems like maybe a permissions issue then. Thanks for letting me know. Basically just rebase and remove line 11 - here's a screenshot of my comments:
Screenshot_20240620-154427.png

Kintar commented 1 week ago

I think that big green "Finish Your Review" button is why I can't see the comments. :D But I've made the requested changes now.

mcknly commented 1 week ago

Seems like I need some GH training too! Yeah I guess I need to "finish review" before any of it is visible. Sorry for the confusion. Everything looks good with the cyw43 driver build on my end, merged.