I realize that I have some delays in here but am wondering what takes so long for my commands from the Master, or from the Serial Monitor to get processed .
I am only transmitting a few of the variables but it takes a long time for the state engine to reflect the changes.
I am using the TX/RX pins to send the messages.
case 65://an "A" was sent
Don't you think case 'A': is much easier to read?
Why write eleven lines, when you can do it in seventeen?
if (IsMailMessage){//check for Message
tagMessage = Color [m] [thirdDigit] [2] | message;
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [0]);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [1]);
shiftOut(dataPin, clockPin, LSBFIRST, tagMessage);
digitalWrite(latchPin, HIGH);
delay(500);
}
else{
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [0]);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [1]);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [2]);
digitalWrite(latchPin, HIGH);
delay(500);
}
or
tagMessage = Color [m] [thirdDigit] [2];
if (IsMailMessage)
{
tagMessage |= message;
}
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [0]);
shiftOut(dataPin, clockPin, LSBFIRST, Color [m] [thirdDigit] [1]);
shiftOut(dataPin, clockPin, LSBFIRST, tagMessage);
digitalWrite(latchPin, HIGH);
delay(500);
yeah, probably 
when one codes for themselves, well stuff like that can happen, I guess. I was messing about with different ways to encode the receipt of the transmit from the Master, got shown the State Engine and never touched the Switch.
It seems that It takes several iterations through the main loop to capture a single message, even if I am not running a weather function... I am wondering what I am overlooking.
and I suppose the bit math is a bit flabby too...
I realize that I have some delays in here but am wondering what takes so long for my commands from the Master, or from the Serial Monitor to get processed .
The delay()s are killing you. How long does DisplayTemperature() take to execute? You are calling it on EVERY pass through loop().
for (int m = 0; m < 2;m=m++)
Clearly, you don't understand what m++ does. Assigning the value that is returned to m is silly.
There is NOTHING in the code to explain the significance of the one letter name m. Why are you looping twice?
// wait for 30 milliseconds to see the dimming effect
delay(60);
If you are going to have a useless comment, you MUST keep it up-to-date. Otherwise, you risk looking like a dork. Oops, too late.
long duration = (millis() - starttime)/1000;
//Serial.print("Duration= ");Serial.println(duration);
}
A local variable that immediately goes out of scope is useless.
void translateMessage()
This never gets called. Why is it here?
PaulS:
I realize that I have some delays in here but am wondering what takes so long for my commands from the Master, or from the Serial Monitor to get processed .
The delay()s are killing you. How long does DisplayTemperature() take to execute? You are calling it on EVERY pass through loop().
for (int m = 0; m < 2;m=m++)Clearly, you don't understand what m++ does. Assigning the value that is returned to m is silly.
There is NOTHING in the code to explain the significance of the one letter name m. Why are you looping twice?
// wait for 30 milliseconds to see the dimming effect
delay(60);
If you are going to have a useless comment, you MUST keep it up-to-date. Otherwise, you risk looking like a dork. Oops, too late.long duration = (millis() - starttime)/1000;
//Serial.print("Duration= ");Serial.println(duration);
}A local variable that immediately goes out of scope is useless.void translateMessage()
This never gets called. Why is it here?
The idea is to display the outside and inside temps, thus the two passes through the function. and then the weather. That is all it is doing... that is why I have m incrementing, looping twice, once for each of the two temperatures.
even if I add the millis() timer to the function, it is still a call out of the main loop and essentially a delay for the purposes of handling the Serial message, no?
believe me I am not being defensive in any way, I know my code is shoddy, that is why I'm here!
typedef enum { NONE, GOT_W, GOT_S, GOT_M, GOT_I, GOT_N, GOT_O, GOT_F } states;
If you just used the received letter as the state (withe NONE == zero), you'd simplify quite a bit.
I'd estimate that your code is about 100 lines too big, probably more.
I was using the Nick Gammon example.
I'm not clear what you are saying here:
AWOL:
typedef enum { NONE, GOT_W, GOT_S, GOT_M, GOT_I, GOT_N, GOT_O, GOT_F } states;If you just used the received letter as the state (withe NONE == zero), you'd simplify quite a bit.
but this part I understand.
I'd estimate that your code is about 100 lines too big, probably more.
I have been learning my way through this one milestone at a time. This was my first time with the shift register and I thought I did a good job of getting it to work, albeit it can be simplified. Not being a programmer (aka 'dork' 
) you have to understand that my library of quality experiential examples is quite limited.
I am just in need of getting it so that it doesn't take 5 minutes to process the state changes. I'd like to get it to the point where IsMailMessage is updated once a minute, the others can wait very 15 or even 30 minutes.
I am just in need of getting it so that it doesn't take 5 minutes to process the state changes.
So, you need to figure out where the time is being spent. My candidate is DisplayTemperature(). Try commenting out that call. I know that, in the end, you need the function in place, but to test if that is the function that needs a complete overhaul, try not calling it.
If that is the function that is causing the delays (as I suspect), then there is one thing to do. If not, there is something else to do.
By the way, what are you displaying temperature(s) on?
PaulS:
I am just in need of getting it so that it doesn't take 5 minutes to process the state changes.
So, you need to figure out where the time is being spent. My candidate is DisplayTemperature(). Try commenting out that call. I know that, in the end, you need the function in place, but to test if that is the function that needs a complete overhaul, try not calling it.
If that is the function that is causing the delays (as I suspect), then there is one thing to do. If not, there is something else to do.
that is a great suggestion and I am embarrassed to say that I didn't try it already. Will do immediately.
By the way, what are you displaying temperature(s) on?
an array of LEDs (one for each number, its sign, the scale and the message indicator) where they will backlight the digits on a panel. Sort of an artsy project; it will flash the temperature MINUS, ONE, FOUR, C, for example. I want the message light to appear as if it is independent of what's happening with the temperature, as they are controlled by the same shift register, as you can tell.
I'll be back with the diagnosis.
PaulS:
If that is the function that is causing the delays (as I suspect), then there is one thing to do. If not, there is something else to do.
So DisplayTemperature slows it all down. In fact, it takes at least 3 turns through that function to get the serial processed even though it is passing the serial read call each time....?
thanks for the assistance, by the way.
In fact, it takes at least 3 turns through that function to get the serial processed even though it is passing the serial read call each time....?
If you are sending something like "A", that's three characters. If you are detecting the as the end of packet marker, then, the 3rd character is what it takes to trigger the change.
Clearly, DisplayTemperature() needs to be rewritten to be non-blocking. The blink without delay example shows how, though in a somewhat trivial way. You need to determine, each time DisplayTemperature() is called, if it is time to do something (different). If it is, you need to do something (different). Clearly, what you need to do depends on what you did last time. So, you need a(nother) state machine.
Before you embark on a rewrite, can I just point out that this
if (IsMailMessage){//check for Message
if(IsFarhenheit){//test for Fahrenheit
tagMessage = Farhenheit | message;
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, tagMessage);
digitalWrite(latchPin, HIGH);
delay(500);
}
else{
tagMessage = Celcius | message;
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, tagMessage);
digitalWrite(latchPin, HIGH);
delay(500);
}
recurs in various similar forms.
Factor this into a single function, and save yourself a lot of typing/debugging.
PaulS:
If you are sending something like "A", that's three characters. If you are detecting the as the end of packet marker, then, the 3rd character is what it takes to trigger the change.
OK, I see. I can use a timer for DisplayTemperature and instead keep it on the main loop. My issue will only then be the 6000ms it takes for the weather skits to display...
It seems to me that allowing the serial read embedded in the DisplayTemp (in-loop, timer version) will allow enough opportunity to collect 'complete' messages. That is, I can use the timer version of DisplayTemp to process all the transmissions completely and only really break for the weather. In essence, I can process a complete round of state changes every turn of weather.
I would in essence be able to update IsMailMessage every 6-7seconds.
Thanks, I'm on it!
AWOL:
Before you embark on a rewrite, can I just point out that thisif (IsMailMessage){//check for Message
if(IsFarhenheit){//test for Fahrenheit
tagMessage = Farhenheit | message;
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, tagMessage);
digitalWrite(latchPin, HIGH);
delay(500);
}
else{
tagMessage = Celcius | message;
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, 0x00);
shiftOut(dataPin, clockPin, LSBFIRST, tagMessage);
digitalWrite(latchPin, HIGH);
delay(500);
}
recurs in various similar forms. Factor this into a single function, and save yourself a lot of typing/debugging.
OK, I saw your earlier post and did recognize that I really only needed a single tagMessage function that does the bit math if IsMailMessage. Other problems got my focus first!
Will do!
AWOL:
Before you embark on a rewrite, can I just point out that this recurs in various similar forms.
Factor this into a single function, and save yourself a lot of typing/debugging.
OK, cleaned up:
void DisplayTemperature(){
for (int m = 0; m < 2;m=m++){
if (Subzero [m]){ //Test for a negative temperature, display (-) if true
LEDdisplay(NegativeFlag[0], NegativeFlag[1], NegativeFlag [2]);
}
//Get the first, second and third digits
int outTemp = abs(displayTemp[m]);
firstDigit = outTemp/100;
firstDigit==1 ? t = outTemp-100: t = outTemp;
secondDigit = t/10;
thirdDigit = t % 10;Serial.print(m);Serial.print(" value:"); Serial.print(firstDigit); Serial.print(secondDigit); Serial.println(thirdDigit);
//Display first digit if nonzero
if (firstDigit==1){
LEDdisplay(Color [m] [firstDigit] [0], Color [m] [firstDigit] [1], Color [m] [firstDigit] [2]);
}
//Flash blank if first and second digits are equal
if (secondDigit == firstDigit){
LEDdisplay(0x00, 0x00, 0x00);
}
//
//Display the second digit only if displayTemp is greater than 9
if (abs(displayTemp[m]) >9){
LEDdisplay(Color [m] [firstDigit] [0], Color [m] [secondDigit] [1], tagMessage = Color [m] [secondDigit] [2]);
}
// Flash
if (secondDigit == thirdDigit){
LEDdisplay(0x00,0x00, 0x00);
}
//Display the third digit
LEDdisplay(Color [m] [firstDigit] [0], Color [m] [thirdDigit] [1], Color [m] [thirdDigit] [2]);
//Display Fahrenheit or Celcius
IsFarhenheit == true ? tagMessage = 0x04: tagMessage = 0x02;
LEDdisplay(0x00, 0x00, tagMessage);
//Clear all out
tagMessage = 0x00;
LEDdisplay(0x00, 0x00, tagMessage);
}
}
with:
void LEDdisplay(byte a, byte b, byte c)
{
if (IsMailMessage){
c = c | message;
}
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, a);
shiftOut(dataPin, clockPin, LSBFIRST, b);
shiftOut(dataPin, clockPin, LSBFIRST, c);
digitalWrite(latchPin, HIGH);
delay(500);
}
More than 125 lines out.
Now to get that into a non-blocking format in the main loop
AWOL:
typedef enum { NONE, GOT_W, GOT_S, GOT_M, GOT_I, GOT_N, GOT_O, GOT_F } states;If you just used the received letter as the state (withe NONE == zero), you'd simplify quite a bit.
I'd estimate that your code is about 100 lines too big, probably more.
Sometimes the received letter is a number that follows the letter that sets the state before it's time to evaluate.
To which you may reply; And swallowed a spider to catch the fly, and I don't why she swallowed the fly.
Well, it gets complicated, don't it?
void LEDdisplay(byte a, byte b, byte c)
{
if (IsMailMessage){
c = c | message;
}
digitalWrite(latchPin, LOW);
shiftOut(dataPin, clockPin, LSBFIRST, a);
shiftOut(dataPin, clockPin, LSBFIRST, b);
shiftOut(dataPin, clockPin, LSBFIRST, c);
digitalWrite(latchPin, HIGH);
delay(500);
}
Whatever calls this function needs to set up a start time and interval and put code following the call into a block that checks time whenever loop() runs.
You can use more than one state variable if it will help, or ... if you have a common sub-task in loop() that many other tasks call (like to evaluate numbers) that when it is done needs to run a different state depending on which task called it then (and I've done this) you can save the when-finished-go-to-state in the task that calls the common sub-task and when the common sub-task is done it sets the state to the when-finished value, as below.
------- this is pseudocode to hopefully demonstrate a trick
switch ( state )
{
case step1 // no, I'm thinking up meaningful names for a general example so don't ask!
// something. this may break before the next lines and run next loop as well. it does the next lines when finished
retstate = step10
state = common-task
break
case step2
// something as above
retstate = step11
state = common-task
break
................ other cases between
case step10
// something that picks up from step1, maybe checks time, I dunno, whatever fits
break
case step11
// something that picks up from step2, maybe checks time, I dunno, whatever fits
break
................ other cases between
case common-task
// something, when it's done then run the next lines
state = retstate // the next thing done changes depending on what sent execution to this case
break
................ other cases
}
One definition of program is code + data. You can run generalized code with different data to achieve what many similar blocks of code will do. Imagine if each block costs $100, you'd be generalizing! One thing I found early on fixing a business code package was that about half the bugs I had to deal with were due to bad data. That was one of the drivers to get me to write my own user-I/O to replace the BASIC INPUT$ routines in the original package.
You can go miles datafying programs. Once the code is right, you can pass it data and have it work with all the flexibility you wrote in. It's very satisfying but beware, you may need to write a program to ensure that the data set is good. 
Any program can be bigger than it needs to be. How small it can be and work right is unknown until it's done or the task is so trivial the answer is obvious. And even then, Morris Dovey might come along and surprise you!
Can anyone tell me why this 'blink without delay' doesn't work?
It seems that the lights just flash very fast, rather than in the state during the intervals of the elapsedMillis...
void loop()
{
//
if (Serial.available ())
processIncomingByte (Serial.read ());
//
//DisplayTemperature();
unsigned long currentMillis = millis();
elapsedMillis = currentMillis - previousMillis;
int outTemp = abs(displayTemp[tempState]);
firstDigit = outTemp/100;
firstDigit==1 ? t = outTemp-100: t = outTemp;
secondDigit = t/10;
thirdDigit = t % 10;Serial.print(tempState);Serial.print(" value:"); Serial.print(firstDigit); Serial.print(secondDigit); Serial.println(thirdDigit);
if (elapsedMillis < 500){
//display the negative
if (Subzero [tempState]){
LEDdisplay(NegativeFlag[0], NegativeFlag[1], NegativeFlag [2]);
}
}
if (501 < elapsedMillis < 750){
LEDdisplay(0x00, 0x00, 0x00);
}
if (751 < elapsedMillis < 1250 ){
if (firstDigit==1){
LEDdisplay(Color [tempState] [firstDigit] [0], Color [tempState] [firstDigit] [1], Color [tempState] [firstDigit] [2]);
}
}
if (1251 < elapsedMillis < 1500){
LEDdisplay(0x00, 0x00, 0x00);
}
if (1501 < elapsedMillis < 2000){
if (displayTemp [tempState] > 9){
LEDdisplay(Color [tempState] [secondDigit] [0], Color [tempState] [secondDigit] [1], Color [tempState] [secondDigit] [2]);
}
}
if (2001 < elapsedMillis < 2250){
LEDdisplay(0x00, 0x00, 0x00);
}
if (1501 < elapsedMillis < 2000){
LEDdisplay(Color [tempState] [thirdDigit] [0], Color [tempState] [thirdDigit] [1], Color [tempState] [thirdDigit] [2]);
}
if (2001 < elapsedMillis < 2250){
LEDdisplay(0x00, 0x00, 0x00);
}
if (2251 < elapsedMillis < 2750){
IsFarhenheit == true ? tagMessage = 0x04: tagMessage = 0x02;
LEDdisplay(0x00, 0x00, tagMessage);
}
if (2751 < elapsedMillis < 4000){
LEDdisplay(0x00, 0x00, 0x00);
}
if (elapsedMillis > 4000){
previousMillis = currentMillis;
tempState == 1 ? tempState = 0: tempState = 1;
}
}
if (501 < elapsedMillis < 750){
What do you think that does?
You're almost certainly wrong.
Look at using &&
I've been looking at the code attached to the first post in this Thread (not sure if that is the latest complete version) and I have also read over the relevant posts in the other Thread.
It's not clear to me what input the program is receiving. In Reply #24 in the other Thread it seems as if the format is "W1S1M1I72N1O95F1x"
If this were my project my loop() would probably be as short as
void loop() {
receiveBytes(); // this just puts the bytes into a message buffer without caring what values they have
processBytes();
displayTemperature();
}
I would organize things so that no processing would take place until a full message is received - that avoids the problem of the processing interfering with the reception. It makes the receiving code very simple.
It also means that all of the data is available so there is no wait for more information before the processing can continue.
And (if you use alternating message buffers) it does not prevent parts of a new message from being received while the existing message is being processed. That may make the time to receive a message invisible.
As I think others are pointing out, the fading needs to be done using millis() rather than delay().
As far as I can see all the "weather" routines (sunny, cloudy etc) are the same apart from the pins they act on. It should be possible to do it all in one function with the relevant pin data in an array and appropriate parameters or global variables. That will be especially valuable when it comes to converting the code to use millis() - only one piece of code will need to be produced.
...R