Update WiFiMulti.cpp to support FreeRTOS #1878
Conversation
Added clearAPList method to allow loading of new APs for WiFiMulti.
Added clearAPList method to allow loading of new APs for WiFiMulti.
Changed whitespace before clearAPList method.
When used with FreeRTOS, the delay statement caused the code to stop responding as FreeRTOS was stuck. Replaced delay with another loop to do nothing for 5ms.
Attempted to fix indentations to pass astyle checks
|
@earlephilhower, I've made a few attempts to get this to pass the astyle check but still fails there. If you could let me know what may be wrong here, I will try to correct formatting in the future. In the meantime, I would appreciate it if you could pull this into the code base. I also had a question regarding having Arduino IDE pick up these changes. I assume a new version needs to be created, which is done on a schedule or major update of the library. |
|
The action details always show you why something failed. In this case, you are missing a space before the bracket of an
You can also execute the checks locally like the CI does arduino-pico/.github/workflows/pull-request.yml Lines 49 to 52 in 0e4fd05 |
|
@maxgerhardt, Thanks, I looked at the output, but did not see the issue on line 562. Thank you, I assumed it was the preceding spaces and did not look at the spaces in the statements. I appreciate your reply and will try running the checks locally as you mentioned. |
There was a problem hiding this comment.
Please give an MCVE showing the failure you're correcting here. WiFi is under use by many folks w/FreeRTOS already so I'm not sure exactly what you're seeing or trying to accomplish here.
Please also explain your reasoning for swapping out a task-switching delay for a busy wait. I'm not grokking any difference except this ties up a complete core while waiting for WiFi vs. letting other tasks run...
libraries/WiFi/src/WiFiMulti.cpp
Outdated
| // failure when using RTOS | ||
| unsigned long tWifiDelayForRtos_start = millis(); | ||
| while (millis()-tWifiDelayForRtos_start <= 5) { | ||
| if(millis()<tWifiDelayForRtos_start) { |
There was a problem hiding this comment.
FYI, with unsigned math there is no need for this check at all. Wraparound deltas just work.
| delay(5); | ||
| // Replaced delay(5); with the following to prevent | ||
| // failure when using RTOS | ||
| unsigned long tWifiDelayForRtos_start = millis(); |
There was a problem hiding this comment.
delayMicroseconds(5000); // Avoid task switch in FreeRTOS does the same thing, more simply.
|
I think this PR is a direct result of #1873. Let me just test their example code that they say hangs.. |
|
Yeah, I saw the full app. I was hoping for a nice, small MCVE w/just WiFiMulti and FreeRTOS. The app looks like is uses a SD card for settings and other extraneous stuff which may be the cause of their issue, not WiFiMulti. The code being changed is essentially the beginning of every WiFi example we have: (start wifi, while !connected delay()) and a common configuration. I would bet anyone using FreeRTOS and WiFi would be doing the same kind of thing... The change as-is seems to be a pure no-op logically. If it's really needed, then there's another, deeper problem in the WiFi stack because the WiFi chip should be connecting async from whatever the main core is doing anyway. |
|
I've just tested this with the // Overwrite config object
clockSdCard.sdCardConfig = {
.hostname = "My Pico W",
.timezone = "GMT",
.networks = {
{
.ssid = "Vodafone-1E90",
.pass = "XXXXXXXXXXXXXXX"
}
}
};
configLoadMillis = millis();And ran the sketch with |
|
I can't reproduce the failure here using a simplest MCVE with only WiFiMulti. Would you be able to try adding Random guessing here, but what may be happening is noise on the power supply for some PicoWs affecting WiFi performance. When you The debug logs w/RSSI might help see if it's a weak signal problem. |
|
In a minimal sketch using WiFiMulti and FreeRTOS I can also not determine a difference between #include <Arduino.h>
#include <FreeRTOS.h>
#include <task.h>
#include <semphr.h>
#include <WiFi.h>
#include <WiFiMulti.h>
#define WIFI_SSID "Vodafone-1E90"
#define WIFI_PASS "XXXXXXXXXXXXXX"
WiFiMulti multi;
int _beginLoopCount;
bool skip = false;
void setup() {
while(!Serial);
multi.addAP(WIFI_SSID, WIFI_PASS);
delay(1000);
Serial.println("Connecting to " + String(WIFI_SSID));
Serial.flush();
}
void loop() {
if(skip) return;
Serial.println("Running on core " + String(get_core_num()) + " task ID " + String(uxTaskGetTaskNumber(xTaskGetCurrentTaskHandle())));
unsigned long startConnect = millis();
if (multi.run() != WL_CONNECTED) {
if (_beginLoopCount < 5) {
Serial.println("\nFailed to connect to Wi-Fi. Trying again...");
_beginLoopCount++;
delay(1000);
} else {
Serial.println("\nFailed to connect to Wi-Fi. Quiting");
_beginLoopCount = 0;
}
} else {
_beginLoopCount = 0;
Serial.print("\nWiFi connected, SSID: ");
Serial.print(WiFi.SSID());
Serial.print(", IP address: ");
Serial.println(WiFi.localIP());
Serial.printf("Connected in %d ms\n", millis() - startConnect);
skip = true;
}
} |
|
I also tried the full app example (commenting out and hacking in the config like you did @maxgerhardt and making all debug go to USB). I did not use the edited WiFiMulti.cpp/.h in the I've power-cycled the PicoW about ten times so far and it always connects on the 1st try for me. That said, I am ~3 meters from the AP it's connecting to w/a RSSI of about -32. So if this is noise related there's plenty of headroom in the signal it's seeing which might not be the case at the @richteel's location... |
|
I will give those options a try and see if I still have an issue. It could be a power supply issue although the hub that it is connected to has its own power supply and nothing else is connected to that hub. It should not be an issue with signal as my AP is about 4 foot and my second testing one is about 2 feet away. If anything, I would think it is a power issue but really doubt that. I will try it on a Pico W that is not in circuit too just to be certain.
The reason I suspected it was the delay as the I read somewhere in the FreeRTOS documentation that delay should not be used but vTaskDelay should be used instead if a delay is required. When I was stepping through the code when it was stuck, I noticed that it was bouncing around in the FreeRTOS code. Don’t recall exactly but remember seeing it look at get minimum delay or something similar.
|
|
Oh, I should have stated that when I was having the issue, it would take about 30 seconds before it seemed to lock up. Once I resolved the issue, it took 8 seconds to connect. Another reason I’m sure it is no the signal or noise. If it is anything it would be power but seriously doubt that.
|
|
Re: arduino-pico/libraries/FreeRTOS/src/variantHooks.cpp Lines 155 to 157 in 280fc43 |
|
Also, by "power" I meant the onboard power supply. There's an LDO on the PCB which can operate in different modes to bring the 5V USB down to the 3.3V (or lower) required by the chip. I was suggesting that the transients could come from that and not the USB cable or hub itself (although on the ESP8266 there are many, many instances where a poor quality USB cable caused WiFi problems there with Vdroop because it was so power hungry while transmitting...) Checking the seen RSSI would be a useful thing just to see if it's at the margin or if it's loud and clear to the Pico. |
|
Here are the results of running the ScanNetworks example code to get the RSSI for each network. Running the minimal example code that @maxgerhardt, posted above, I get the same behavior as I saw in my example code. The Raspberry Pico W seems to lock up as it fails to execute the remaining code. Also, when I attempt to upload a new sketch, I need to unplug it and press the BOOTSEL button while I plug it back in. I tried this on three Pico W devices, with one of them being brand new. Here is the output of the sample code above. It has been sitting for about 10 minutes now without advancing any further. I tried the code without any references to FreeRTOS and it connects right away. Here is the results of that code. It connects successfully with debugging but takes a long time to connect as it fails with the first attempt. After unplugging the Pico W and plugging it back into the PC, it seemed to lock up again but it did try to make a second attempt. I saw the same behavior on all three Pico W boards that I tried. |
|
Lockups and slow connections are very different symptoms with very different causes. I suggest debugging them separately. For lock ups, please make sure you are using the latest and greatest When you get a lock up, please grab the stack trace for both cores. If that's all scrambled, or PicoProbe can't connect to the SWD debugger, then it's a physical/voltage problem and not SW... |
Fixed formatting issues for astyle check
|
Okay, the plot thickens. Delay is not the issue but having only delay in the while statement causes the issue. I had removed the if block from the while loop and I saw the same behavior as the delay. I then added the if block back in and commented out the while statement and closing bracket and added the delay statement back in and it would connect on the first or second attempt. Any idea why there needs to be something in addition to delay in the loop? Calling it a night so will pick this back up tomorrow afternoon. BTW: Thanks for checking on this and pushing back a bit to find out what is really going on. It looks like it needs more work to get to the bottom of the issue. I just got lucky yesterday by putting that if statement in the loop. |
|
Take note I tested this with PlatformIO and the commit d2461a1, not 3.6.1 stable. That is exactly the commit that has the IRQ fix. If any delay somewhere causes a task switch which causes the incorrect re-enabling of IRQs, that could be the problem. Can you manually apply the above commit and retry? |
Yes, you are correct. I was doubtful because I thought I would have had that change from the start as I had copied the latest code into my library, but my guess is I had copied the code before this commit. I had pulled the latest code again at some point yesterday when I was doing testing because of Earl's comment about making certain I was using the latest code on GitHub. I had thought that I tried only delay in the loop and was seeing the same issue. I pulled the code again this morning and reverted to having only delay in the loop and it is indeed working. Thank you both for all the help. I'm closing this pull request as it is not needed and did not fix the root problem. I have a suspicion that what may have happened is I've been seeing some errors as I edit or overwrite the files in the library. "Failed uploading: cannot execute upload tool: exec: "C:\Users\USER\AppData\Local\Arduino15\packages\rp2040\hardware\rp2040\3.6.1/system/python3/python3": file does not exist". If I uninstall the Pico W board from the Arduino IDE and add it back, it resolves the issue. Not sure why that is happening but is only occasionally, and typically after a reboot if I edit or overwrite the RP2040 files in the library. I think that may have happened and I did not overwrite all the files, only the WiFiMulti. h and cpp files. |
When used with FreeRTOS, the delay statement caused the code to stop responding as FreeRTOS was stuck. Replaced delay with another loop to do nothing for 5ms.
Attempting to fix astyle failed test.