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.

Sign up for GitHub

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

Jump to bottom

20 wifi support - Initial Work #22

Merged
merged 11 commits into from
Jul 19, 2024
Merged

20 wifi support - Initial Work #22

merged 11 commits into from
Jul 19, 2024

Conversation

Kintar
Copy link
Contributor

@Kintar Kintar commented Jul 8, 2024

Well, that was a wild journey, and it's still not QUITE working. At this point, the wifi connection works, but the NVM system fails to initialize.

I've done quite a few things in this branch, let me give a quick overview:

  • Reworked the way the platform/rp2040 directory is handled, creating an INTERFACE library instead of adding sources directly to the main project
  • Updated the FreeRTOS_Config.h file to bring in changes necessary for the pico FreeRTOS and lwIP ports
  • Made more tweaks to the hardware_config files to (hopefully) properly initialize cyw43 when wifi isn't needed but skip that init when the wifi service is enabled

There's still an issue with the watchdog forcing a reboot occasionally, and wifi intermittently fails to connect if the watchdog restarts the system.

Also, I still can't get the _async version of the wifi connection to function correctly, so the wifi service uses a blocking connect call, although this doesn't actually block the RTOS, just that task.

Kintar added 10 commits June 27, 2024 16:51
@Kintar
refactor: cmake and interfaces
a24cbce 
Retooled the cmake configuration such that the hardware library
is now an INTERFACE library, rather than directly modifying the
root project. This allows us to remove numerous link_lib defs
from the cli and root project, as well as provides an easier method
to pass hardware-level compiler definitions up to the main project
in the cases where that is necessary.
@Kintar
feat(mcknly#20): need configUSE_PASSIVE_IDLE_HOOK defined for SMP ver…
3f0a0db 
…sions of FreeRTOS
@Kintar
feat(mcknly#20): more necessary FreeRTOS config updates
7323fbf
@Kintar
feat(mcknly#20): include lwip_sys_freertos lib, decrease heap space a…
73857af 
…ccordingly
@Kintar
wip: wifi functions
fd6e09e
@Kintar
wip: implemented all necessary wifi wrapper func
732ad59
@Kintar
wip: correctly toggle hw_wifi.c inclusion based on HW_WIFI
38f7e6b
@Kintar
wip: first pass at the actual service
9eee055
@Kintar
fix(mcknly#20): apparently, configNUM_CORES still has to be defined!?…
00f3779 
…!?!?
@Kintar
fix(mcknly#20): watchdog priority, update cli printing from wifi
7f7af47
@Kintar Kintar linked an issue Jul 8, 2024 that may be closed by this pull request
WiFi Support #20
Open
@mcknly
Copy link
Owner

mcknly commented Jul 18, 2024

@Kintar - I'm finally back in front of a screen and taking a look at this. First off great work hammering on this and getting something functional. A few comments right off the bat:

  • Most likely the watchdog reboots are happening because of the blocking connect - it will block kicking the dog based on priority and resulting OS timer tick task switching schedule
  • I see you increased watchdog priority, I assume to alleviate the above. Watchdog needs to stay lowest priority otherwise it will not do its job properly. We'll have to figure out how to make the connect non-blocking, or some other workaround.
  • I still suspect some reset issues when RP2040 reboots but CYW43 does not. Will have to figure out some method to force-reset the wireless module which may clear up some of the re-connect issues
  • Looks like there are some changes present that already got merged in Fix #11 - Onboard LED for pico_w not working #21, but it's on v0.3-pre branch. I'll merge that into the 20-wifi-support branch, you can rebase off there, and that will be our working branch for this feature (stand by).
  • You've done some good things that probably address some of Decouple project top-level from RP2040/Pico #8 - I have to look at this more

That's as far as I've gotten from reading your code from a phone screen. Thanks again. Will be in touch shortly.

@mcknly
Copy link
Owner

mcknly commented Jul 18, 2024

20-wifi-support branch is up to date with your other CYW43 library/LED changes. Use that as your upstream, rebase and update your PR and we can work off that as a feature branch.

@Kintar Kintar changed the base branch from main to 20-wifi-support July 18, 2024 12:37
@Kintar
Copy link
Contributor Author

Kintar commented Jul 18, 2024 •
edited
Loading

Welcome back, @mcknly! I hope the self-punative bike ride was a success. ;)

  • From what I could determine, the "blocking" connect actually uses an RTOS async connect under the hood when the FreeRTOS lib is in use, it just doesn't return until the link is up or failed. This is indeed why I changed the watchdog priority, so that the two tasks would share time slices until WiFi was done with its work, but this was a stop-gap during debugging. I thought about just disabling the watchdog until WiFi was up, but then I discovered the SDK doesn't have a method to disable the pupper once it's let off its leash. :/ It looks like it's possible, but the register writes are a bit beyond my comfort zone in MCU code at the moment. Therefore, config twiddling. I just forgot to add the todo comment to remind myself to change it back.
  • Issues with the rp2040 resetting and CYW43 not resetting are almost certainly part of the culprit. Over the last week, I've run across a few threads online that claim the hardware reset on the Pico (not the Pico W) doesn't actually reset everything on the board. It would surprise me absolutely none if this issue was also relevant to the wifi module on the Pico W. Not really sure what to do about that.
  • I've adjusted the PR to target the feature branch. Since the changes I added from v0.3-pre were part of the git graph on both repos after your merge, the rebase on my side was functionally a no-op. You can verify this by looking at the oneline output from git log. It shows 20-wifi-support and v0.3-pre from your repository as sharing the same commit, f08115a, which is part of the history of my 20-wifi-support branch.
  • As for the changes related to Decouple project top-level from RP2040/Pico #8, glad you approve, but the update was entirely self-serving. I was having a hard time reasoning about the link and CMake graphs in my head without making those changes. XD Anything you'd like changed on this branch to better fit with your idea of project strucure, feel free. Now that I have the compiler playing nicely with the wifi toggle, I don't need to keep thinking about that stuff. :D

Looking forward to hearing more from you!

@Kintar Kintar changed the title 20 wifi support - NON FUNCTIONAL 20 wifi support - Initial Work Jul 18, 2024
@Kintar
Copy link
Contributor Author

Kintar commented Jul 18, 2024

I've also changed the title of this PR. Since we have a feature branch, tagging it as "NON FUNCTIONAL" seems a tad redundant.

mcknly
mcknly requested changes Jul 18, 2024
View reviewed changes
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.

Just a few nit-picky things to fix before merging into the feature branch, otherwise I will forget I was thinking of them.

main.c Outdated Show resolved Hide resolved
project.cmake Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
build_options.cmake Outdated Show resolved Hide resolved
hardware/rp2040/hw_wifi.h Outdated Show resolved Hide resolved
rtos/FreeRTOSConfig.h Show resolved Hide resolved
services/services.c Show resolved Hide resolved
services/services.h Outdated Show resolved Hide resolved
@mcknly mcknly merged commit 0e74be9 into mcknly:20-wifi-support Jul 19, 2024
@Kintar Kintar mentioned this pull request Jul 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcknly mcknly mcknly requested changes

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

Successfully merging this pull request may close these issues.

WiFi Support
2 participants
@Kintar @mcknly