Wireless Rickroll using Arduino

Question

This is a script I made for the ESP8266 that uses the Wi-Fi chip and creates a new Access Point every 5 seconds in order to display a message on WiFi lists on nearby devices that looks like this:

The code is:

#include <ESP8266WiFi.h> const char* ssids[] = {"Never gonna give you up","Never gonna let you down","Never gonna run around","Never gonna make you cry","Never gonna say goodbye","Never gonna tell a lie"}; const char* pass = "pass_goes_here"; void setup() { // put your setup code here, to run once: Serial.begin(9600); int currentssidno = 0; while (true) { const char* ssid = ssids[currentssidno]; Serial.print("SSID: "); Serial.println(ssid); WiFi.softAP(ssid, pass); delay(5000); WiFi.softAPdisconnect(false); currentssidno = currentssidno + 1; if (currentssidno == 6) //please change this count if you change the amount of ssids { currentssidno = 0; } } } void loop() { } 

The code is also on GitHub.

How can I improve this code? This is my second or third project with Arduino and I pretty much made it in 15 minutes and looking at it now, some parts feels like dirty code (especially the part where I switch reset the SSID counter), how can I improve this?

Extra resources: Wi-Fi Library, ESP8266 Arduino Core

Answer 1

No particular comment on the Arduino-specific or Wifi-specific stuff, but here:

currentssidno = currentssidno + 1; if (currentssidno == 6) //please change this count if you change the amount of ssids { currentssidno = 0; } 

Two things. First, you can rephrase this conditional logic as simply

currentssidno += 1; currentssidno %= 6; // please change this, etc. 

or equivalently

currentssidno = (currentssidno + 1) % 6; // please change this, etc. 

depending on where you'd prefer to repeat the name currentssidno. (I'd probably rename that variable to i and then use the latter form.)

Secondly, don't hard-code magic numbers like 6, if you can express them as "non-magic" numbers such as "the size of this particular array I've got".

// Put this macro in your toolkit. You'll use it frequently. #define DIM(arr) (sizeof (arr) / sizeof (arr)[0]) /* ... */ currentssidno = (currentssidno + 1) % DIM(ssids); 

Now you no longer need the comment, because the code no longer has that weird subtle dependency on the length of the ssids array. It Just Works, no matter what ssids you want to use.

Answer 2

One small suggestion:

setup(), is, as described, only meant to run once, at startup.

Once setup() is finished, loop() will be called in an infinite loop.

So you could remove your while(true) loop from setup, and move the code to loop:

#include <ESP8266WiFi.h> #define DIM(arr) (sizeof (arr) / sizeof (arr)[0]) const char *ssids[] = { "Never gonna give you up", "Never gonna let you down", "Never gonna run around", "Never gonna make you cry", "Never gonna say goodbye", "Never gonna tell a lie" }; const char *pass = "pass_goes_here"; void setup() { Serial.begin(115200); } void loop() { static int currentssidno = 0; const char *ssid = ssids[currentssidno]; Serial.print("SSID: "); Serial.println(ssid); WiFi.softAP(ssid, pass); delay(5000); WiFi.softAPdisconnect(false); currentssidno = (currentssidno + 1) % DIM(ssids); }