For loop not stopping causing error

I have no formal training in C, only in "high level" programing languages like FORTRAN.
Yes, I am 68...
I have added the serial print commands to help me to debug the code.
A copy of the printing is attached below.
As you can see, the loop keeps incrementing the conrol variable (t) indefinitely, causing an error.
This is probablay a VERY STUPID question, but I have googled and spent hours trying to figure out the error, without succes.
Any help would be welcome.
Thanks
///////////////////////////////////////////////////////////////////////
 // ack_Alarm  (active alarm => ack_Alarm = LOW)
 //////////////////////////////////////////////////////////////////////

 void ack_Alarm(){
                   Serial.println("------------------Ack Alarm -------------");
  if ( digitalRead(ack_Alarm_Button_Pin) == LOW) 
          {
            Serial.println(" ---ÃCK BUTTON PRESSED---");  
            for (int t = 1; t <= 7; t++)
             {
            if ( fault_ID[t] == HIGH)
              { ack_ID[t] = HIGH;   
                 tft.fillScreen(ILI9341_BLACK);  
                 tft.setCursor(0, 0);
                 tft.setTextColor(ILI9341_GREEN);  tft.setTextSize(3);
                 tft.println("ALARM ACKNOWLEDGED");
                 delay(2000); //      <======================== CHECK IF REQUIRED WITH DISPLAY ALARMS
               
                Serial.println( "t= " + String(t) + "time=  " + String(millis()) + "  ackAlarmStatus= " + String(ackAlarmStatus) );
             }
         }
    }

    else { Serial.println(" --- No Button pressed");}
 }

This is the Serial Print output****

------------------Ack Alarm -------------
 ---ÃCK BUTTON PRESSED---
t= 3time=  112077  ackAlarmStatus= 0
t= 10time=  114112  ackAlarmStatus= 0
t= 16time=  116145  ackAlarmStatus= 0
t= 17time=  118179  ackAlarmStatus= 0
t= 23time=  120213  ackAlarmStatus= 0
t= 24time=  122247  ackAlarmStatus= 0
t= 30time=  124281  ackAlarmStatus= 0
t= 31time=  126315  ackAlarmStatus= 0
t= 36time=  128349  ackAlarmStatus= 0
t= 37time=  130383  ackAlarmStatus= 0
t= 38time=  132417  ackAlarmStatus= 1
t= 43time=  134451  ackAlarmStatus= 1
t= 44time=  136485  ackAlarmStatus= 1
t= 45time=  138519  ackAlarmStatus= 1
t= 47time=  140553  ackAlarmStatus= 1
t= 49time=  142587  ackAlarmStatus= 1
t= 50time=  144621  ackAlarmStatus= 1
t= 51time=  146655  ackAlarmStatus= 1

Do you by any chance have a global variable named t ?

fault_ID[7] = {"whatami"};
ack_ID[7]   = {"urempty";

it's interesting the the code starts at 1, not 0 and ends when <= 7, not < 7.

it would help to see the rest of the code, where ack_ID[] and fault_ID [] are defined.

bear in mind that ack_ID [7] is the 8th element of the array, not the 7th. is it defined as possibly byte ack_ID [8]?

Please do not flaunt your youth in front of us old guys.

:older_man:

1 Like

Click on the <\> symbol in the message composition windoew tool bar, and paste your entire sketch where it will tell you to do.

With only what you posted, it is hard impossible to know what is goink wrong.

And yeah, 68 will impress us when it's in hexadecimal. :wink:

TIA

a7

Thank you all for the replies.
I am not posting the entire code here because it is very long, probably more than 250 lines and somewhat complex without a detailed description.

the answers:
1 - there is no global variable t
2 - The variables fault_ID and ack_ID are arrays containing the boolean state (H or L) of each of [i] position
3 - I skip the index 0, since I am not used to it ( in old times, indexes started at 1...)
But you have already given me one clue.
The variables are defined as [7], not [8]
I will correct this mistake and, if the problem persists, I will post the initial part of it where the variables are defined and the setup/main loop, which is short.
I should have done that in the first place. Sorry for that.
Thank you very much for the help,

Since I will be busy today, I am posting the initial part of the code to help you in assisting me.

float Irms, flow[3], Pdisch, DpFilter, Pinlet, lampCurrent,auxVol, totalLiters, rawDp_float[3], flapstate_float;
bool flapstate;
char *myVarName[] = {"IRMS", "Flow[1]", "Pdisch", "DpFilter", "Pinlet", "lampCurr", "rawDp[1]", "flap_flo", "auxVol", "flow[2]", "totalLit", "rawDp[2]" };

#include <TFT_eSPI.h> // Hardware-specific library

//   In the TFT_ESPI user_setup.h file
//   be sure to comment OUT the line for TFT_CS         //   <======== BE SURE TO DO THIS EVERYTIME A NEW COMPUTER IS USED ==??? Not found in Dec 2022
//   //#define TFT_CS   21  // Chip select control pin  //             OR the library is Re-installed
//------EDIT THIS LINE IN USER_SETUP_SELECT.h--------
// #include <User_Setups/Setup42_ILI9341_ESP32.h>           // Setup file for ESP32 and SPI ILI9341 240x320 //<===============This is my setup===================
//


//----- Using faster Graphic Library
TFT_eSPI tft = TFT_eSPI();       // Invoke TFT custom library

// *************My setup for Harware SPI for ILI9341***************
// The pins used by the TFT display are defined in the user_setup.h file
// ####### DO NOT ERASE THIS #######
//      TFT_MISO 19
//      TFT_MOSI 23
//      TFT_SCLK 18
//      TFT_CS    15  
//      TFT_DC    2  // Data Command control pin
//      TFT_RST   4  // Reset pin (could connect to RST pin)

// Define variables used to input Data
int myIndex = 0;
float inputValue = 0.0;

//--------------------------------------------------------------------------
// Example 5 - Receive with start- and end-markers combined with parsing
//-----------------------------------------------------------------------------
const byte numChars = 32;
char receivedChars[numChars];
char tempChars[numChars];        // temporary array for use when parsing

      // variables to hold the parsed data
char messageFromPC[numChars] = {0};
int integerFromPC = 0;
float floatFromPC = 0.0;
boolean newData1 = false;

//------Define Pin numbers for buttons & LEDs
const int pump_On_Pin = 26;
const int alarm_On_Pin = 12;
const int alarm_Toggle_Button_Pin = 13;
const int ack_Alarm_Button_Pin = 33;
const int buzzerPin = 25;
const int fault_Led_Pin = 27;
const int warning_Led_Pin = 14;  

// Define Variables used to test alarms/ack functions
bool faultStatus = LOW;
bool warningStatus = LOW;
bool ackAlarmStatus = LOW;
bool delayRunning = LOW;
bool pump_State = LOW;
bool last_Pump_State = LOW;
bool flowDelay = LOW;
bool clearDataFlag = true;
bool cisternaRxLoss = false;
bool caixaRxLoss = false;
bool buzzer_ON_OFF_Toggle = LOW;    //is buzzer ON or OFF
bool buzzer_Status = LOW;
bool levelWarningStatus = LOW;

int rawDp[3] = {0, 0 , 0};
int rawDpMin = 32;
int rawDpMax[3] = {0, 243 , 187}; // values changed in rev 2_A
int levelDp[3] = {0, 0 , 0};
int levelDpLow = 30; // critical level in %
int levelDpMin = 20; // minimal level in %
int levelDpWarning = 50; // warning leval in %

float flowMin[3] = { 0.0 , 1.3 , 10.0 };

//----Toggle button & Misc stuff----
int buzzerState=0;
int alarm_Button_New;
int alarm_Button_Old=1;
//???int ack_Button_New;
//???int ack_Button_Old=1;
int dt=500;
bool ack_ID[7] = {0,0,0,0,0,0,0};
bool fault_ID[7] = {0,0,0,0,0,0,0};
bool fault_ID_Old[7] = {0,0,0,0,0,0,0};
//            unsigned long ack_ID_mil[7] = {0,0,0,0,0,0,0}; // Jan2023
unsigned long time_between_failures = 60000; //3600000; <===== ATTENTION VALUE CHANGED TO CHECK LOGIC!!!!!!

unsigned long fault_ID_mil[7];
unsigned long fault_ID_mil_Old[7] = { 0, 0, 0, 0, 0, 0, 0 };

int i= 1;

int x = 1;
unsigned long t1;
unsigned long t2;

//============

void setup() { //----------------------------------------------------- SETUP BEGINGS HERE -----------------------------------------------------------------------
    Serial.begin(9600);

// ----define Led pins as outputs
  pinMode(alarm_On_Pin, OUTPUT);
  pinMode(pump_On_Pin, OUTPUT);
  pinMode(buzzerPin, OUTPUT);
  pinMode(alarm_Toggle_Button_Pin, INPUT_PULLUP);
  pinMode(ack_Alarm_Button_Pin, INPUT_PULLUP);
  pinMode(warning_Led_Pin, OUTPUT);
  pinMode(fault_Led_Pin, OUTPUT);
  
    //-----Start TFT, set rotation and background color------
  tft.init();
  tft.setRotation(0);  
  tft.setTextSize(2);

//------- Initialize display
  tft.setCursor(0, 0);
  tft.setTextColor(ILI9341_YELLOW);  
  tft.println("ACK ALARM TEST");
  tft.println("Rev0 31/12/2022");
  delay(2000);
  
    Serial.println("This sketch expects 2 pieces of data: an integer (the index) and a floating point value (the value of the variable)");
    Serial.println("Enter data in this style < 1, 24.7>  ");
    Serial.println();

// test: Print variable names in serial port
for (int k = 0; k<12; k++) {       // <====ATTENTION!!! First index is ZERO
    Serial.println(myVarName[k]);
  }
// --------- Define a "NORMAL" Value for the variable being analyzed-----------  <============= REMOVE AFTER TESTING!!!!!!!

 flow[1] = 5.0;
 
 rawDp[1] = 218; // ======> level aprox 90%
// -------Calculate output variables from raw data ---------
   levelDp[1]  = map(rawDp[1], rawDpMin, rawDpMax[1], 0, 100);
}

//----------------------------------------------------------------END OF SETUP--------------------------------

void loop() { //--------------------------------------------------------LOOP BEGINS HERE --------------------------------------------------------------------------------
   
// Read Serial data
    recvWithStartEndMarkers();
    if (newData1 == true) {
        strcpy(tempChars, receivedChars);
            // this temporary copy is necessary to protect the original data
            //   because strtok() used in parseData() replaces the commas with \0
        parseData();
        showParsedData();
        assignValue();
        newData1 = false;
    }

// Check if there are faults
   fake_display_faults();

// Define if buzzer is ON or Off
   buzzer_On_Off_Toggle();
   delay(500);
   
// Check if the buzzer should be turned on
   check_Alarm_Status();

// If alarm pin is HIGH, beep buzzer
   beep_Buzzer(); 

// Check if Alarm was ackoledged
   ack_Alarm();

// Reset Alarm times
//          reset_Alarms();
}

//----------------------------------------------------------LOOP ENDS HERE-----------------------------------------------------

Then they have 7 levels numbered 0 to 6 and your code writes outside of the memory allocated to them and very likely changes the value of another variable when it does so

you've already post 210 lines between the 2 posts. wouldn't be too much to post the complete code or the remaining 40 lines

also, seems like there have been several threads monitoring water levels and/or controlling pumps, so this may not be that unusual a program

somewhat of a red-flag is the large # (18+) of boolean flag variables and arrays. seems odd that the code appears to be monitoring 7 different alarms, but the non-array boolean variables suggest tracking the state of just one (maybe a state machine)?

it's also a somewhat common problem to process serial input when there isn't any. i see you avoid it by having a "newData1" variable, but since i see no INPUTs other than buttons, it appears that this code simply monitors sensor values communicated to it. the remaining function calls in loop() must be to report status

it appears that this code reports alarms, or at least alarm acknowledgements on the TFT screen. what happens if there are multiple alarms? i see a 2 second delay, but couldn't this result in a 14 second cycle that blocks receiving serial messages

wouldn't it be better to report alarms in "timed" background sub-function without blocking code execution using delays?

i worked on fortran code in '85. later, i had a supervisor who said some people were writing "assembler code in C". wonder if you're "writing a fortran code in C", rather than using C/C++ and Arduino supporting functions more efficiently?

[Thank you for your reply.
I will correct the mistakes (more than one) related to array size later, and post the result.
Some comments.
I think I have guessed the program size wrong. What I have posted is about 30% of the total.
The code is in development, so there are a lot of variables that have been declared but are not n use.
When the code works, I will “clean it up”.

The purpose of the program is to receive via wifi the value of 7 variables related to a water supply system. There are two remote stations.

The central station receives and display data.
All variables have an operational range and an alarm will sound if value is outside range.
There is a button to acknowledge the alarm, silencing the buzzer.
Also, the time when the variable gets out of range is registered, so the alarm will not sound again for the same condition for 30min.
I am in the “testing fase” of the program, so I added a serial read input function, so I can input the data that would come via WiFi and check the program logic.
Also, the “time between failures” has been reduced to 1min so I can check the logic faster.

what you're doing make sense.

i'd separate the function for receiving and processing messages and monitor both (not replace one with the other) Wifi and Serial for messages that can be processed from either source to aid testing

does this mean different parameters for each input?

a code review might help find other problems if you're new to C/C++

it also makes sense to maintain a serial interface for debugging/diagnostics, exercising sub-functions and altering behavior (e.g. your 30min timeout) during testing.

i've been working on a model RR "node" which monitors block occupancy and drives signals. i've got WiFi code to share information between nodes (3-4).

i'd like to see your code for monitoring WiFi input from multiple stations.

Thank you for your reply.
Would you please elaborate a little on the “ i'd separate the function for receiving and processing messages and monitor both (not replace one with the other)”?
All the monitored variables have warning and “fault” values.
There are cases were I combine two events to characterize an equipment failure (ex. Pump is ON but there is no flow (or too low flow) on the discharge pipe.
I am a chemical engineer and I have worked 37 in design and troubleshooting refinery installations, so I am trying to “replicate” what is used in the industry.
This code I am working at the moment monitors only one variable, for simplicity.
Once it has been tested and approved, I will copy the relevant sections to the unit in the actual installation (which has a lot less features).
Finally, I have struggled a lot to have the wifi working, and yet the receiver looses connection with either transmitter once in a while.
I am using the nRF24L01+ modules with external antennas.
I can post the wifi related code here, if you want, but I am sure it is very basic (and may contain bugs)….

consider the code below which i quickly hacked together, compiles and runs and processes ASCII message strings from the serial interface and which i believe would process same from a WiFi client if proper credentials were provided and a Wifi source

a message is terminated with a "," (allowing multiple messages to be concatenated together but easily separated and processed independently

of course all the processing routine does is print the message. your code would check parameters for each source or combination of source and set alarms.

some background sub-function invoked in loop() would report alarms, sounds buzzers, monitor kill buttons

don't think i'm telling anything you don't already know, but i thing i am suggesting a different organization


# ifdef ESP32
#  include <WiFi.h>
# elif defined(ESP8266)
#  include <ESP8266WiFi.h>
# endif

WiFiServer server (4445);
WiFiClient clients [5];
unsigned   nClient = 0;

// -----------------------------------------------------------------------------
void
processMsg (
    char *msg )
{
    Serial.print ("processMsg: ");
    Serial.println (msg);
}

// -----------------------------------------------------------------------------
void readWifi (void)
{
    static char buf [80];
    static int  bufIdx = 0;

    // scan for new connections
    clients [nClient] = server.available ();

    if (clients [nClient])  {
        IPAddress ip = clients [nClient].remoteIP ();
        printf ("%s: new connection %d:%d:%d:%d\n",
            __func__, ip [0], ip[1], ip [2], ip[3]);

        nClient++;
    }

    // scan connections
    for (int n = 0; n < nClient; n++)  {
        if (clients [n].connected ())  {
            while (clients [n].available ())  {
                char c = clients [n].read ();
                buf [bufIdx++] = c;
                if (',' == c || (sizeof(buf)-1) <= bufIdx)  {
                    processMsg (buf);
                    bufIdx = 0;
                }
            }
        }
        else {
            IPAddress ip = clients [n].remoteIP ();
            printf (" %s: dropped [%d]  %d:%d:%d:%d\n",
                __func__, n, ip [0], ip[1], ip [2], ip[3]);
        }
    }
}

// -----------------------------------------------------------------------------
void
readSerial (void)
{
    static char buf [80];

    if (Serial.available ())  {
        int n = Serial.readBytesUntil (',', buf, sizeof(buf)-1);
        buf [n] = '\0';     // terminate char string with NUL

        processMsg (buf);
    }
}

// -----------------------------------------------------------------------------
void loop ()
{
    readSerial ();
    readWifi ();
}

// -----------------------------------------------------------------------------
void setup ()
{
    Serial.begin (9600);

    WiFi.mode (WIFI_STA);

    WiFi.begin ("mySSID", "myPassword");
    server.begin ();
}

i just realized that my code needs separate buffers for each client (thanks)

sounds like you need a table of parameters for either each alarm or each source.

for my model RR signal code, each node monitors some blocks for occupancy and updates a table. a different piece of code reads the table to drive signals. it displays a STOP signal if one block is occupied and an yellow signal if the next block is occupied and CLEAR when neither block is occupied.

the reason for the code updating a table between the block monitoring and signal logic is the block may be on a different node. the wifi code updates the block table as well

i'm went to NJIT (newark NJ) and spent most of my time there in the chemE department

Hi,
thank you very much for your work!!!
It will take me a reasonable time to understand it, since, as I said, I have never learned C/C++.

The idea of keeping both input "modes" available in the "operational unit" to help debugging future changes seems very good.
The way I am handling the status of the variables in a function that tests if there is a "FAULT".
Each fault may involve one or more variables.
There are also less important deviations that generate "Warnings".
Up to now, there are 7 critical deviations "FAULTS" and 3 "WARNINGS".
If one Fault is detected a flag is set HIGH (fault_ID[i].
Later the program displays a message on the local screen(the receiver unit) and sounds a buzzer to alert me that there is something wrong.
This approach has worked fine for me.
Is there any reason I should change it?

All the code that I have developed for the 3 modules was implemented in steps, using a basic "problem solving algorithm"
1 - divide the sketch into "problems" (or tasks to be solved)
2 - search the internet for a similar problem/tasks
3 - evaluate the solutions and select a candidate.
4 - Try to understand the selected code (more googling)
5- Adapt the code to my requirements
6 - test (usually more googling to understand what is wrong...)
7 - move to the next "problem/task"

I started this some 3 years ago when I bought a country house where all the water came from a natural spring, so I have one local reservoir to store a reasonable amount of water, and another reservoir close to the house (3m above the ceiling) to supply the house.
There is also a pump (near the lower reservoir) , a simple electric level control for the upper reservoir .
Since the reservoirs are far from the house it became obvious (after the first weeked "water crisis", that I needed a system to monitor the variables of the "system".
Now, after these years I can say I have a pretty basic understanding of C, but I have found, by experience, that every new snippet has a built-in challenge for me.
Since I am retired and have worked with design all my life, most of the time it is fun...

edits in bold

these sound more like requirements than a design. of course you shouldn't change your requirements, but it's not clear what the design is

based on your descriptions i think there are 3 parts:

  • there are several sources of external inputs that need to be monitored
  • there are some combination of inputs that may result in alarms and levels of alarm
  • multiple alarms need to be displayed, presumably one at a time and there is a button to clear current alarm indications (i.e. buzzer)

i don't know that there is a cookbook approach for "problem solving algorithm". your approach sounds similar to Jon Bentley who suggested that super-programmers find and modify existing code rather than starting from scratch. but i've seen professionals implement code with much obfuscation, as well as cleanly and obvious.

i agree that dividing a problem into pieces is key. but still not so obvious. both The Elements of Programming Style and C++ Programming Style evaluate bugs in textbook examples of code and poorly designed object oriented code.

as i mentioned, the numerous boolean variables suggest unnecessary complications

what i am finding to be true for applications such as yours is to make the problem data driven such that even someone without programming experience can modify parameters and add new cases in tables rather than logic. and i think this benefits the person who wrote the program as well, focus on the problem domain, not the implementation.

one irony of a data driven approach is that the logic can be messy as long as the data descriptions are cleanly understood

in your case it may make sense for input message processing to simply update input values in some table with a record for each possible input and possibly have a 2nd table describing relationships between inputs and thresholds for each possible alarm. A separate sub-function processes both tables to determine alarms levels.

of course this may be wrong without a clean understanding of goals and without seeing your complete code

my opinion is that it's difficult to teach how to do this; it is better to demonstrate with cleanly written correct, complete and unambiguous code

it's not clear what the above is saying. in the 1st code snippet you posted, there is a loop from 1-7. Is this evaluating 1 of 7 alarm levels for a single input(?) or is it evaluating one of 7 possible inputs?

my experience is that at work, we don't usually have time to go back and redo something that is tested and meets requirements because other work is pending. but there's usually time for improvements, both that enhance functionality as well and making things more maintainable in home projects

so presumably my comments are in line with your interests, developing a clean maintainable program

i'd like to see your complete code. my model RR code is currently 1500 lines in 13 files. a steam throttle i've been working with people as far away as australia is 4300 line and 36 files. the numerous files help separate the problem into pieces. a smaller model RR project was over 1200 lines, with one ~300 line file that data description

send me a PM if interested

image.png

The code may not reflect the exact number I have stated, since, as time goes by, some "events" (a combination of process deviations) that I thought would be meaningful were proven NOT to be.
Also, some sensors have proven to send wrong signals from time to time, and I had to "deactivate" that FAULT (an alarm condition) to prevent going into a false alarm condition.
I plan to do a major maintenance on the hardware soon, since it seems that the design had not required any change during the last year.
Yes, I would like to take a look at your RR code, since it would be helpful to see how an experienced program develops its code.
I will send you the code of the 3 units as soon as I have this version of the receptor unit working (at least partially).
Please take in consideration that I have not "cleaned" the code of the transmitter units, as well as this one.
I also have other interests (I am an electronics hobbyist who likes to repair old test equipment, mostly from Tektronix and HP) and it is more fun than spending time on a code that is working...
I seldom use this forum. Does it contain my contact info so you can send me the code?
Thank you very much for your contributions.

Hi Greg,
Thank you for your replies.
I am afraid we have to continue to communicate via Email, since I live in Rio de Janeiro, Brazil.
I have sent you a PM with my email, although it is available in my account info.

One question,
After correcting the array sizes, the errors were gone, but the program fails to recognize when a button is pressed.
I am using a pullup resistor and a debouncing 100nF capacitor.
This behavior happens and goes away, depending on changes I make to the code.
I have observed this behavior so many times that I am convinced that is is software dependent and not hardware related.
I have googled it and none of the answers so far seems to be applicable to this case.
Any ideas?
Thanks

Please let me correct the last message.
The button was not working because I had a broken wire.
However, the time it takes for the button press to be detected varies with the code and I do not understand why.
I have seen dozen of "detect button press " codes and they are very similar to the one I am using.
I have added a 500ms delay on the loop just after the function that detects the button press in a attempt to allow the event to be detected.
What is the standard practice that REAL programmers use?
Thanks

again, hard to say without seeing all of the code, but i had mentioned that the 2 sec delay in ack_Alarm can amount to a 14 sec delay if all alarms are active that blocks other code. the conventional approach is a timer using millis() to avoid blocking