Variable typing question (SOLVED)

I’m having what I’m sure is an obvious problem with variable typing, but I can’t sort it out. And because the keywords in question are things like, “long”, “float”, and “divide”, it’s very difficult to search for the problem I’m having, because of so many results.

Here’s a relevant code snippet:

long arcMillis;
long windowMillis;

// these lines always produce 0 in workCycle
// float workCycle = arcMillis / windowMillis;
// float workCycle = (float)arcMillis / (float)windowMillis;

// but this works...
float x = arcMillis;
float y = windowMillis;
float workCycle = x / y;

Here’s another example of what might be the same fundamental lack of knowledge on my part:

int arcMinutes = 0;
long arcMillis = 0;

// this line produces divide by zero error
arcMinutes = arcMillis / 60 / 1000;

I realize that I may be using sloppy technique by relying on the system to convert the (long)arcMillis to an (int)arcMinutes, versus doing the rounding myself, but I wouldn’t think a divide-by-zero error should result. I’m dividing by 60 and 100, which are definitively NOT zero.

I am using Virtronics simulator for Arduino Pro version 0.98G if that’s relevant. I realize there’s a small chance this is a bug in the simulator, but I don’t actually have an Arduino board to test with yet, and I think it’s far more likely that I’m missing some nuance of variable typing that is causing me problems.

The problem is, the compiler sees integer operands, so assumes the arithmetic must be integer too. If the divisor is greater than the dividend, then the result will be zero, because an integer cannot represent fractions. Try casting one or both operands to "float"

float workCycle = (float)arcMillis / (float)windowMillis;

I can't see anything wrong with this.

AWOL: The problem is, the compiler sees integer operands, so assumes the arithmetic must be integer too. If the divisor is greater than the dividend, then the result will be zero, because an integer cannot represent fractions. Try casting one or both operands to "float"

I see. So it does 60 / 1000 first, gets zero, and then tries to divide by that. Got it. Order of operations. Thanks.

float workCycle = (float)arcMillis / (float)windowMillis;

I can't see anything wrong with this.

Ok. Thanks for the confirmation. I will have to try it out on my actual Arduino board once it comes in the mail. For now, I will curse at my emulator.

Yes. This worked:

arcMinutes = (float)arcMillis / 60.0 / 1000.0;

Thanks very much.

// this line produces divide by zero error
arcMinutes = arcMillis / 60 / 1000;

What error message exactly? From what software?

And you haven't posted all your code so no-one else can try to recreate the problem...

BTW in C/C++ those divisions are required to execute left-to-right because division is not associative. (a/b)/c != a/(b/c)

MarkT:

// this line produces divide by zero error

arcMinutes = arcMillis / 60 / 1000;




What error message exactly? From what software?

I am using Virtronics simulator for Arduino Pro version 0.98G.

And you haven’t posted all your code so no-one else can try to recreate the problem…

BTW in C/C++ those divisions are required to execute left-to-right because division
is not associative. (a/b)/c != a/(b/c)

I would have thought so, but then I can’t understand why I would get a divide-by-zero error on that operation. I’ll be glad to post the entirety of the code, but I didn’t want to just blurt it up like that if the question could be addressed more concisely. Here it is:

const int greenLED = 13; // green LED's output number
const int redLED = 12; // red LED's output number
const int windowMinutes = 10; // number of minutes in the duty cycle "window"
const int loopDelay = 250; // ms delay between loops

// Variables related to time-keeping

long previousMillis; // milliseconds at previous loop
long triggerMillis = -1; // time at which arc first triggered window to start, -1 means no trigger
long arcMillis = 0; // number of milliseconds that the arc has been active within this window
int windowExpireMinutes = windowMinutes; // time until current window expires
int arcMinutes = 0; // minutes of arc active during this window

// Variables related to detecting the arc

int sensorRead; // holds readings from analog sensors
float arcVolts; // voltage from the welding leads
int welderOCV = 5; // nominal OCV of the welder, relative to 5 volts
float arcOCVpercent = 0.90; // percent of OCV that indicates arc is active

// Variables related to calculating the duty cycle

long windowMillis = windowMinutes * 60 * 1000;  // number of milliseconds in the duty cycle "window"
float dutyCycle = 0.01; // duty cycle of the welder

void setup() {
  pinMode(greenLED, OUTPUT);
  pinMode(redLED, OUTPUT);
  digitalWrite(redLED, LOW);
  digitalWrite(greenLED, HIGH);

  // this will immediately get overwritten at the first loop, but we have to set it to something
  previousMillis = millis();
}

// Current method of detecting presence of arc is to measure voltage between welding leads.
// OCV (no arc) will be higher than arc voltage. This method may prove to be less than ideal,
// in which case some other method will be implemented.

boolean checkArc() {
  sensorRead = analogRead(A0);   // read input from analog pin A0
  arcVolts = sensorRead * (5.0 / 1023.0); // convert sensor read to volts on 5 volt scale
  return arcVolts < (welderOCV * arcOCVpercent);
}

void loop() {
  long currentMillis = millis(); // note the time at the beginning of the loop, for consistent reference

  // stuff to do only if the arc is active during this loop
  if (checkArc()) {
    if (triggerMillis < 0) { // if this is the first time since window expiration that the arc has triggered the window
      triggerMillis = currentMillis; // note the time the window was triggered
    } 
    else { // we are in the middle of a window and the arc is active
      arcMillis += currentMillis - previousMillis; // add to the cumulative arc time

      // no idea why float workCycle = (float)arcMillis / (float)windowMillis always produces 0

      float x = arcMillis;
      float y = windowMillis;
      float workCycle = x / y;

      // What to do when duty cycle has been exceeded

      if (workCycle > dutyCycle) {

        // set LEDs. This will be redundant for most loops, but I bet it's not a worse
        // penalty than implementing logic to only do it the first time

          digitalWrite(redLED, HIGH); // turn on the red LED -- stop welding!
        digitalWrite(greenLED, LOW); // turn off the green LED

        // If the arc continues to be active after duty cycle has been exceeded
        // then add the additional arc-active time to the current window. For example,
        // if the welder has a 20% duty cycle (2 minutes), but the operator welds for 
        // 3 minutes, the total window time will be extended to 11 minutes, rather than
        // 10. This is implemented by simply adding one loopDelay to the trigger time
        // for each loop during which the arc is active after duty cycle has been
        // exceeded.

        triggerMillis += loopDelay;
      }
    }
  } 

  if ((currentMillis - triggerMillis) > windowMillis) { // if window has expired
    triggerMillis = -1; // reset the triggered state
    arcMillis = 0; // reset the cumulative arc time   
    digitalWrite(redLED, LOW); // turn off the red LED (if it was off, no change)
    digitalWrite(greenLED, HIGH); // turn on the green LED -- good to weld!
  } 

  // check status of "reset" button

  // check status of "calibrate" button -- assumes OCV method is used to detect arc

  // check status of "duty cycle" pot 

  arcMinutes = (float)arcMillis / 60.0 / 1000.0;
  windowExpireMinutes = (currentMillis - triggerMillis) / 60.0 / 1000.0;

  delay(loopDelay); // delay -- must make sure each loop is > 1 ms at least to allow correct accumulation of arcMillis
  previousMillis = currentMillis; // set previousMillis for the next run of the loop
}

MarkT:

BTW in C/C++ those divisions are required to execute left-to-right because division is not associative. (a/b)/c != a/(b/c)

I thought division [u]is[/u] left-to-right associative.

econjack: MarkT:

BTW in C/C++ those divisions are required to execute left-to-right because division is not associative. (a/b)/c != a/(b/c)

I thought division [u]is[/u] left-to-right associative.

(100 / 10) / 2 == 5 100 / (10 / 2) == 20

Precedence and associativity are not the same thing:

(100 / 10) / 2 == 5 100 / (10 / 2) == 20

The parentheses alters the precedence order. In the absence of parentheses when all operators are of the same precedence level, expression parsing is dictated by the associativity rules. Most operators are left-to-right associative, including division.

100 / 10 / 2 = 5 100 / 2 / 10 = 5

void setup()
{
  Serial.begin(9600);
  int arcMinutes = 0;
  long arcMillis = 0;

  // this line produces divide by zero error
  arcMinutes = arcMillis / 60 / 1000;
  Serial.println(arcMinutes);
  arcMinutes = arcMillis / 0;
  Serial.println(arcMinutes);
}

void loop()
{
}

just prints 0 -1 , no error message at all ...???

robtillaart: just prints 0 -1 , no error message at all ...???

Well, thanks. I guess this means there is an issue in the simulator. Dang it.

LOL … simulator error :wink:

Simulator error indeed. I have since gotten my board in the mail, and as expected, the code works fine. I have a support inquiry in with Virtronics, and if they can’t give me a satisfactory answer, I’m going to request a refund. Honestly, I like looking at variable watchpoints better than watching a serial window scroll by, but if the code doesn’t compile right, the simulator is worthless.

long arcMillis;
long windowMillis;

millis() returns an unsigned long.

Eventually this will byte you.

[quote author=James C4S link=topic=181234.msg1348608#msg1348608 date=1376099332] millis() returns an unsigned long. Eventually this will byte you. [/quote]

I think what you're saying is that if I declare it as long (implicitly signed), I'm losing significant digits relative to an unsigned long, and after so many days, I will run into problems. Is that right?

Interestingly, I took my example from the arduino.cc examples, so somebody should tell them the same. I don't see any down-side to declaring as unsigned long, so not sure why they did it that way. Actually, upon double-checking, I see they do declare the current time as unsigned long, but they declare previousMillis as long. I'm sure there's a good reason that I'm just too dense to understand right now.

// constants won't change. Used here to 
// set pin numbers:
const int ledPin =  13;      // the number of the LED pin

// Variables will change:
int ledState = LOW;             // ledState used to set the LED
long previousMillis = 0;        // will store last time LED was updated

// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long interval = 1000;           // interval at which to blink (milliseconds)

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);      
}

void loop()
{
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the 
  // difference between the current time and last time you blinked 
  // the LED is bigger than the interval at which you want to 
  // blink the LED.
  unsigned long currentMillis = millis();

  if(currentMillis - previousMillis > interval) {
    // save the last time you blinked the LED 
    previousMillis = currentMillis;   

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW)
      ledState = HIGH;
    else
      ledState = LOW;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

joshuabardwell:
I don’t see any down-side to declaring as unsigned long, so not sure why they did it that way.

Not everything on the internet is correct. This is especially true of the examples on the Arduino site, unfortunately.