code to allow user to change settings on the fly... using while loop? clunky!!

Projects Programming
1

Background:
Hi there. This is my first ever Arduino project, albeit an ambitious one. No coding experience at the outset, but doing my best to learn as I go. My code started simple... just take input from 4 BME280 temp/humidity sensors and display that info on an LCD and use that data to control a humidifier, dehumidifier, heater, and cooler via a relay.

Complicating the issue:
Now as time passes, I'm adding code to allow the user to change max and min temperature and humidity settings while the device is on and if settings are changed, write that to EEPROM in case of power loss. ( I know EEPROM has limited write per bit, so eventually was gonna add a counter for when the user changes settings so that it stores the variables in different bits) I'm also looking to include lighting schedules and clock settings (later on, once I add an external clock).

The Dream:
To accomplish the ability for the user to change settings, I wrote code to include a second LCD (lcd2) to display current max temp and min temp targets. The regular code will loop and bypass this "settings code" until the user selects settings with Button1 (out of 4). Once they select settings, I want the code to loop a different "settings code" altogether, and the LCD2 will display various setting changes.

Current Code:
I'm currently writing the settings code in a "while loop" starting on line 444 ( I know, a lot of code) and testing if different buttons are pressed and if they are, respond accordingly. So, button 1 starts "while loop" (user settings) it holds that button state in another variable so that this code loops until the user exits the settings with button 4. Button 2 confirms this setting as the one to change, button 3 moves to the next setting.

The Problem:
The code is starting to get out of control here... or at least seems to be. If I continue as is, the first while loop is going to grow tremendously until all the settings menus are written out. The code compiles, and may work (haven't tested in the real world yet) But it looks ugly, seems clunky, hard to understand, and I don't think this is the best way to write the code. Please, if you know how to write this in a better way, I'm open to any opinion or advice.

if you do code and this seems like waaay too much work to take on for free, maybe we can come to another arrangement? I'm getting to the point of bidding this job on freelancer or something.

new_code_experiment.ino (24 KB)

2 
  pinMode(button1, INPUT); //defining pin of button as input
  pinMode(button2, INPUT); //defining pin of button as input
  pinMode(button3, INPUT); //defining pin of button as input
  pinMode(button4, INPUT); //defining pin of button as input 
  
  pinMode(2, OUTPUT); //defining pin of heater as output
  pinMode(3, OUTPUT); //defining pin of cooler as output
  pinMode(4, OUTPUT); //defining pin of humidifier as output
  pinMode(5, OUTPUT); //defining pin of dehimidifier as output

Make it easy on yourself - give the pins meaningful names.

Edit: You did, but didn't bother using them :o

float hum1;  //Stores humidity value
float tempC1; //Stores temperature value
float tempF1; //Stores temperature value

When you start tacking digits on the end of variable names, arrays beckon.

3

I know, I included those at the outset, but have not written code for them yet. I'm working on the settings menu portion first, then ill add the button code

4

Creating an effective user interface can require more code than the rest of the program. The broad idea must be to ensure that no part of the code blocks any other part. That means not using delay() and not using FOR or WHILE loops that take more than few millisecs to complete.

The concept of a State Machine (look it up) is probably essential to keep track of what is going on - for example what stage the user input is at, or whether it is finished, or whether the user has gone off to have his lunch without finishing it.

...R

5

The user will not be changing settings regularly. Just during setup and maybe once a month. so the rest of the code does not need to run while the user is changing settings. it would be good to keep track if the user leaves without completing setting changes. though they are so simple, I'm sure they would complete them.

6

I agree that your code is sort of out of control. 750 lines of code in a single .ino file is madness. You should consider compartmentalizing the code into functional blocks and giving them their own .cpp/.h files.

I would also step away from the keyboard for a bit to "design" your code now that you (might) have basics in place. Use flowcharts and state diagrams to get a "ten thousand foot view" of the code, what the inputs and outputs are, what error-handling looks like, what the UI is and how to integrate them all.

Seamless operation of a system almost always requires that each section of code not hog processor cycles with things like delay(); use the millis timer to implement pauses or delays in execution when needed for specific sections without impacting the execution of other parts of the code.

What you've got looks like what happens when you start very simple and just keep adding things but don't have a specific design you're working toward.

7

Not what you asked for :wink:

In your other thread I mentioned functions. Start using them.

Long ago (in the time of dot-matrix printers) I was told that a function should fit on one page of A4. So I would first start cleaning up; your loop() function exceeds that one page.

For the cleanup and demonstration of functions, I have concentrated on the sensor; you're already familiar with structs so that makes it a little easier. I'm not a C++ programmer and don't have BME280 to test, I leave that up to you.

In the below code, I've combined all relevant data for the BME in a struct and next created an array of the structs. It's now a lot easier to loop through them which will make your code shorter and easier to read.

// struct with info for sensor
struct BMESENSOR
{
  Adafruit_BME280 bme;  // the bme
  bool status;          // status of bme
  float hum;
  float tempC;
  float tempF;
};

// array of sensor structs
BMESENSOR sensors[] =
{
  {bme1, 0, 0, 0, 0},
  {bme2, 0, 0, 0, 0},
  {bme3, 0, 0, 0, 0},
  {bme4, 0, 0, 0, 0},
};

To get the number of elements in any type of array, I added below near the top of the code

// macro to calculate number of elements in any array
#define NUMELEMENTS(x) (sizeof(x) / sizeof(x[0]))

Next I wrote two functions, one to check the status and one to read the sensor; both will modify the struct

/*
  check for presence of a sensor; sets status flag in struct
  In
    index in sensors array (zero based)
  Returns:
    false if index out of range, else true
*/
bool checkSensor(uint8_t sensorNumber)
{
  if (sensorNumber >= NUMELEMENTS(sensors))
  {
    Serial.println(F("Invalid sensor number"));
    return false;
  }

  sensors[sensorNumber].status = sensors[sensorNumber].bme.begin();
  return true;
}

/*
  read sensor data; fills struct
  In
    index in sensors array (zero based)
  Returns:
    false if index out of range, else true
*/
bool readSensor(uint8_t sensorNumber)
{
  if (sensorNumber >= NUMELEMENTS(sensors))
  {
    Serial.println(F("Invalid sensor number"));
    return false;
  }

  sensors[sensorNumber].hum = sensors[sensorNumber].bme.readHumidity();
  sensors[sensorNumber].tempC = sensors[sensorNumber].bme.readTemperature();
  sensors[sensorNumber].tempF1 = (sensors[sensorNumber].tempC1 * 1.8 ) + 32;
  return true;
}

And below demonstrates how they can be used

// macro to calculate number of elements in any array
#define NUMELEMENTS(x) (sizeof(x) / sizeof(x[0]))

#include <Adafruit_BME280.h>

#define BME_SCK 13
#define BME_MISO 12
#define BME_MOSI 11

#define BME1_CS 6
#define BME2_CS 7
#define BME3_CS 8
#define BME4_CS 9

Adafruit_BME280 bme1(BME1_CS, BME_MOSI, BME_MISO, BME_SCK); // software SPI
Adafruit_BME280 bme2(BME2_CS, BME_MOSI, BME_MISO, BME_SCK); // software SPI
Adafruit_BME280 bme3(BME3_CS, BME_MOSI, BME_MISO, BME_SCK); // software SPI
Adafruit_BME280 bme4(BME4_CS, BME_MOSI, BME_MISO, BME_SCK); // software SPI

// struct with info for sensor
struct BMESENSOR
{
  Adafruit_BME280 bme;  // the bme
  bool status;          // status of bme
  float hum;
  float tempC;
  float tempF;
};

// array of sensor structs
BMESENSOR sensors[] =
{
  {bme1, 0, 0, 0, 0},
  {bme2, 0, 0, 0, 0},
  {bme3, 0, 0, 0, 0},
  {bme4, 0, 0, 0, 0},
};

void setup()
{
  Serial.begin(57600);

  // loop through the sensors to check status
  for (uint8_t sensorCnt = 0; sensorCnt < NUMELEMENTS(sensors); sensorCnt++)
  {
    checkSensor();
    if (sensors[sensorCnt].status == false)
    {
      Serial.print(F("Sensor "));
      Serial.print(sensorCount + 1);
      Serial.println(F(" not found");
    }
  }
}

void loop()
{
  // loop through the sensors
  for (uint8_t sensorCnt = 0; sensorCnt < NUMELEMENTS(sensors); sensorCnt++)
  {
    // check status
    if (checkSensor() == false)
    {
      // hang forever if index out of range; not needed in this setup
      for (;;);
    }
    // if sensor there
    if (sensors[cnt].status == true)
    {
      // read and print
      if (readSensor(sensorCnt) == false)
      {
        // hang forever if index out of range; not needed in this setup
        for (;;);
      }
      Serial.print(F("Sensor ")); Serial.println(sensorCnt);
      Serial.print(F("\thum   = ")); Serial.println(sensors[cnt].hum);
      Serial.print(F("\ttempC = ")); Serial.println(sensors[cnt].tempC);
      Serial.print(F("\ttempF = ")); Serial.println(sensors[cnt].tempF);
    }
  }
}

Note how the first part of loop() compares to the first part of your loop(); 50% of lines saved (including some (in this case unneeded) hardening.

Try to break your code in blocks that you can put in functions; once a function is working properly, you can forget about it and just use it.

A last note:
You want blocking code while the setup is done; make sure thatb your temperature control is switched off as it might run out of hand while the user is entering data. I'm sure that a non-blocking approach is safer.