const int PULSE_PERIOD = 20 / 1000;
const int ZERO_POSITION_WIDTH = 5 / 10000;
const int FULL_POSITION_WIDTH = 24 / 10000;
It does compile and upload but it doesn't work. unsigned int doesn't work either. maybe 0.020 pulse period (20ms) can't be converter to int (20 / 1000 = 0.02).
Also, in order to prevent overflow or underflow would the constrain function be enough?
const int ZERO_POSITION_WIDTH = 5 / 10000;
const int FULL_POSITION_WIDTH = 24 / 10000;
It does compile and upload but it doesn't work.
Of course it does not work. 5 divided by 10000 gives you 0.0005 and the smallest value you can have in an int is 1. Anything less than 1 is treated as 0.
You need to make sure that none of the calculations in your code produces a number less than 1 (unless it is intended to be 0).
Robin2:
Of course it does not work. 5 divided by 10000 gives you 0.0005 and the smallest value you can have in an int is 1. Anything less than 1 is treated as 0.
You need to make sure that none of the calculations in your code produces a number less than 1 (unless it is intended to be 0).
...R
Well, that was pretty obvious, my bad. However, I've done the math and found out that the values are already always int, and pretty big too.
In fact:
const unsigned long PULSE_WIDTH_COUNT = F_CPU / PRESCALER * PULSE_PERIOD;
const unsigned long PULSE_WIDTH_COUNT = 16000000 / 8 * 0.020;
and the result is 100000000
Same goes for ZERO_POSITION_COUNT and FULL_POSITION_COUNT.
With that being said, I used Serial.print for OCR1A and OCR2A with those results:
(AVERAGE_X and AVERAGE_Y is 512)
OCR1A = 3000
OCR2A = 3000
The values goes from 1500 (lowest value) up to 4500 (highest value) of 2 analog pot on A3 and A4
For completeness, here's the full code (I've replaced some arduino function with some bare instructions taken from the datasheet):
const unsigned long PRESCALER = 8;
const float PULSE_PERIOD = 0.020;
const float ZERO_POSITION_WIDTH = 0.0005;
const float FULL_POSITION_WIDTH = 0.0024;
const unsigned long PULSE_WIDTH_COUNT = F_CPU / PRESCALER * PULSE_PERIOD;
const unsigned long ZERO_POSITION_COUNT = F_CPU / PRESCALER * ZERO_POSITION_WIDTH;
const unsigned long FULL_POSITION_COUNT = F_CPU / PRESCALER * FULL_POSITION_WIDTH;
const int NUMREADINGS = 32;
int READINGS_X[NUMREADINGS];
int READINDEX_X = 0;
int TOTAL_X = 0;
int AVERAGE_X = 0;
int READINGS_Y[NUMREADINGS];
int READINDEX_Y = 0;
int TOTAL_Y = 0;
int AVERAGE_Y = 0;
void setup()
{
analogReference(EXTERNAL); //take out ADMUXA and ADMUXB (to do)
Serial.begin(115200);
DIDR0 = bit (ADC4D) | bit (ADC3D); //turn off digital output on adc3 and adc4
TCCR1A = 0;
ICR1 = PULSE_WIDTH_COUNT - 1;
TCCR1A = bit (WGM11);
TCCR1B = bit (WGM13) | bit (WGM12) | bit (CS11);
TCCR1A |= bit (COM1A1);
TCCR2A = 0;
ICR2 = PULSE_WIDTH_COUNT - 1;
TCCR2A = bit (WGM21);
TCCR2B = bit (WGM23) | bit (WGM22) | bit (CS21);
TCCR2A |= bit (COM2A1);
DDRB = 0b0100; //set PB2 output
DDRA = 0b01000000; //set PA6 output
}
void loop()
{
GETVALUES();
OCR1A = ZERO_POSITION_COUNT + (AVERAGE_X * (FULL_POSITION_COUNT - ZERO_POSITION_COUNT) / 1024) - 1;
Serial.print(OCR1A);
Serial.print(" - ");
OCR2A = ZERO_POSITION_COUNT + (AVERAGE_Y * (FULL_POSITION_COUNT - ZERO_POSITION_COUNT) / 1024) - 1;
Serial.println(OCR2A);
delay(1);
}
void GETVALUES()
{
TOTAL_X = TOTAL_X - READINGS_X[READINDEX_X];
READINGS_X[READINDEX_X] = analogRead(A3); //to be changed on new board
TOTAL_X = TOTAL_X + READINGS_X[READINDEX_X];
READINDEX_X = READINDEX_X + 1;
if (READINDEX_X >= NUMREADINGS) {
READINDEX_X = 0;
}
AVERAGE_X = TOTAL_X / NUMREADINGS;
TOTAL_Y = TOTAL_Y - READINGS_Y[READINDEX_Y];
READINGS_Y[READINDEX_Y] = analogRead(A4); //to be changed on new board
TOTAL_Y = TOTAL_Y + READINGS_Y[READINDEX_Y];
READINDEX_Y = READINDEX_Y + 1;
if (READINDEX_Y >= NUMREADINGS) {
READINDEX_Y = 0;
}
AVERAGE_Y = TOTAL_Y / NUMREADINGS;
}
Robin2:
Then you have a problem because 100,000,000 won't fit in a 16 bit number. The max is 65535. You need to increase the size of the prescaler.
When you post the next version of your code please also tell us what it does when you run it.
...R
Isn't it 4,294,967,295 (2^32 - 1)? I mean, it's not an int, it's an unsigned long (const, but that doesn't matter for the size, it doesn't change anyway).
AVERAGE_X and AVERAGE_Y are the only int in the whole sketch.
If I upload and run the code, both servos moves perfectly as they are supposed to (32 readings per adc pin are plenty enough); there's no jitter at all (I've made a couple of low pass filter on the analog inputs too in order to filter spikes/noise and slow down the values and so the servos movements, thus preventing the need of a specific library such as VarSpeedServo).
Actually, this was exactly the main purpose of the whole thread, but I'm sure that the code can be improved and simplified (for example, I want to remove analogReference and analogRead and replace them with the pin or port specific command).
c0rsa1r:
Isn't it 4,294,967,295 (2^32 - 1)? I mean, it's not an int, it's an unsigned long (const, but that doesn't matter for the size, it doesn't change anyway).
c0rsa1r:
I do have two 16 bit timer (1 and 2) plus an 8 bit timer (0).
I know.
You are using Timer1 which is the 16 bit timer. The largest count it can deal with is 16 bits long
Could this the reason behind the fact that it seems to work?
I have no idea. If something is working by "accident" a small change will probably cause it to go awry.
What all this amounts to is that to control a 16 bit timer you must arrange your maths to produce values that are in the range required by the Timer. You can, of course have larger intermediate values in a calculation. That is often useful to retain precision. But the number must be reduced to a max of 16 bits before being used to set the Timer registers.