Use unsigned long from mills() in a division

Hello everyone.

I have the following code where I want to extract the unsigned long value from a millis() calculation and divide it by an integer:

unsigned long startTime;
unsigned long endTime;
unsigned long newDuration;
byte timerRunning;
unsigned long lastPosition = 0;

void calculate_new_last_position(){

  int A = 30000L;
  int B = 100L;
  int X = newDuration;

  int Y = (B*X)/A; //A simple rule of three calculation
  lastPosition = Y;
 
  Serial.println(String("Total Traveled Position: ") + (int)lastPosition + String(" %"));// here I get just 0, why?
}

setup(){...}

void loop() {

if (timerRunning == 0 && pressed){
  startTime = millis();
  timerRunning = 1;
  }
  if (timerRunning == 1 && !pressed){
  endTime = millis();
  timerRunning = 0;
  newDuration = (endTime - startTime);
  Serial.print (String("Total Traveled in ms: ") + newDuration + String(" ms"));// this value here prints like a charm
  }
}

If I do this on calculate_new_last_position() it works and it prints 50 to the Serial:

  int A = 30000L;
  int B = 100L;
  int X = 15000L;// an example number that could come from the loop

  int Y = (B*X)/A; //A simple rule of three calculation to get a percentage
  lastPosition = Y;
 
  Serial.println(String("Total Traveled Position: ") + (int)lastPosition + String(" %"));// here I get 50 that's correct

I am not a programmer just a hobbyist, so please, ELI5.

What am I doing wrong?

Thank you.

Use ‘type’ ‘unsigned long’ int Y

See:

VanaArdod:
What am I doing wrong?

Please post a complete program that illustrates the problem. I can't make sense of your snippets and I can't try them.

I would expect this code to work as expected

unsigned long result = millis() /  12345;

Separately, it is not a good idea to use the String (capital S) class on an Arduino as it can cause memory corruption in the small memory on an Arduino. This can happen after the program has been running perfectly for some time. Just use cstrings - char arrays terminated with '\0' (NULL).

...R

"What am i doing wrong ? " well you are trying to do a 32-bit calculation using a 16-bit variables in the function (?)
Also a function in this case should take parameters and return the result... really.. A & B as variable names ?

Okey here is all my code and a little more of background.

The project: I have an esp8266 (specifically a Sonoff Dual from Itead) with two relays controlling an external AC motor for the window cover, I have two tactile buttons connected to the headers to move the cover up or down accordingly, I've also integrated an MQTT client that reads and send messages from and to Home Assistant, in addition, I implemented a simple web server to control and monitor the status of the cover, so I basically can control the cover position from 3 instances, Home Assistant, the web server and the physical buttons.

The problem: I want to calculate the exact time that happens between a relay been active until it's stops, that will give me some milliseconds that I can use to calculate the actual percentage that the covers just moved, because home assistant will read percentages not milliseconds when communicating with the window cover.

I can already convert the percentage sent by Home Assistant to milliseconds, so I can move the cover from the HA interface, but I'm stuck converting the total milliseconds that any of the relay was active into percentage, that will give home assistant a feedback of how much the covers actually moved.

For instance: my cover takes 30 seconds to go up or down, in Home Assistant 0 means covers are closed and 100 means cover are opened, I calculated using a rule of three that gives me the exact time a cover has to move based on how much percent is sent by HA trough MQTT in order to reach the actual position, wich then will be converted to last position so I can calculate how much to move next time home assistant sends a percentage command.

At the beginning I was just setting the target position send by HA to be the latest position(lastPosition = targetPosition on my code), this works ok but then if I use the physical button I don't get the new las position synchronised, that's the reason I want to calculate the actual time the relays are active.

My full code is here on this link

I hope you can understand, thank you.

It's a lot of code, and i can not be sure, but you somehow indicated that this equation did not provide the expected result.unsigned long Y = (B*X)/A;
Now i am not sure, but i think first B and X are multiplied, then divided by A, all using 16-bit calculation, and then put into a 32-bit variable. try casting B or X as such

unsigned long Y = ((unsigned long) B*X)/A;

casting in Arduino C++ is much more sensitive than in normal C++, and also the ESP core uses different a different math library than an Arduino does. but if i understood the documentation correctly, once any of the variables used in a calculation is 32-bit a 32-bit calculation is used (as for 16 and for float). The result is not actually considered to be part of the calculation.

casting in Arduino C++ is much more sensitive than in normal C++

What do you mean?

Deva_Rishi:
It's a lot of code, and i can not be sure, but you somehow indicated that this equation did not provide the expected result.

unsigned long Y = (B*X)/A;

Now i am not sure, but i think first B and X are multiplied, then divided by A, all using 16-bit calculation, and then put into a 32-bit variable. try casting B or X as such

unsigned long Y = ((unsigned long) B*X)/A;

casting in Arduino C++ is much more sensitive than in normal C++, and also the ESP core uses different a different math library than an Arduino does. but if i understood the documentation correctly, once any of the variables used in a calculation is 32-bit a 32-bit calculation is used (as for 16 and for float). The result is not actually considered to be part of the calculation.

Thank you, but sadly it still gives me 0 on the serial instead of the desired percentage.

This is how my serial looks like

The last position should be the same as Target position, but is not happening.

Rather than show just the output, why not let us see the code that produced it?

AWOL:
Rather than show just the output, why not let us see the code that produced it?

I did in the previous post

My full code is here on this link

If you don't see the Link copy and paste this URL:
https://github.com/valerie16901/SonoffDual-Home-Assistant-MQTT-Cover-Firmware/blob/master/firmware/firmware.ino#L151-L177

The problematic line is around line 177

Please post code here if you me (and many others) to look at it.

If you are just having a problem with division can't you just write a simple 10 or 20 line program to illustrate it?

...R

VanaArdod:

  int A = 30000L;

int B = 100L;
 int X = 15000L;// an example number that could come from the loop

int Y = (B*X)/A;



I am not a programmer just a hobbyist, so please, ELI5.
What am I doing wrong?

(100 * 15000) / 30000 == 50. If the answer you get is 50, what makes you think you are doing something wrong?
On most Arduinos, like an UNO, this would not work because the math would get truncated to 'int'. A 16- bit 'int' can't hold the value 1,500,000 (result of B*X). The largest number a 16-bit signed 'int' can hold is 32,767.
Perhaps you are using a 32-bit processor where an 'int' is 32 bits long.

Robin2:
Please post code here if you me (and many others) to look at it.

If you are just having a problem with division can't you just write a simple 10 or 20 line program to illustrate it?

...R

That's what I did on the first post, but then you said this

Robin2:
Please post a complete program that illustrates the problem. I can't make sense of your snippets and I can't try them.

Instead of this:

int A = 30000L;
int B = 100L;
int X = newDuration;
unsigned long Y = ((unsigned long) B*X)/A;

Try this:

unsigned long A = 30000UL;
unsigned long B = 100UL;
unsigned long X = newDuration;
unsigned long Y = (B*X)/A;

This should return the desired value. If A, B and X need to be integers, the can be cast to unsigned longs in the calculation like this:

int A = 30000;
int B = 100;
int X = newDuration;
unsigned long Y = ( (unsigned long)B * (unsigned long)X ) / (unsigned long)A;

johnwasser:
(100 * 15000) / 30000 == 50. If the answer you get is 50, what makes you think you are doing something wrong?
On most Arduinos, like an UNO, this would not work because the math would get truncated to 'int'. A 16- bit 'int' can't hold the value 1,500,000 (result of B*X). The largest number a 16-bit signed 'int' can hold is 32,767.
Perhaps you are using a 32-bit processor where an 'int' is 32 bits long.

I am not getting 50, I am getting 0 because the variable I want to use comes from the loop in wich I am calculating an elapsed time that I convert to the unsigned long newDuration

Why add a fudge factor of '-99' in line 307? Things will go funny if 'newDuration' is negative and you do your calcuations in 'unsigned'? A small negative number looks like a very large positive number when you store it in an 'unsigned' variable.

  newDuration = (endTime - startTime) - 99;

johnwasser:
Why add a fudge factor of '-99' in line 307? Things will go funny if 'newDuration' is negative and you do your calcuations in 'unsigned'? A small negative number looks like a very large positive number when you store it in an 'unsigned' variable.

  newDuration = (endTime - startTime) - 99;

That minus is just to compensate a delay I have in other part of the code, the result is the same with or without the minus there.

WattsThat:
Instead of this:

int A = 30000L;
int B = 100L;
int X = newDuration;
unsigned long Y = ((unsigned long) B*X)/A;

Try this:

unsigned long A = 30000UL;
unsigned long B = 100UL;
unsigned long X = newDuration;
unsigned long Y = (B*X)/A;

This should return the desired value. If A, B and X need to be integers, the can be cast to unsigned longs in the calculation like this:

int A = 30000;
int B = 100;
int X = newDuration;
unsigned long Y = ( (unsigned long)B * (unsigned long)X ) / (unsigned long)A;

Thank you, I tried both ways and none of them seems to be working, I am about to go crazy with this error.

This is what I changed:

void calculateLastPosition(){
  unsigned long A = 30000UL;
  unsigned long B = 100UL;
  unsigned long X = newDuration;
  unsigned long Y = (B*X)/A;
  Serial.println(String("Last Position: ") + (unsigned long)Y + String(" %"));
}

and

void calculateLastPosition(){
  int A = 30000;
  int B = 100;
  int X = newDuration;
  unsigned long Y = ( (unsigned long)B * (unsigned long)X ) / (unsigned long)A;
  Serial.println(String("Last Position: ") + (unsigned long)Y + String(" %"));
}

but nothing, I still get 0 on the terminal :confused:

First of all, since you're using an ESP8266, you can forget about all of the comments about "int" vs "long" - on an ESP8266, int and long are both 32bits.

client.setCallback(callback);

I suspect that since you're using a callback function, which is sort-of like an interrupt, that you need to make newDuration into a "volatile":

volatile unsigned long newDuration;

What's probably happening is that the compiler notices that newDuration is initialized to 0, and there is no code path that ever sets it, it will always be 0, so there's no need to do that actual calculation at runtime.
This could apply to any other variables modified in the callback and uses elsewhere as well.