patternMinor
Wireless Rickroll using Arduino
Viewed 0 times
usingarduinowirelessrickroll
Problem
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:
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
The code is:
#include
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
Solution
No particular comment on the Arduino-specific or Wifi-specific stuff, but here:
Two things. First, you can rephrase this conditional logic as simply
or equivalently
depending on where you'd prefer to repeat the name
Secondly, don't hard-code magic numbers like
Now you no longer need the comment, because the code no longer has that weird subtle dependency on the length of the
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.Code Snippets
currentssidno = currentssidno + 1;
if (currentssidno == 6) //please change this count if you change the amount of ssids
{
currentssidno = 0;
}currentssidno += 1;
currentssidno %= 6; // please change this, etc.currentssidno = (currentssidno + 1) % 6; // please change this, etc.// Put this macro in your toolkit. You'll use it frequently.
#define DIM(arr) (sizeof (arr) / sizeof (arr)[0])
/* ... */
currentssidno = (currentssidno + 1) % DIM(ssids);Context
StackExchange Code Review Q#131588, answer score: 8
Revisions (0)
No revisions yet.