I have an arduino controlled inflatable installation that runs through a cycle of inflating and deflating. With help from this forum I cobbled together code that has been working for the past three days. It seemed to get stuck on a certain position in the program that did not make sense to me.
I use both millis and an RTC for timing. I wonder if I am using millis incorrectly and at some point the checking of millis caused it to freeze?
I think it froze performing the function deflate();
Inline is the Switch case that I am using as well as the millis timing function in case there is something obvious there.
I attached the complete code below.
void loop() {
currentMillis = millis();
Blink15Min();
Blink4Times();
WinchTime();
InflateCycle();
}
//Delay time expired function
boolean CheckTime(unsigned long &lastMillis, unsigned long wait)
{
//is the time up for this task?
if (currentMillis - lastMillis >= wait)
{
lastMillis += wait; //get ready for the next iteration
return true;
}
return false;
}
//END of CheckTime()
void InflateCycle() {
switch (wState) {
case FirstInflate:
inflate ();
if (CheckTime(InflateWait, 2400000UL)) {
InflateWait = millis(); //initialize the next wait time
wState = StayInflated;
}
break;
case StayInflated:
inflate ();
if (CheckTime(InflateWait, InflatedPeriod)) {
InflateWait = millis(); //initialize the next wait time
if (WinchHours == 1) {
wState = Deflate;
}
else if (WinchHours == 0 ) {
wState = StayInflated;
}
}
break;
case Deflate:
deflate();
if (CheckTime(InflateWait, DeflatePeriod)) {
InflateWait = millis(); //initialize the next wait time
WinchUpCount = 3;
wState = WinchUpCycle;
}
break;
case WinchUpCycle:
if (WinchUpCount > 0 && WinchHours == 1) {
if (winchUpState == HIGH) {
if (CheckTime(InflateWait, UpOffTime)) {
winchUpState = LOW; // Turn it on
digitalWrite(winchUpPin, winchUpState);
InflateWait = millis(); //initialize the next wait time
}
}
else {
if (winchUpState == LOW) {
if (CheckTime(InflateWait, UpOnTime)) {
winchUpState = HIGH; // turn it off
digitalWrite(winchUpPin, winchUpState);
WinchUpCount = WinchUpCount - 1;
InflateWait = millis(); //initialize the next wait time
}
}
}
}
else {
wState = StayDeflated;
InflateWait = millis(); //initialize the next wait time
}
break;
case StayDeflated:
deflate();
if (CheckTime(InflateWait, DeflatedPeriod)) {
wState = Inflate;
InflateWait = millis(); //initialize the next wait time
}
break;
case Inflate:
inflate();
if (CheckTime(InflateWait, InflatePeriod)) {
InflateWait = millis(); //initialize the next wait time
WinchDownCount = 2;
wState = WinchDownCycle;
}
break;
case WinchDownCycle:
if (WinchDownCount > 0) {
if (winchDownState == HIGH) {
if (CheckTime(InflateWait, DownOffTime)) {
winchDownState = LOW; // Turn it on
digitalWrite(winchDownPin, winchDownState);
InflateWait = millis(); //initialize the next wait time
}
}
else {
if (winchDownState == LOW) {
if (CheckTime(InflateWait, DownOnTime)) {
winchDownState = HIGH; // turn it off
digitalWrite(winchDownPin, winchDownState);
InflateWait = millis(); //initialize the next wait time
WinchDownCount = WinchDownCount - 1;
}
}
}
}
else {
wState = StayInflated;
InflateWait = millis(); //initialize the next wait time
}
break;
} // end switch(wState)
} // end inflate cycle
I thought millis ran over after 49 days not 3. The Arduino is always plugged in so maybe that is why? But the power was cut 4 days ago so wouldn't that reset the millis timer and eliminate that as a possible problem?
tammytam:
The code you linked and the code you posted are different.
The code you posted didn't call the InflateCycle() function.
I only posted an excerpt inline and attached the full code.
I mean a "cold air inflatable" similar to the gorillas at car dealers or kids bounce houses.
I agree. I just am not sure there the code is messing up. I went through security camera footage and it seemed to get stuck at 4:37 pm which is strange since I only have changes on the hour aside from the millis multi tasking.
Because the deflate fan was running and the inflatable was deflated. The only way for the deflate fan to run is for the deflate() function to be called. The strange thin is that the nightLights() function also seemed to be running this morning past its time so two functions seem to be stuck.
So, I re-uploaded the code and everything is working as expected. Is there some error in my if statements? I know that I did not code everything in the best way but did it in a way that seemed to work as I am learning. I am not a coder.
Thank you for the help. Any suggestions on where to look for problems?
Lukinio:
I only posted an excerpt inline and attached the full code.
Don't think you understand what Im getting at. The inflateCycle() is not even being called in your attached code. And if the attached code is not the code thats blocking, then there's no point combing through it.
This is used a lot, although not an issue as you reset most of the timers after calling it:
boolean CheckTime(unsigned long &lastMillis, unsigned long wait)
{
//is the time up for this task?
if (currentMillis - lastMillis >= wait)
{
lastMillis += wait; //change this line to lastMillis = currentMillis;
return true;
}
return false;
}
// typically called liek this
if (CheckTime(MinuteTime, 2000UL)) {
MinuteTime = millis(); // ***This line not needed with above change
If you replace "lastMillis += wait;" with "lastMillis = currentMillis;" You won't have to reset the timer each time you call the function.
tammytam:
Don't think you understand what Im getting at. The inflateCycle() is not even being called in your attached code. And if the attached code is not the code thats blocking, then there's no point combing through it.
My mistake. inflateCycle() was commented out during a code reset this morning. I did not upload the versions that I am using with it not commented out. It is not commented out in the main loop when it is uploaded to the installation.
tammytam:
This is used a lot, although not an issue as you reset most of the timers after calling it:
Is there a time that I do not reset the timer that you see? That could be it.
tammytam:
If you replace "lastMillis += wait;" with "lastMillis = currentMillis;" You won't have to reset the timer each time you call the function.
I did it with the "lastMillis += wait;" since I saw an example on the forums doing it that way so the timing would be more accurate by adding the interval. If it might be causing the problem then I can change it but if not then I want to find out why it is stuck.
Do you mean electro magnetic interference? Does that mean having the Arduino in a box with other wires will cause errors? I do not have a schematic. The hardware box is inaccessible right now but I have this partial shot that I took of the relay blocks.
We use the == operator to compare, a single = is assignment. ie:
I understand that and have made that mistake before. I do not see anywhere that I wrote "if (winchDownState = LOW)" I did a find in the code and couldn't locate it.
Edit: I do use the "==" to compare the LOW / HIGH I do not use the single "=" in the if statements. Where do you see that?
tammytam:
You've used the equality operator here too:
The installation also functions like a belltower by blinking the lights. I was using that to make it a 12 hr clock rather than 24hr. Thank you for spotting it.
Is this another case of the code you linked is not your current code ....
This is why I had an issue with spending time reading the wrong file at the beginning despite you convincing me the only change was a commented out line.
Open the file you attached and make sure its the current code, if not, re attach the actual current code.
tammytam:
Is this another case of the code you linked is not your current code ....
This is why I had an issue with spending time reading the wrong file at the beginning despite you convincing me the only change was a commented out line.
Open the file you attached and make sure its the current code, if not, re attach the actual current code.
Tammytam, you are right. The code is different. That is strange because I just did a save as of the current code this morning and it seemed to change the "==" to "=". Obviously the computer did not do that I somehow I got the versions mixed up. Not sure how though.
In any event, I just did a save as of the code I uploaded 30 sec ago.
Next question is, did you compile and upload the correct version to the actual Arduino?
Both compile, but one is obviously wrong. Unfortunately no easy way to verify this other than ensuring the correct version is uploaded and see if it does it again.
tammytam:
Next question is, did you compile and upload the correct version to the actual Arduino?
Both compile, but one is obviously wrong. Unfortunately no easy way to verify this other than ensuring the correct version is uploaded and see if it does it again.
I agree. So now I uploaded the "correct version." I am now sure how the "incorrect version" got into circulation. Nevertheless, it is uploaded now and it seems to be functioning. If it makes it past 3-4 days I will consider it solved. If not I will report back here with new ideas and questions.
but its never set anywhere other than globally. Is this intentional?
That is residual code from my first attempt to set a variable to indicate when it was ok to winch and when it wasn't. That evolved into the function winchTime() that specifies how long one case in the switch case is. In the middle of the night and before the venue opens, the stayInflated state is much longer so nothing really happens then.
if (WinchHours == 1) {
wState = Deflate;
}
else if (WinchHours == 0 ) {
wState = StayInflated;
}
I will remove this code since it is better to not have extra stuff. I looked at it and wondered if it was getting stuck at "wState = Deflate;" but quickly realized that it wouldn't because then it goes to the next case.
case WinchUpCycle:
if (WinchUpCount > 0 && WinchHours == 1)
{
if (winchUpState == HIGH)
{
if (CheckTime(InflateWait, UpOffTime))
{
winchUpState = LOW; // Turn it on
digitalWrite(winchUpPin, winchUpState);
InflateWait = millis(); //initialize the next wait time
}
}
else
{
if (winchUpState == LOW)
{
if (CheckTime(InflateWait, UpOnTime))
{
winchUpState = HIGH; // turn it off
digitalWrite(winchUpPin, winchUpState);
WinchUpCount = WinchUpCount - 1;
InflateWait = millis(); //initialize the next wait time
}
}
}
}
else
{
wState = StayDeflated;
InflateWait = millis(); //initialize the next wait time
}
break;
I don't like this state... I've mentioned it previously. Lets strip that back down, as its not being used anyway:
case WinchUpCycle:
{
wState = StayDeflated;
InflateWait = millis(); //initialize the next wait time
}
break;
This one also needs stripping down, and is more of a concern as the logic allows it to actually get in this one and dance about for a while until WinchDownCount is 0 (I'm only guessing it gets this far). Lets also strip this right back:
case WinchDownCycle:
{
wState = StayInflated;
InflateWait = millis(); //initialize the next wait time
}
break;
I'm concerned your lights have stayed on, does sound like it may of locked up, and if it wasn't for the fact it can be reproduced accurately every 2 days, I would say check the wiring.
Also might be worth littering your .ino with Serial.print(), in each state of each state machine. then either log it, or plug your computer into it when its crashed.
tammytam:
I don't like this state... I've mentioned it previously. Lets strip that back down, as its not being used anyway:
case WinchUpCycle:
{
wState = StayDeflated;
InflateWait = millis(); //initialize the next wait time
}
break;
If I strip it down that much then the winch won't be able to run through its cycle. Since the winch is not a servo I count the seconds that it takes to go up and down and divide it into segments. So for the WinchDownCycle it is two 15 second runs and the WinchUpCycle is three 10 second runs. FYI there are mechanical limit switches that prevent it from overrunning in either direction.
I could take out that winch counter and just have the winch go for the full ~30 seconds each direction but wanted that to be something I could control.
tammytam:
This one also need stripping down, and is more of a concern as the logic allows it to actually get in this one and dance about for a while until WinchDownCount is 0 (I'm only guessing it gets this far). Lets also strip this right back:
case WinchDownCycle:
{
wState = StayInflated;
InflateWait = millis(); //initialize the next wait time
}
break;
So you mean that it shouldn't count the winch in either direction?
tammytam:
I'm concerned your lights have stayed on, does sound like it may of locked up, and if it wasn't for the fact it can be reproduced accurately every 2 days, I would say check the wiring.
I would think the wiring as well but I can manually send the Arduino commands to do any of the functions, so it probably isn't the wiring. All AC as well as the DC for the winch are controlled by relays that are controlled by the Arduino.
tammytam:
Also might be worth littering your .ino with Serial.print(), in each state of each state machine. then either log it, or plug your computer into it when its crashed.
I will do that.
One thought, could it be my overuse of global variables? I know that this is not best practice but do not know how to change it.
I think about the lights staying on and that state is controlled by a global "hr" variable.
