-2

I'm debugging a "big" program in Arduino. At some point it started to throw strange results, so I followed it to an operation that shouldn't be giving negative numbers. The result should be 248,625 and as it is stored in an int variable (giving me the same -79). I've already tried casting to int and applying ceil, floor, without success. Any help will be appreciated.

Debug prints:

B -> (255*195)/200+0 = -79

The code:

// the setup routine runs once when you press reset:
void setup() {
  Serial.begin(115200);
}

// the loop routine runs over and over again forever:
void loop() {
  int _num_steps = 200;
  int _current_step = 195;
  int _delta_B = 255;
  int currentValues_B = 0;

  Serial.print(" B -> ");
  Serial.print("(");
  Serial.print(_delta_B);
  Serial.print("*");
  Serial.print(_current_step);
  Serial.print(")/");
  Serial.print(_num_steps);
  Serial.print("+");
  Serial.print(currentValues_B);
  Serial.print(" = ");
  Serial.println(((_delta_B*_current_step) / _num_steps) + currentValues_B);
  while(true){}
}
wablab
  • 1,703
  • 13
  • 15
Sergio
  • 1,383
  • 2
  • 13
  • 28
  • 8
    Arduino `int` is 16-bit, I believe, so the maximum value you can hold in it is 32767. (Assuming you're talking about the Uno or Mega: https://www.arduino.cc/en/Reference/Int) – wablab Sep 01 '17 at 18:50
  • 3
    you may want to look up the term "integer overflow". The short version is that once an integer goes above its maximum value, it loops back to it's lowest possible value, in the case of a 16-bit integer, that means that `32767+1=-32768` – Zinki Sep 01 '17 at 18:52
  • Try to use long int. It has a minimum size of 32 bits, and can hold the value without overflow. – rbelli Sep 01 '17 at 18:53
  • 1
    It would be more civilized to express the point for down voting when doing so. If not it may seem that there's just only penalization for choosing one answer instead of their own. – Sergio Sep 01 '17 at 19:33
  • Even when there seems to be an overflow, it is not as apparent as you seem to assume. If you put some ints into an operation and **the result it's assured to be between 0 and 255**, the intermediate steps in the calculations shouldn't be relevant. At least, I've never faced this in more than 20 years. So the cause is not using ints, but some other like internal use of pointers or something. – Sergio Sep 01 '17 at 19:37
  • 1
    @Sergio sadly you can't really look for civilized anything in the c++ tag on stack overflow. Just a bunch of people that link the standard in comments and downvote your question. – Josh Sep 01 '17 at 19:41
  • @Josh May be. I chosen that question because it was the one having reference, quote and explanation, so It pointed my problem. I never thought it should be only arduino related because I tested it on C++ before posting with same result. And I solved it doing an internal cast to long. Not using equations or changing types. ¿? The result was 7. – Sergio Sep 01 '17 at 19:48
  • 1
    @Sergio its not about internal pointers. Its just the intermediate multiplication step results in 49725, which is a 16 bit integer overflow. Here is an example: https://stackoverflow.com/q/12488522/128581 – Josh Sep 01 '17 at 19:48
  • Yes, the first multiplication results in 49725. But in my limited logic (coming from c#) that shouldn't be treated as an int or evaluated. Just before is divided by 200 and the result fits in the target int variable. That's why I think it may be doing something similar than when you aggregate char* – Sergio Sep 01 '17 at 19:53
  • @Josh Yes, indeed the link you provide is right. (even when says nothing about Arduinos) But I may understand the cause. Thank you very much, Josh. – Sergio Sep 01 '17 at 20:00
  • 1
    The downvote button says *"This question does not show any research effort; it is unclear or not useful"*. I just clicked it because spending a little effort searching online (or even on stack-overflow) would have yielded you dozens of possible duplicates to your question, e.g. [this](https://stackoverflow.com/questions/34545445/positive-integers-that-multiply-to-a-negative-value), and [this](https://stackoverflow.com/questions/20594628/arduino-arithmetic-error-negative-result) or [this](https://stackoverflow.com/questions/19642667/why-do-i-get-a-negative-number-by-multiplying-two-short-ints). – Patrick Trentin Sep 01 '17 at 22:18
  • 1
    Also, *Arduino* documentation warns about this precise issue, e.g. [Arduino - Arithmetic](https://www.arduino.cc/en/Reference/Arithmetic). Should we conclude that you did not read the documentation before asking for help? I cannot speak with certainty, but I believe other people downvoted your question for the same reason. – Patrick Trentin Sep 01 '17 at 22:18
  • Other possible reasons for downvotes that I can think of, based on my experience of being downvoted: 1. The infinite loop at the end, which serves no purpose and does not allow the `void loop` to wrap around as it should 2. The apparent lack of a few lines of source code that should supposedly be there to handle `_current_step` and possible related wrap-around issues on this variable, which however cannot be gauged on the source code as it is. These are nitpicks *imho*, but some people are easily triggered.. :) – Patrick Trentin Sep 01 '17 at 23:13
  • @PatrickTrentin Dear, thanks for your clarification. I must say that you're not right. You are making assumptions and taking action on them. You don't know nothing about the size or architecture of this program. I had to make three smaller ones to isolate where was the problem. And make a sample specially for posting. Before that, I spent more than a day trying to solve, and researching. Perhaps not being english speaking may be a problem obtaining useful results. Perhaps writing this takes me one hour. – Sergio Sep 04 '17 at 10:07
  • I made no assumptions, the same issue was outlined by over a dozen people. – Patrick Trentin Sep 04 '17 at 10:09
  • If an infinite loop in a small sample is a problem, maybe you're rating my programming capabilities. And finally, the cause and the fix, even when produced an overflow was not related to the capabilities of the mcu or types chosen. It was an implicit cast made by the compiler that was solved putting an in step explicit. I didn't had to change anything in code or architecture. That's the link you posted, yes I read it. And Josh made a point to think on it. (clear enough for me) – Sergio Sep 04 '17 at 10:14
  • Maybe you should read again what I wrote, because you clearly misunderstood that part. You don't need to explain me the issue, it's very clear. But thanks, and best regards. :) – Patrick Trentin Sep 04 '17 at 10:45

4 Answers4

3

https://www.arduino.cc/en/Reference/Int

On the Arduino Uno (and other ATMega based boards) an int stores a 16-bit (2-byte) value. This yields a range of -32,768 to 32,767 (minimum value of -2^15 and a maximum value of (2^15) - 1).

You can try long (which on Arduino is 4-bytes). I don't necessarily see what you're intending to do, but this can't grow forever either. You need to handle a case where you wrap (or reset the interval or whatever) if they can grow forever. Further if the value is not supposed to ever go negative, you can use an unsigned primitive datatype and double its positive range.

Josh
  • 12,602
  • 2
  • 41
  • 47
  • This is only a basic operation required to fade a led from 0 to 255 value in this case. There are steps in light changes and delta is the difference. in this case from 0 to 255. I'm really suprised I'm getting into this. Since the result of the operation must be between 0-255 I still don't see why I have problems. If I cast one operand to long it should then do the operation and then I should be forced to recast to int. Isn't it? – Sergio Sep 01 '17 at 19:16
  • @Sergio consider using [Map](https://www.arduino.cc/en/Reference/Map), an *Arduino* function that is designed exactly for achieving that purpose without incurring in any numerical error. It also makes the code more readable. – Patrick Trentin Sep 01 '17 at 22:06
  • @PatrickTrentin its clear from comments on the main question that this is a simple int overflow. It doesn't continue to grow, so map is inappropriate in this case. – Josh Sep 02 '17 at 00:13
  • Pray excuses, but how exactly does it not apply? He wants to map a value from the interval 0-200 into the interval 0-255, plus possibly an offset, that's exactly what function map is there for. – Patrick Trentin Sep 02 '17 at 03:44
2

My guess is that your device is running with 16 bit integers. If we do the math with 16 bit signed integers and overflow:

255 * 195 = -15810   Overflow math:  (-32768 + (49725 - 32767))
-15810 / 200 = -79

You can verify the size of your integers by printing a sizeof(int)

Dan
  • 858
  • 7
  • 18
1

As other users pointed out in the comments, it is an overflow problem. Given that "int", "long", etc have by definition only a fixed minimum number of bits (and int minimum number is 16), you have this problem. Probably you never met this problem before, because nowadays sometimes int is implemented with 32 bits.

To know for sure every single time the number of bits your are using for your int types, you can use something like this:

typedef int32_t myint;
typedef u_int32_t myuint;

and then use only myint. This way you know exactly the number of bits used and if you ever decide to change the implementation according to some new needs you can change it in a single place.

DrHell
  • 461
  • 3
  • 17
1

Since ints are represented using 16 bits n Arduino, you'll have to fall back on basic math to get the answer you need.

You are trying to compute (A*B)/C.

(A*B)/C = ((k1*C+a) * (k2*C+b)/C, where
k1 = A/C and a = A%C, and k2 = B/C and b = B%C.

(k1*C+a) * (k2*C+b) is equal to (k1*k2*C*C + b*k1*C + a*k2*C + a*b)

If you divide that by C, you get k1*k2*C + b*k1 + a*k2 + (a*b)/C.

That number should stay within the limits of a 16-bit number.

A sample program that demonstrates the idea on a 64-bit machine:

#include <iostream>

int main()
{
   int A = 255;
   int B = 195;
   int C = 200;

   int k1 = A/C;
   int a = A%C;
   int k2 = B/C;
   int b = B%C;

   std::cout << k1*k2*C << std::endl;
   std::cout << k1*b << std::endl;
   std::cout << k2*a << std::endl;
   std::cout << a*b << std::endl;

   int sum = k1*k2*C + k1*b + k2*a + (a*b)/C;
   std::cout << sum << std::endl;
   std::cout << (A*B)/C << std::endl;
}

Output:

0
195
0
10725
248
248
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • @PatrickTrentin, the difference is that `A*B` exceeds the limits of a 16 bit integer while the other terms won't. – R Sahu Sep 02 '17 at 05:36
  • @PatrickTrentin, then I am not explaining why my suggestion should work clearly. Take the numbers from the OP. Compute each term in the expression I suggested. They will remain within the limits of a 16 bit integer. – R Sahu Sep 02 '17 at 05:39
  • Oh, now that's clear. +1 :) -- I didn't get at first you were proposing a different expression for the whole computation. – Patrick Trentin Sep 02 '17 at 05:40
  • > "Since sizeof(int) is 16 in Arduino" is not perfectly correct ;) sizeof returns the number of bytes, not bits. – datafiddler Sep 05 '17 at 11:52