Putting a While Loop within a While Loops

[Derekspeegle 0]

I'm having an issue with Energia where it isnt allowing me to put a while loops within another while loops or any loop for that matter. I need my program to run a loop for as long as the source is correct but run another loop inside of it a specific number of times. It always gives me an error asking for a ";" before a numeric value. Here is my code. 

const int voltPin = P1_5; 
const int voltoutPin = P1_7; 
const int voltoutPin2 = P1_6; 
float voltage1;
float voltage2;
float voltage3;
int fadeValue;
int fadeValue2;







void setup() {


pinMode(voltoutPin, OUTPUT);
pinMode(voltoutPin2, OUTPUT);
pinMode(voltPin, INPUT);
analogFrequency(10000);

}  

void loop() { 
  while(voltage3 >= 12){
    while(fadeValue < 255){
  fadeValue ++10;  
  analogWrite(voltoutPin, fadeValue);
}
analogWrite(voltoutPin, fadeValue);
while(fadeValue2 > 0){
fadeValue2 --10;
  analogWrite(voltoutPin2, fadeValue2);
}
  }
   while(voltage3 < 12){
while(fadeValue2 < 255) {
 fadeValue2 ++10;
  analogWrite(voltoutPin2, fadeValue2);
}
analogWrite(voltoutPin2, fadeValue2);
while(fadeValue > 0){
fadeValue --10;
  analogWrite(voltoutPin, fadeValue);
}
  }
  
  
  
  
  
  float voltage;

  voltage1 = analogRead(voltPin);

  voltage2 = (voltage1 / 1024) * 3.3;

  voltage3 = voltage2 * 5.65;



} // void loop close 

Let me know how that I can imbed a loop within another loop or just a better way to do this function. Essentially I'm reading a voltage then when the voltage reaches a certain limit I start to cutoff the voltage source by cutting off the PWM. When I cut it off instantly it shut down the whole system, probably from a voltage surge. So I want it now to ramp it down to avoid that. 

[spirilis 1,265]

what is "++10" or "--10"?  never seen that notation

 

usually it's += 10 or -= 10

 

also can you please indent the code correctly?  It's hard to ascertain what right-braces "}" corresponds to which loop the way it's formatted here.

[Derekspeegle 0]

Here's my updated code, it should make more sense.  Instead of trying to do a while within a while, I just changed it so that the values are constrained. It isnt working the way it should though. The output will go to 100% duty cycle but when It is suppose to fade out it just starts going haywire. I don't think its fading to 0 for some reason. Look at this updated code and let me know what can be done. Thanks. 

const int voltPin = P1_5; 
const int voltoutPin = P1_7; 
const int voltoutPin2 = P1_6; 
float voltage1;
float voltage2;
float voltage3;
int fadeValue;
int fadeValue2;


 





void setup() {


pinMode(voltoutPin, OUTPUT);
pinMode(voltoutPin2, OUTPUT);
pinMode(voltPin, INPUT);
analogFrequency(10000);

}  

void loop() { 

  
  
 


  //Obtain RAW voltage data
voltage1 = analogRead(voltPin);
voltage2 = (voltage1 / 1024) * 3.3;
voltage3 = voltage2 * 5.65; 
  


//If the priority is high 
 while(voltage3 >= 12) { 
fadeValue = constrain(fadeValue, 0, 255);  
fadeValue2 = constrain(fadeValue2, 0, 255); 
voltage1 = analogRead(voltPin);
voltage2 = (voltage1 / 1024) * 3.3;
voltage3 = voltage2 * 5.65; 
fadeValue = fadeValue + 10;  
fadeValue2 = fadeValue2 - 10;
analogWrite(voltoutPin, fadeValue);
analogWrite(voltoutPin2, fadeValue2);
 }



// If priority is low
while(voltage3 < 12) {
fadeValue = constrain(fadeValue, 0, 255);  
fadeValue2 = constrain(fadeValue2, 0, 255);   
voltage1 = analogRead(voltPin);
voltage2 = (voltage1 / 1024) * 3.3;
voltage3 = voltage2 * 5.65;   
fadeValue = fadeValue - 10;
fadeValue2 = fadeValue2 + 10;
analogWrite(voltoutPin2, fadeValue2);
analogWrite(voltoutPin, fadeValue);
}

  
  
  
 

}  

[spirilis 1,265]

Well I don't see anything glaring wrong with the code (edit: other than using float's, although if it doesn't need blistering performance that might still be OK), but I should add if this is MSP430 there's a glitch in the way analogWrite() works where it will do short-cut updates to the PWM registers (without waiting for the period to reset) when you repeatedly analogWrite a port.  The result is glitchy garbage.  What chip (or launchpad) are you using?

[abecedarian 330]

One thing I see is that during the first pass through loop(), "fadeValue" and "fadeValue2" are being accessed in constrain() functions without having any value assigned to them prior to first use.

[abecedarian 330]

@@Derekspeegle - maybe...:

const int voltPin = P1_5; 
const int voltoutPin = P1_7; 
const int voltoutPin2 = P1_6; 
float voltage1;
float voltage2;
float voltage3;
int fadeValue;
int fadeValue2;

// get voltages function 
void getVoltages() {
  //Obtain RAW voltage data
  voltage1 = analogRead(voltPin);
  voltage2 = (voltage1 / 1024) * 3.3;
  voltage3 = voltage2 * 5.65; 
}

void setup() {

  // configure input and output pins as required:
  pinMode(voltPin, INPUT);
  pinMode(voltoutPin, OUTPUT);
  pinMode(voltoutPin2, OUTPUT);

  // configure analog/PWM output frequency:
  analogFrequency(10000);

  // get our voltages
  getVoltages();

  // set initial fade values based on voltage3 priority
  // may need some work + additional math so initial values scale properly
  if (voltage3 >=12) {   // high priority
    fadeValue = 0;       // will start at 0 and count up
    fadeValue2 = 255;    // will start at 255 and count down
  }  
  else {                 // low priority
    fadeValue = 255;     // will start at 255 and count down
    fadeValue2 = 0;      // will start at 0 and count up
  }
}

void loop() { 
  // When the sketch started running, voltage3, fadeValue and fadeValue2 were
  // initialized in setup() so we don't need to get them again, at the moment.
  // However, after the first pass through loop(), fadeValue and fadeValue2 
  // will have been re-calculated within loop(), so all we need to do is get
  // voltage3 before we loop() again, so fadeValue and fadeValue2 can be 
  // re-calculated before the outputs are changed.

  // Decide what to do based on voltage3 priority:
  if (voltage3 >= 12) {  // priority is high 
    fadeValue += 10;     // add 10 to fadeValue; store result in fadeValue
    fadeValue2 -= 10;    // sub 10 from fadeValue2; store result in fadeValue2

    // limit fadeValue and fadeValue2 to values from 0 to 255
    fadeValue = constrain(fadeValue, 0, 255);  
    fadeValue2 = constrain(fadeValue2, 0, 255); 
  }  
  else  {                // priority is low
    fadeValue -= 10;     // sub 10 from fadeValue; store result in fadeValue
    fadeValue2 += 10;    // add 10 to fadeValue2; store result in fadeValue2

    // limit fadeValue and fadeValue2 to values from 0 to 255
    fadeValue = constrain(fadeValue, 0, 255);  
    fadeValue2 = constrain(fadeValue2, 0, 255);   
  }

  // update voltout pins:
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, fadeValue2);

  // get our voltages for the next pass through loop()
  getVoltages();         
}

[roadrunner84 466]

what is "++10" or "--10"?  never seen that notation

 

usually it's += 10 or -= 10

 

also can you please indent the code correctly?  It's hard to ascertain what right-braces "}" corresponds to which loop the way it's formatted here.

fadeValue ++10;

should be interpreted as

fadeValue + +10;

or "sum fadeValue and +10".

Alas, the result of this equation is not assigned to anything and thus discarded. A compiler might even optimize it away. So, it is valid syntax, but totally not what it's meant to do.

float voltage1, voltage2, voltage3;

voltage1 = analogRead(voltPin);
voltage2 = (voltage1 / 1024) * 3.3;
voltage3 = voltage2 * 5.65; 

// could better be

voltage = analogRead(voltPin) / 1024.0 * 3.3 * 5.65;

// since now the compiler can pre-calculate 1024.0^-1 * 3.3 * 5.65,
// resulting in something like voltage = analogRead(voltPin) * 0.0182080078125;

// To make your code much more speedy, consider that
// 1/55 = 0.01818... which is very close to 0.0182080078125 or only 0.144% off
// So you could write voltage = analogRead(voltPin) / 55;
// This would suffer from integer rounding, but it might be acceptable.

int voltage;
voltage = analogRead(voltPin) / 55;

[roadrunner84 466]

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  //If the priority is high
  while(voltage >= 12)
  {
    fadeValue = constrain(fadeValue, 0, 255);  
    fadeValue2 = constrain(fadeValue2, 0, 255); 
    voltage = analogRead(voltPin) / 55.0;
    fadeValue = fadeValue + 10;  
    fadeValue2 = fadeValue2 - 10;
    analogWrite(voltoutPin, fadeValue);
    analogWrite(voltoutPin2, fadeValue2);
  }

  // If priority is low
  while(voltage < 12)
  {
    fadeValue = constrain(fadeValue, 0, 255);  
    fadeValue2 = constrain(fadeValue2, 0, 255);   
    voltage = analogRead(voltPin) / 55.0;
    fadeValue = fadeValue - 10;
    fadeValue2 = fadeValue2 + 10;
    analogWrite(voltoutPin2, fadeValue2);
    analogWrite(voltoutPin, fadeValue);
  }
}

I cleaned up the spacing in your code, as you can see, the only difference between the two blocks is the sign of the changes to fadeValue and fadeValue2. You could consider moving the rest of the code to a common place. Or, to move the constrain() call into the update of fadeValue(2).

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  //If the priority is high
  while(voltage >= 12)
  { 
    voltage = analogRead(voltPin) / 55.0;
    fadeValue = constrain(fadeValue + 10, 0, 255);
    fadeValue2 = constrain(fadeValue2 - 10, 0, 255);
    analogWrite(voltoutPin, fadeValue);
    analogWrite(voltoutPin2, fadeValue2);
  }

  // If priority is low
  while(voltage < 12)
  {
    voltage = analogRead(voltPin) / 55.0;
    fadeValue = constrain(fadeValue - 10, 0, 255);
    fadeValue2 = constrain(fadeValue2 + 10, 0, 255);
    analogWrite(voltoutPin, fadeValue);
    analogWrite(voltoutPin2, fadeValue2);
  }
}

Also, I guess you're trying make some kind of fade in/out function, which is not what you're really doing. The while loops will be so fast that there is no real fading. The gross result will be that the outputs will drift to some mean point, or maybe oscillate a little around that value at a very high frequency. I assumed in this that voltoutPin and voltPin are tied together, an analog lagging filter would of course result in some kind of slower sinoid signal.

It triggers me that your two cases seem to be exactly opposite in behavior. You might consider removing those while loops alltogether, since loop() is a loop itself.

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  if(voltage >= 12) // high priority
  { 
    fadeValue = constrain(fadeValue + 10, 0, 255);
    fadeValue2 = constrain(fadeValue2 - 10, 0, 255);
  }
  else // low priority
  {
    fadeValue = constrain(fadeValue - 10, 0, 255);
    fadeValue2 = constrain(fadeValue2 + 10, 0, 255);
  }
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, fadeValue2);
}

Then, in setup() you set either fadeValue or fadeValue2 to 255, the other to 0. Why not do that calculation on the spot, since you'd always have the sum of fadeValue and fadeValue2 be 255.

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  if(voltage >= 12) // high priority
    fadeValue = constrain(fadeValue + 10, 0, 255);
  else // low priority
    fadeValue = constrain(fadeValue - 10, 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

And lastly, I'd like to introduce you to the ternary operator, use it with care, also it might make your code a little more obscure.

The ternary operator is written like this

(condition) ? (true-value) : (false-value)

In contrast to the if() ... else ... construction, the ternary operator returns a value, it is not intended to run statements. Example:

food = (animal == monkey) ? banana : grass;

so if the animal variable is monkey, food becomes banana, otherwise it becomes grass.

Another example, now with real values

abs_val = (val > 0) ? val : -val;

This will check is val is positive, then abs_val becomes that val, otherwise abs_val becomes the negative of val, and the negative of a non-positive number is a positive number itself. So now we have abs_val containing the absolute value of val.

 

In your code, you either add 10 or subtract 10 depending on a condition (voltage >= 12), adding -10 is the same as subtracting 10, so we could either add 10 or -10 depending on your condition:

fadeValue = constrain(fadeValue + ((voltage >= 12) ? 10 : -10), 0, 255);

Note that for readabilities sake I put the ternary operator between braces, though that's not necessary.

Your code would become:

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  fadeValue = constrain(fadeValue + (voltage >= 12 ? 10 : -10), 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

You see that the code might become a little harder to read, also it does not speed your code up; the compiler will do something very similar to what it already did.

Note that voltage is now only used once, so you could slide that in too, but your code would become even more obscure and it would not speed anything up either.

A little more readable:

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  fadeValue += (voltage >= 12 ? 10 : -10);
  fadeValue = constrain(fadeValue, 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

[abecedarian 330]

Like the OP code, your code's fadeValue and fadeValue2 do not have any value assigned before entering loop() and being accessed in constrain() functions so are undefined.

 

 

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin) / 55.0;

  //If the priority is high
  while(voltage >= 12)
  {
    fadeValue = constrain(fadeValue, 0, 255);  
    fadeValue2 = constrain(fadeValue2, 0, 255); 
    voltage = analogRead(voltPin) / 55.0;
    fadeValue = fadeValue + 10;  
    fadeValue2 = fadeValue2 - 10;
    analogWrite(voltoutPin, fadeValue);
    analogWrite(voltoutPin2, fadeValue2);
  }

  // If priority is low
  while(voltage < 12)
  {
    fadeValue = constrain(fadeValue, 0, 255);  
    fadeValue2 = constrain(fadeValue2, 0, 255);   
    voltage = analogRead(voltPin) / 55.0;
    fadeValue = fadeValue - 10;
    fadeValue2 = fadeValue2 + 10;
    analogWrite(voltoutPin2, fadeValue2);
    analogWrite(voltoutPin, fadeValue);
  }
}

 

[roadrunner84 466]

Like the OP code, your code's fadeValue and fadeValue2 do not have any value assigned before entering loop() and being accessed in constrain() functions so are undefined.

 

That is not my code, that's OP's code, but with nice indentation

As I said directly below that code block:

 

I cleaned up the spacing in your code

Eventually the values of OP's code would settle to 0 and 255 or mean out to anything that's a sum of those. Since I later optimize fadeValue2 away, the only thing undefined is fadeValue, but that's limited to the 0 to 255 range because of the constrain() call. This means the only thing undefined is where in the settling/oscillation loop the sketch begins, which is okay-ish.

 

A little more speedy, based on my latest code example.

As you know you can haul values to the other side of the equation

a / b = c

a = c * b

You can do this too to the voltage variable, allowing for you to get rid of the float variable alltogether:

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin);

  fadeValue += (voltage >= 12 * 55.0 ? 10 : -10);
  fadeValue = constrain(fadeValue, 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

See how I moved the / 55.0 to the other side of the equation in the ternary operator.

Now since voltage is always an integer value (analogRead gives us an integer value), we can know for sure that the float behaviour is not required:

int voltage;

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin);

  fadeValue += (voltage >= 12 * 55 ? 10 : -10);
  fadeValue = constrain(fadeValue, 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

see how I changes 55.0 to 55, no float constants, so no float calculations.

Now consider that 55 is a bit magic, (as is 12), consider moving those to constants.

#define Threshold (12)
#define VoltScale (55) /* 5.65 times the voltage at the analog pin ( 3.3 * 5.65 / 1024) */
int voltage;

void loop()
{
  //Obtain RAW voltage data
  voltage = analogRead(voltPin);

  fadeValue += (voltage >= Threshold * VoltScale ? 10 : -10);
  fadeValue = constrain(fadeValue, 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

Well, now there is really no good use for the voltage temporary variable:

#define Threshold (12)
#define VoltScale (55) /* 5.65 times the voltage at the analog pin; 1/(3.3 * 5.65 / 1024) */

void loop()
{
  fadeValue += (analogRead(voltPin) >= Threshold * VoltScale ? 10 : -10);
  fadeValue = constrain(fadeValue, 0, 255);
  analogWrite(voltoutPin, fadeValue);
  analogWrite(voltoutPin2, 255 - fadeValue);
}

[Derekspeegle 0]

Wow, holy cow. Thank you guys for your amazing input. I have a lot to work through now, I will return and let you guys know how it all went. Thanks a ton. Do you guys think you could look over my other post, ADC with the MSP430? That one is still unanswered. Also, to answer spirilis's questions, I am using the mps430fr4133, can you elaborate more on the problem with analogwrite? I'm using it for another project which involves using analogwrite for rgb values and ive noticed the glitching. Is there a work around?

[abecedarian 330]

Wow, holy cow. Thank you guys for your amazing input. I have a lot to work through now, I will return and let you guys know how it all went. Thanks a ton. Do you guys think you could look over my other post, ADC with the MSP430? That one is still unanswered. Also, to answer spirilis's questions, I am using the mps430fr4133, can you elaborate more on the problem with analogwrite? I'm using it for another project which involves using analogwrite for rgb values and ive noticed the glitching. Is there a work around?

@@spirilis or @@energia are the ones most able to address the glitches.

[energia 485]

If you repeatedly call analogWrite() in a loop very fast, the PWM signal will not have the time to complete a full period @ 490 Hz. Hence, there is a chance you get garbage. Reading Arduino source code, the same is true for the avr implementation.

 

Robert