arithmetic operation outcome differs from the expected outcome

Question

I am writing a function

void Sound_Play(int frequency,int millisec)

A part of this function is:

 counting=(millisec*frequency/1000); //counting is type int
 Serial.println((String)"millisec="+millisec+" frequency="+frequency+" (millisec*frequency/1000)="+(millisec*frequency/1000)+" counting set to="+counting);

But when I call the function with millisec=500 and frequency=220, the result on the Serial Monitor is:

millisec=500 frequency=220 (millisec*frequency/1000)=-21 counting set to=-21

Shouldn't counting should be 110 instead.I tried casting the result to int but in vain. What am I doing wrong and how to correct it? Thanks! Bhuvnesh

Answer 1

Once again an example to Why should I learn C/C++ first before learning Arduino.
The main point here is: know your data types.

Your calculation millisec*frequency/1000 would work if it was a compile time constant that is evaluated by the pre processor. In any other case this is a runtime value and so the limitations of the datatype apply.

Unless stated otherwise I assume your variables millisec, frequency and counting are of type int. int has a platform specific width. On Arduino this is 16-bit. (Other platforms have other widths)

It is also signed, that is why it can hold values between -32768 and 32767. You calculation millisec*frequency evaluates to 500 * 220 = 110000 which is many times larger than the value that can be represented.

You can do two things:

• Use long which is 32-bit on Arduino. But actually I'd prefer to use a type with an obvious width. As you don't seem to need negative values I'd prefer uint32_t.

• If milliseconds is a constant, you can make it smarter by reducing your calculation to a division by two (or better a single right shift).

• Bhuvnesh: Thankyou for your Answer. I was a bit distracted could not see the fact that you mentioned. Changing millisec and frequency to type long solved the issue
• chrisl: You forgot the negative sign in the range for int. Otherwise great answer. Upvote