New here - working sketch improvements

Hi All

Im new to the forum but have been using Arduino / wemos / ESP's for a while.

Im no way a coder but do change / modify code (and normally get something working).

My question is, would it be OK to post my working code and ask what improvements could be made.

Thanks

Antdg

Yes. If possible, please post the sketch directly to the forum using code tags (</> button on the toolbar). If the sketch is longer than the 9000 characters allowed by the forum then it's OK to add it as an attachment.

If you're using the Arduino IDE, please do a Tools > Auto Format on your code before posting it to the forum so it will be easier for us to read.

Thanks for your swift response.

The below is running on a wemos d1 mini (just what I had available).

This code counts the number of times a button has been pressed and stores that number to EEPROM (I know this can only be written to a finite number of times).
After a power cycle the EEPROM is read and the counter continues to count.
A reset button has been added to reset the counter to zero if ever required.
The button count is then displayed on an OLED 64x48 (0.66)
Im sure this is poor code but as i say it does work :slight_smile:

#include <SPI.h>
#include <Adafruit_GFX.h>
#include "Adafruit_SSD1306.h"
#include <EEPROM.h>
#include <ESP8266WiFi.h>
#define OLED_RESET 0  // GPIO0
Adafruit_SSD1306 display(OLED_RESET);

// this constant won't change:
const int  buttonPin   = 16;    // D0 WEMOSthe pin that the pushbutton is attached to
const int  resetButtonPin   = 0;    // D3 Wemos the pin that the pushbutton is attached to
const int addr = 0; //memory address for EEPROM

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the main button
int lastButtonState = 0;// previous state of the main button

int buttonStateReset = 0;         // current state of the reset button
int lastButtonStateReset = 0;     // previous state of the reset button


void setup()
{
  WiFi.disconnect();          //Disable WIFI
  WiFi.mode(WIFI_OFF);        //Disable WIFI
  WiFi.forceSleepBegin();     //Disable WIFI
  delay(1);                   //Disable WIFI

  display.begin(SSD1306_SWITCHCAPVCC, 0x3C);  // initialize with the I2C addr 0x3C (for the 64x48)
  display.display();

  pinMode(buttonPin , INPUT);
  pinMode(resetButtonPin , INPUT_PULLUP);
  EEPROM.begin(512);

  checkCounter();       // read number from EEPROM
}

void loop()
{
  resetButton();        // has reset button been pressed if yes set buttonPushCounter to 0
  buttonPress();        // count button presses
  displayOLED();        // display buttonPushCounter on OLED
}


void checkCounter() {
  buttonPushCounter = EEPROM.read(addr); // read buttonPushCounter from EEPROM.
}


void resetButton() {
  // read the pushbutton input pin:
  buttonStateReset = digitalRead(resetButtonPin);

  // compare the buttonState to its previous state
  if (buttonStateReset != lastButtonStateReset) {
    if (buttonStateReset == LOW) {
      // if the current state is LOW then the reset button was pressed
      buttonPushCounter = 0;
      EEPROM.write(addr, buttonPushCounter);
      EEPROM.commit();
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }
  // save the current state as the last state, for next time through the loop
  lastButtonStateReset = buttonStateReset;
}


void buttonPress() {
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin);
  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button went from off to on:
      buttonPushCounter++;
      EEPROM.write(addr, buttonPushCounter);
      EEPROM.commit();
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }
  // save the current state as the last state, for next time through the loop
  lastButtonState = buttonState;
}


void displayOLED() {
  //clear display and set cursor on the top left corner
  display.clearDisplay();
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(0, 0);
  display.println("Count");
  display.setTextSize(2);
  display.setCursor(0, 25);
  display.print(buttonPushCounter);
  display.display();
}

Your sketch as is looks fine and there is really no reason to change it if this is all it's going to do.

However if you wish to expand it to do more or simply wish to learn there are a few things that could be addressed.

Datatype's: Memory is limited especially with the 8bit arduino's(uno,nano,etc.) so it is good to use the smallest datatype needed.
For instance

// this constant won't change:
const int  buttonPin   = 16;    // D0 WEMOSthe pin that the pushbutton is attached to
const int  resetButtonPin   = 0;    // D3 Wemos the pin that the pushbutton is attached to
const int addr = 0; //memory address for EEPROM

None of these variables will ever be larger than the smallest datatype(byte) which can hold numbers ranging from 0-255. So changing from int(16bits) to byte(8bits) will save 3 bytes(24bits) of dynamic memory space. Again no problem here but can really help on larger programs when you start running out of memory.

Delay(): Often it is needed to do more than one thing at a time and the Delay() function effectively stops everything for the duration and thus will interfere with other functions. learn the "blink without delay" example. there are also tutorials pinned at the top of this section of the forum.

EEPROM: As you know eeprom is limited in the number of writes. This is a little more advanced but there are ways to increase the life. One is wear leveling there are multiple techniques but the general idea is to spread the writes over all the addresses so you aren't wearing out one area. Another is to sense when the arduino is powering down and only save to eeprom then. This is more hardware based and there is one example in the "Introductory tutorials" section of the forum.

Again none of this is necessary for your sketch except maybe the eeprom issue. your code writes to a single address approx. 20 times a second(much more often if you eliminate the delay()). which will wear that address out pretty quickly.

display.println("Count");

For constant string, you could put it to program memory instead to save ram.

display.println(F("Count"));

Another thing is you don't need to update the display every time through loop() this could also cause delays that effect other functions. you can slow it down by only calling that function periodically. Another good place to use millis() timing code(blink without delay).

Hi All

Thanks all for your support, I will reread your replies over the next few days and try to expand my knowledge.
Great to know help / support is just a post away.

Once again thank you.

Antdg