Division Problem

m I being a complete lemon here and missing something staring me in the face.

In my code I divide a variable by 100 and always return 1. The variable in question is servoPosition.

class hController {
public:
  hController();
  void servoControl(byte servoPercentage);
private:
  int servoPosition;
};

void hController::servoControl(byte servoPercentage) {
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage; // OK
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;
  Serial.print("after div=");
  Serial.println(servoPosition,DEC);
  analogWrite(hPin,(int)servoPosition);
}

Example screen dump


81
20655
after div=1

82
20910
after div=1

Read it wrong, sorry.

Think about the type of your variable - int(eger). And you want to have it store 0.8?
Float(ing point) is what you want, assuming that fractions are necessary (they are very, very slow to compute)

Have you try:
servoPosition = servoPosition / 100; ?

I’m missing it too, if that’s any consolation. Are you absolutely positive that output came from that source code? The four print statements you have don’t seem to correspond to the output you have logged. Where did the “-------” come from? Perhaps your upload failed, or you uploaded to the wrong Arduino type or the wrong port or something. Change the output to prove beyond doubt that you’re running the code you think you are, before you go any further.

I can't reproduce that. Testing with:

class hController {
public:
 // hController();
  void servoControl(byte servoPercentage);
private:
  int servoPosition;
};

void hController::servoControl(byte servoPercentage) {
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage; // OK
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;
  Serial.print("after div=");
  Serial.println(servoPosition,DEC);
}

void setup ()
{
  Serial.begin (115200);
  hController foo;
  foo.servoControl (81);
}

void loop () {}

Results:

81
20655
after div=206

The ----- are at the end of the main while loop for visual easy of interpretation. I've only shown the class I'm having a problem with, and have all the Serial.print's in the class so there shouldn't be any conflicts. Ignore the two print's of 'time' related stuff. That was from another class that I've just finished debugging!

I'm running the Arduino Nano with ATMega328 and that is all set up fine. There is a reasonable amount of code and everything works fine bar this one division and it's doing me nuts lol.

Here is what I have:

class hController {
public:
  hController();
  void servoControl(byte servoPercentage);
private:
  int servoPosition;
};

void hController::servoControl(byte servoPercentage) {
  Serial.print("servoPercentage=");
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage; // OK
  Serial.print("servoPercentage*255=");
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;
  Serial.print("after div=");
  Serial.println(servoPosition,DEC);
  analogWrite(hPin,(int)servoPosition);
}

Producing this result: (snippet of a larger loop, hence the changing position)

servoPercentage=68
servoPosition*255=17350
after div=198
timeCycle=3877
timeDelay=-2877

servoPercentage=67
servoPosition*255=17094
after div=198
timeCycle=3886
timeDelay=-2886

servoPercentage=66
servoPosition*255=16838
after div=198
timeCycle=3895
timeDelay=-2895

servoPercentage=68
servoPosition*255=17350

Looks like division is not the only thing you're having problems with.

Lol yes good spot. My mental arithmetic didnt pick that up :cold_sweat: Only 10 out tho, soooo close!

This is quite frustrating and I'm grateful for any replies and suggestions. Cheers!

@Magician I did try a non-compound operator and it gave the same result :frowning:

Is anything else going on at the same time that could be modifying those global variables? You could try doing that arithmetic in local variables and see if that changes anything?

Right then, prepare yourselves for a mind explosion.

Here is my code using a local variable for servoPosition. Where I have put ‘<-------HERE’, if I do NOT put ‘int’ before the variable name, the next print states:

servoPercentageLocal=198

// H2 Flow Control
class hController {
public:
  hController();
  void servoControl(int servoPercentage);
private:
  int servoPercentageLocal;
  int servoPosition;
};

void hController::servoControl(int servoPercentage) {

 int servoPercentageLocal = 10;  <--------HERE
  
  Serial.print("servoPercentageLocal=");
  Serial.println(servoPercentageLocal,DEC);

  servoPosition = 255*servoPercentageLocal;

  Serial.print("servoPercentage*255=");
  Serial.println(servoPosition,DEC);

  servoPosition /= 100;

  Serial.print("after div=");
  Serial.println(servoPosition,DEC);

  analogWrite(hPin,(int)servoPosition);
}

I have no idea how it manages that after being set to be 10! With the ‘int’ the value comes out as 10, and the division works fine too!


servoPercentageLocal=10
servoPercentage*255=2550
after div=25
timeCycle=5617
timeDelay=-4617

servoPercentageLocal=10
servoPercentage*255=2550
after div=25
timeCycle=5626
timeDelay=-4626

SO IN CONCLUSION
It seems to be a casting problem. Arduino doesn’t like mixing ints and bytes in a calculation. This may have been obvious with floats and ints but I didn’t know this too. I have casted servoPercentage as an int (when it is called in the main loop not shown) and good things now happen!

THANK YOU ALL!!

// H2 Flow Control
class hController {
public:
  hController();
  void servoControl(int servoPercentage);
private:
  int servoPercentageLocal;
  int servoPosition;
};

void hController::servoControl(int servoPercentage) {

  //int servoPercentageLocal = 10;  <--------HERE
  
  Serial.print("servoPercentage=");
  Serial.println(servoPercentage,DEC);

  servoPosition = 255*servoPercentage;

  Serial.print("servoPercentage*255=");
  Serial.println(servoPosition,DEC);

  servoPosition /= 100;

  Serial.print("after div=");
  Serial.println(servoPosition,DEC);

  analogWrite(hPin,(int)servoPosition);
}

I was thinking it's either the CPU or the compiler.

I had one yesterday printing out bytes as hex with spaces between. Arduino casts the bytes into unsigned long and if bit 7 is set then the added 3 bytes are all FF. So if the byte is 7 the hex prints 7 but if the byte is C7 it prints FFFFFFC7.

I remembered someone else having that in a post a day or two before that and the answer was to mask the value in the print but I forgot the syntax and went an extra variable and step getting around.

So there's a third option on where's the trouble. Not the CPU or the compiler but user unaware of Yet Another Detail of how the compiler works.

I'm -wondering- if there's any chance that Serial.print() might change to read the type it's fed and stick to it? It shouldn't add to the compiled code but rather to the compiler code. It should also make the compiled code smaller by not doing the cast.
There's probably a reason why not. Like it wouldn't be compatible with what came before and old code might probably break. But we don't use xor to flip HIGH and LOW because maybe ever in future those might not be 1 and 0 even if there is old code that would break if they changed.

Yes, I now too think there is a wider problem.

I think there is a threading issue with Serial. My code below is a simple class to force the main while loop to run at ?Hz.

The final stage is to set timeLast=timeNow, before going through the loop again. The timeLast seems to stick, but interestingly when you look at the output, timeLast is set to be a number that was never displayed as timeNow!!! Very odd.

servoPosition=198
DECREASING
timeNow=131014
timeLast=65734
timeCycle=0
timeDelay=1000

servoPosition=198
DECREASING
timeNow=132038
timeLast=65734
timeCycle=65536
timeDelay=-64536

servoPosition=198
DECREASING
timeNow=132038
timeLast=131270
timeCycle=0
timeDelay=1000

:smiley-crying:

// Realtime
class time {
public:
  time(){    
    timeNow=0,timeCycle=0,timeDelay=0,ledState=LOW; 
  }
  void realtime();
  void flashy();
  void init();
  void setLast(long input);
  long getLast();
  static long timeLast;
private:
  long timeNow;
  long timeCycle;
  long timeDelay;
  int ledState;
};

long time::timeLast=0;
void time::setLast(long input){
  timeLast=input;
}
long time::getLast(){return timeLast;}

void time::realtime(){
#ifdef verbose
  timeNow = millis();

  Serial.print("timeNow=");
  Serial.println(timeNow,DEC);
  Serial.print("timeLast=");
  Serial.println(getLast(),DEC);

  timeCycle = (timeNow - getLast());

  Serial.print("timeCycle=");
  Serial.println(timeCycle,DEC);

  timeDelay = (long)frequency - timeCycle;

  Serial.print("timeDelay=");
  Serial.println(timeDelay,DEC);

  if (timeCycle < frequency) {
    delay(timeDelay);
  }
#endif
  setLast((long)timeNow);
}

Purely out of hope you could try uninstalling your compiler then downloading and installing a fresh one.

Other than that, thanks for the notice that simple * and / might get so @#$%&-ed up!

Please, please don't refer to an interval as 'frequency'. Frequency and interval are the inverse of each other.

Is there any way for any of this code to be accessed from within an interrupt? If you multi-threading without using 'volatile' that could easily cause this sort of anomally.

Is there any conceivable way there could be more than one instance of your time class (or subclass of it)? Because timelast is a static, which means it's going to be shared ...

Given the complexity of your code I'd be more inclined to suspect there was something unexpected happening in the execution of your application, than that the compiler can't handle arithmetic. Can you reproduce a similar problem just putting a straight forward calculation in loop()?

Hi there, I know freq is the inv of time period. I originally was setting the frequency then decided to change my code to use time and didnt change the names! I’ll do it for the release version.

The only interrupts are I2C and currently the ArduinoNano isn’t connected to anything so the interrupts wont be firing, and, the interrupts dont effect this code.

Interestingly I have now noticed a similar problem in another section of code, where I just get =198 after dividing. Also, servoPercentage*255 is defo not coming out right!!! It’s all integer maths so shouldnt be causing a problem:

// H2 Flow Control
class hController {
public:
  hController();
  void servoControl(int servoPercentage);
private:
  int servoPosition;
};

void hController::servoControl(int servoPercentage) {
  #ifndef verbose
  servoPosition = 255*servoPercentage;
  servoPosition /= 100;
  #endif
  
#ifdef verbose
  Serial.print("servoPercentage=");
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage;
  Serial.print("servoPosition[*255]=");
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;
  Serial.print("servoPosition[/100]=");
  Serial.println(servoPosition,DEC);
#endif

  analogWrite(hPin,servoPosition);
}

servoPercentage=10
servoPosition[*255]=2502 <----- No it isn’t!!!
servoPosition[/100]=198 <----- No it isn’t!!!
servoPercentage=11
servoPosition[*255]=2758 <----- No it isn’t!!!
servoPosition[/100]=198 <----- No it isn’t!!!

Quite annoying. There should be only one instance of the class being opened in the main loop:

void loop()
{
  adc * myAdc;             // Construct the ADC class
  hController * hControl;  // Construct the H2 flow control class
  time * myTime;           // Construct the Time class
  int pos=1;
  int flip=false;

  while(true){
hControl->servoControl((int)pos);
myTime->realtime();}
}

There’s a lot more of code in the while loop but nothing that would conflict

  #ifndef verbose
  servoPosition = 255*servoPercentage;
  servoPosition /= 100;
  #endif
  
#ifdef verbose
  Serial.print("servoPercentage=");
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage;
  Serial.print("servoPosition[*255]=");
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;
  Serial.print("servoPosition[/100]=");
  Serial.println(servoPosition,DEC);
#endif

Calculate a new value if verbose is not defined. Print it if it is. OK.

I'm not sure I follow what you mean paul?

I think I've found the problem was because I was using pointers to classes and I c**ked it up. So I've taken the memory and speed hit & have memory copying everywhere. Now I have all the calculations running as they should be.

I really appreciate all the help everyone has given, cheers!

I'm not sure I follow what you mean paul?

You have this:

  #ifndef verbose
  servoPosition = 255*servoPercentage;
  servoPosition /= 100;
  #endif

The ifndef preprocessor directive says to compile this block of code if verbose is NOT defined. When verbose IS defined, this code is NOT compiled.

Then, you have this:

#ifdef verbose
  Serial.print("servoPercentage=");
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage;
  Serial.print("servoPosition[*255]=");
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;
  Serial.print("servoPosition[/100]=");
  Serial.println(servoPosition,DEC);
#endif

The ifdef preprocessor directive says to compile this block of code if verbose is defined. When verbose IS defined, this code is compiled.

So, if verbose is defined, printing occurs, but calculations do not.
If it is not defined, calculations are performed, but not printed.

So, if verbose is defined, printing occurs, but calculations do not.
If it is not defined, calculations are performed, but not printed.

It doesn't read that way to me (it's been a long day), but certainly the calculation should not be conditional upon anything.

So, if verbose is defined, printing occurs, but calculations do not.
If it is not defined, calculations are performed, but not printed.

But the same calculations are in both??

#ifdef verbose
  Serial.print("servoPercentage=");
  Serial.println(servoPercentage,DEC);
  servoPosition = 255*servoPercentage;    // <--------------HERE
  Serial.print("servoPosition[*255]=");
  Serial.println(servoPosition,DEC);
  servoPosition /= 100;                         // <--------------HERE
  Serial.print("servoPosition[/100]=");
  Serial.println(servoPosition,DEC);
#endif