Serial Slow using State Engine

Almost certainly, funny.

So thanks!!

I added a counter on the displayTemp part to goo through two displays and then does weather. Works really well, but I think I will leave getting the weather skits embedded for another day.

Thanks for your help.

I attached the ‘semi-final’ if you have any comments or more ideas for improvements XD

ClockSlave4.ino (13.1 KB)

BulldogLowell: Can anyone tell me why this 'blink without delay' doesn't work?

Can you tell us (in English) what you think it should be doing - because I can't make any sense of it (even allowing for the error that @AWOL pointed out.

You have very confusing IF statements with a succession of increasing elapsed times but different subsidiary tests.

Usually it is more effective to test from high to low because if (x > 300) automatically exculdes a later else if (x > 200) without anything being overlooked. (Hope I have that the right way round).

...R

Just a bit about using successive if-else-if-else-if-else-etc to chop up a range of possibilities.

// the first if slices off everything under 500, then you keep slicing farther and farther
  if ( elapsedMillis < 500 )  // ** why put the brace on a line of its own?
  {                                       // ** because it is a lot easier to visually balance braces with the same indents
                                          // ** spaces can also make it easier to tell { from ( from 1, etc
    //display the negative
    if ( Subzero [tempState] )
    {
      LEDdisplay(NegativeFlag[0], NegativeFlag[1], NegativeFlag [2]);
    }
  }
// the next if follows an else, the first slice is gone so the next gets everything under 750
  else if (elapsedMillis < 750)
  {
    LEDdisplay(0x00, 0x00, 0x00);
  }
// and we can chain else-if's to the max. next slice!
  else if ( elapsedMillis < 1250 )
  {
    if (firstDigit==1){
      LEDdisplay(Color [tempState] [firstDigit] [0], Color [tempState] [firstDigit] [1], Color [tempState] [firstDigit] [2]);
    }
  }
// ha! you already knew!
   else if ( elapsedMillis < 1500 )
  {
    LEDdisplay(0x00, 0x00, 0x00);
  }
// and then forgot.
  else if ( elapsedMillis < 2000 )
  {
    if (displayTemp [tempState] > 9)
    {
      LEDdisplay(Color [tempState] [secondDigit] [0], Color [tempState] [secondDigit] [1], Color [tempState] [secondDigit] [2]);
    }
  }
  else if ( elapsedMillis < 2250)
  {
    LEDdisplay(0x00, 0x00, 0x00);
  }

// ------------ this part belongs inside the if < 2000 block above
  if ( 1501 < elapsedMillis < 2000)
  {
  LEDdisplay(Color [tempState] [thirdDigit] [0], Color [tempState] [thirdDigit] [1], Color [tempState] [thirdDigit] [2]); 
  }

// ------------ this part belongs inside the if < 2250 block above
   if (2001 < elapsedMillis < 2250)
  {
    LEDdisplay(0x00, 0x00, 0x00);
  }

// I leave the rest to you, in time you can reorganize it but still use the same "what to do" lines inside the if's

  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;
  }

BulldogLowell: I attached the 'semi-final' if you have any comments or more ideas for improvements

I wrote my earlier post before seeing this and I think my comments still apply.

If you were to leave this project now and look at the code after 6 month's absence would you understand it after a single read-through?

...R

Robin2:
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

I am illuminating single LEDs by moving three bytes into a shift register that controls 24 LED’s. In the loop, I first send the SIGN, then the FIRST digit then the SECOND digit, THIRD digit and the SCALE. I do that twice for two sets of LEDs to indicate by colour indoor or outdoor temperature. I then am running the weather skit if there is weather reported.

this is all interlaced with an email indicator that illuminates when I have email (Master side) and furs off when mailbox is empty.

It is sort of a household visual information piece.

and from your first post…

Actually the state engine works well and I can transmit smaller chunks for more frequent updates of say Email indicator versus outdoor temperature.

So far on this project I learned a lot. Believe me I would love to get into the integration of the skits with the fade in the main loop but right now I’m a long way from where I was (on this project I did a shift register, connected two arduinos, and used a state engine… not bad for an ME).

any other ideas are appreciated.

  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) { 
    // sets the value (range from 0 to 255):
    analogWrite(pwmPin2, 255-fadeValue);
    analogWrite(pwmPin, fadeValue/2.5);         
    // wait to see the dimming effect    
    delay(60);

(255 / 5) * 60 = 3060 ms.
Times 2 (fade up and down) , is over six seconds

AWOL: (255 / 5) * 60 = 3060 ms. Times 2 (fade up and down) , is over six seconds

yes I 100% agree, but it looks [u]really[/u] cool...

I am really getting close to finalizing all of the breadboard stuff and onto the build. putting this stuff on the 2nd board was key to my keeping int somewhat manageable, so far.

That for-next loop needs to come out of the for-next and get put into loop() as a task. Whatever the code is before this it must set start time to millis() and state to fade. And where I write state, you need to come up with a name that says What This State Is For unless it's the only state variable.

if (( state == fade ) && ( now - start >= 60 )) { // analog stuff if ( fadeValue == 255 ) { // set state for the next sub-task --- use state(s) to control execution, not nested loops, etc } else { fadeValue += 5; start = now; } }

Not sure I understand...

are you saying that I layer in all 6 skits into the loop() and then use state engine to toggle between them?

All six could be made to fit into one. but otherwise, yes. It won't block or slow loop() down that way.

Once you trim loop() down but taking out blocking code and splitting long tasks into sub-task pieces, you should find that loop() will average a few to 20+ executions per millisecond.

A processor running at 16 MHz has 16000 cycles per milli(). Simple instructions run in 1 or 2 cycles though an integer multiply or divide takes a lot more and floating point makes that look fast. The compiler is good at optimizing though as I have found in timing algorithms using micros() here and there, and checking millis() between finishing processing one serial char and the arrival of the next. I consider the 105 micros it takes to analog read as quite a while in Arduino time. But check and always think of algorithm alternatives, it's faster to shift bits than multiply by powers of 2 and to add or subtract than multiply or divide so math that can arranged in those operations will generally run faster.

I will try to explain again:

AWOL:

  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) { 

// sets the value (range from 0 to 255):
    analogWrite(pwmPin2, 255-fadeValue);
    analogWrite(pwmPin, fadeValue/2.5);         
    // wait to see the dimming effect   
    delay(60);



(255 / 5) * 60 = 3060 ms.
Times 2 (fade up and down) , is over six seconds

Most of the time spent running that loop, nothing is being done. Zip, Nada, Zilch, NUTHIN.

Even if you take the delay() out it is still fixated on going through 256 repetitions of analogWrite(), one of which has a floating point divide which has GOT TO GO, I WILL HELP WITH THAT.

Really, I should have looked closer. Floats are like dragging blocks of concrete when it comes to run speed.

By having loop() and a timer drive the analogWrites you gain most of the time lost by the delay (delay is gone but time checks added) and other code gets a chance to execute in that regained time while the analogWrites still get to run every 60 ms.

That is win-win-win instead of win-lose-lose.

A small wheel that turns incredibly fast will cover more ground per second than a big wheel that turns in slow and halting fashion. YES, you want to break up that for-next loop!

Now about those analogWrites and that float. I know that this is one “pattern” out of many so when you’re ready, lets get them all together and I may be able to give you a 1 size fits all function that you never saw coming.

You did have a good bit of flash left over, even before trimming so many lines from the sketch already, no?
That’s your first clue.
Second clue is: Do you know how PC’s ran even the poor flight sims they did on toy computers like the C-64?

@BulldogLowell, the demo sketch attached to the first post of this Thread illustrates in a very simple way how several LEDS can blink at different times without using blocking code.

I think you need to apply the same general approach to your "weather" function - as well as doing all the weather types in a single function.

...R

Who'da thunkit?

Thanks for the continued support!

GoForSmoke: You did have a good bit of flash left over, even before trimming so many lines from the sketch already, no? That's your first clue. Second clue is: Do you know how PC's ran even the poor flight sims they did on toy computers like the C-64?

You helped with the State Engine and I am still listening. BTW, I found it interesting that taking out so much code (and accomplishing the same thing) didn't really take down the flash so much. There is still plenty of room, yes.

Robin2: @BulldogLowell, the demo sketch attached to the first post of this Thread illustrates in a very simple way how several LEDS can blink at different times without using blocking code.

I think you need to apply the same general approach to your "weather" function - as well as doing all the weather types in a single function.

Thanks Robin, I actually searched before how to 'enhance' blink without delay in order to overlay the thermometer function into the loop() and found your post, thanks (again).

My problem is conceptualizing taking something that happened in the for-next loop and overlaying that with what I have in the loop() already. I guess I have to define just how long that loop will take (timer) and affix events similarly to the way I accomplished it with the thermometer, but then with the fades, I lose it because I am thinking in a linear fashion.

I found it interesting that taking out so much code (and accomplishing the same thing) didn’t really take down the flash so much

Maybe not, but just think how much easier it will be to fix any bugs - you only have to do it in one place.

AWOL:

I found it interesting that taking out so much code (and accomplishing the same thing) didn't really take down the flash so much

Maybe not, but just think how much easier it will be to fix any bugs - you only have to do it in one place.

Right, no argument here. I appreciate the pointers and am quite pleased to do more with less. Thanks again!

BulldogLowell:
My problem is conceptualizing taking something that happened in the for-next loop and overlaying that with what I have in the loop() already. I guess I have to define just how long that loop will take (timer) and affix events similarly to the way I accomplished it with the thermometer, but then with the fades, I lose it because I am thinking in a linear fashion.

Let’s look at one of those. It has 2 sections that do the same basic thing but 1 counts up and the other, down.

//Weather Skits
void PartlyCloudy ()  //Duration is about 6 seconds on an UNO
{ 
  // fade in from min to max in increments of 5 points:
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) { 
    // sets the value (range from 0 to 255):
    analogWrite(pwmPin2, 255-fadeValue);
    analogWrite(pwmPin, fadeValue/2.5);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(60);                            
  } 
  // fade out from max to min in increments of 5 points:
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) { 
    // sets the value (range from 0 to 255):
    analogWrite(pwmPin2,255-fadeValue);
    analogWrite(pwmPin, fadeValue/2.5);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(60);                            
  }
}

These are the 2 lines that matter. Everything else it feed these lines a series of values on a timed basis.

    analogWrite(pwmPin2,255-fadeValue);
    analogWrite(pwmPin, fadeValue/2.5);

And that is called by:

  if (IsWeather){
    Serial.println("there is weather");
    switch (weatherValue)
      {
        case 65://an "A" was sent
          Serial.println("sunny");
          Sunny();
          break;
        case 66://a "B" was sent
          Serial.println("partly cloudy");
          PartlyCloudy();
          break;
        case 67://a "C" was sent
          Serial.println("cloudy");
          Cloudy();
          break;
        case 68://a "D" was sent
          Serial.println("thunderStorms");
          ThunderStorms();
          break;
        case 69://a "E" was sent
          Serial.println("rain");
          Rain();
          break;
        case 70://a "F" was sent
          Serial.println("snow");
          Snow();
          break;
      }
  }

And weatherValue is set by processWeather.

void processWeather (const unsigned int value)
{
  // do something with Weather 
  Serial.print ("weather = ");
  //Serial.println (value);
  weatherValue = WeatherTable[value];
  //Serial.println("weathervalue = "+ weatherValue);
  IsWeather = true;
} // end of processWeather

As the sketch is, loop() has a task block I will call Weather that runs if (IsWeather).
It uses weatherValue as set by processWeather().

For shortness and simpicity, I will show adapting a version of the Weather task that only has PartlyCloudy.
Also needed will be a couple of global bytes:
byte weatherProgress = 0;
byte fadeValue = 0;
And a set of time keeping variables:
unsigned long tNow; // will be set to millis() often
unsigned long tStart = 0UL;
unsigned long tWait = 60UL; // you can vary this (or not) later, it is the weather process delay period

Remember, for the example I only consider PartlyCloudy.
All the setup for whatever weather to display is done here, each Weather task runs until stopped.

void processWeather (const unsigned int value)
{
  // do something with Weather 
  Serial.print ("weather = ");
  //Serial.println (value);
  weatherValue = WeatherTable[value];
  //Serial.println("weathervalue = "+ weatherValue);
  IsWeather = true;

    Serial.println("there is weather");
    switch (weatherValue)
    {
         case 66://a "B" was sent
          Serial.println("partly cloudy");
          break;
    }

// does anybody really know what time it is? does anybody really care? ask Chicago
    tStart = millis();

// these two may differ for other weather types, though probably never weatherProgress
  weatherProgress = 0;
  fadeValue = 0;
} // end of processWeather

And here is the shortened for example Weather task

  if (IsWeather)
  {
    tNow = millis();
    if ( tNow - tStart >= tWait )
    {
      tStart = tNow;

      switch (weatherValue)
      {
          case 'B' :// Partly Cloudy

          analogWrite(pwmPin2,255-fadeValue);
          analogWrite(pwmPin, fadeValue/2.5);         

          if ( weatherProgress == 0 ) // increase fadeValue until 255
          {
             if ( fadeValue == 255 )
             {
               weatherProgress = 1;
             }
             else
             {
                fadeValue += 5;
              }
          }
          else
          {
             if ( fadeValue == 255 )
             {
               weatherProgress = 0;
             }
             else
             {
                fadeValue -= 5;
              }
          }
          break;

      } end switch ( weatherValue )        

    } // end time check

  } // end if ( IsWeather )

I think that should do it for now.

OK, I am going to fiddle with this, I see what you are doing.

let me read it back to you so you know that I understand...

Because the processor is so fast, it makes this possible (i.e. your earlier point) because loop() will Weather Task fast enough as to process the increment/ decrement of the fade. We will control the 'speed' of the fade by attaching a multiplier to leave it in a state of increment(decrement) for a longer(shorter) duration.

Do I have it right, sort of philosophically speaking?

BulldogLowell:
OK, I am going to fiddle with this, I see what you are doing.

let me read it back to you so you know that I understand…

Because the processor is so fast, it makes this possible (i.e. your earlier point) because loop() will Weather Task fast enough as to process the increment/ decrement of the fade. We will control the ‘speed’ of the fade by attaching a multiplier to leave it in a state of increment(decrement) for a longer(shorter) duration.

Do I have it right, sort of philosophically speaking?

Sort of. See the attached code. (I would have inlined it, but it is too large for the character limit. It used to fit, but ever since I added the section for diagnostics to UECIDE grapher, by itself it is too big.)

See how I only change the value of the PWM pin once per call to fadeOutput(), and I only call fadeOutput() when a pre-calculated number of ms have passed? Also carefully look at the fadeOutput() function and see how I use one function to both fade up and down at potentially different rates. Also, note that my timing for the fading is separate from the timing for taking readings from the sensor connected at A0. Some times through loop() none of the functions will be called. Sometimes one or the other will be called. And, sometimes both will be called (particularily if diagnostics() is called, that function has a metric tonne worth of clock cycles flushed down the toilet writing to the serial port).

SharpDistanceSensorPWMoutAverageMedians.ino (10.5 KB)

OK, well I did make the edits you recommended and it 'works'

It fades very slowly, almost not noticeably. I tried to play around with

unsigned long tWait = 60UL;

but it got me nowhere, really.

I can't say I have seen this UL unsigned long constant before. Is that to force the math?