The servo code didn't work

I attached 2 hobby servos on D9 and D10 of UNO, I tested the Arduino IDE example for servo, it works, while when I used registers, it didn't work, could anyone tell me what I lost?

void setup() {
  // put your setup code here, to run once:
  TCCR1A|=(1<<COM1A1)|(1<<COM1B1)|(1<<WGM11);        //NON Inverted PWM 
  TCCR1B|=(1<<WGM13)|(1<<WGM12)|(1<<CS11)|(1<<CS10); //PRESCALER=64 MODE=14(FAST PWM)
  DDRB |= (1<<PB1) | (1<<PB2);   //PWM Pins as Output
  ICR1=2499;  //fPWM=50Hz

void loop() {
  // put your main code here, to run repeatedly:
  OCR1A = 250;
  OCR1A = 375;
  OCR1A = 500;

  OCR1B = 250;
  OCR1B = 375;
  OCR1B = 500;

What does your oscilloscope show?

I haven't use it.

I can't help, since I have never done that port addressing stuff, but please explain what you think it does? Maybe it's doing what you expect, and that might not be what a servo expects.

I see you seem to set the pins as pwm, but servos don't use pwm in the simple Arduino sense.

Yes that indeed is the problem. @MianQi you are using the registers to set the PWM function of the timers. Why do you want to do this anyway?

Servos are fed with PPM, pulse position modulation. That is where a pulse is generated ever 50mS or so and it is the width of this pulse that controls the servo's position. This can be generated on any pin using the servo library. It seems a waste of time using anything else unless you have a good reason.

Yes, I also meant to say that with the PPM approach it's the absolute high time that's important, not the % ala PWM. The frequency (afaik) just needs to make sure the pulse is often enough to get the servo to keep on station.

I do have such a reason, so I have to use registers.

And, I found these in the datasheet of ATmega328p:

To do a 16-bit write, the high byte must be written before the low byte. For a 16-bit read, the low byte must be read before the
high byte.

and, I wrote this:

void setup() {
  // put your setup code here, to run once:
  TCCR1A|=(1<<COM1A1)|(1<<COM1B1)|(1<<WGM11)|(1<<COM1A0)|(1<<COM1B0);        //NON Inverted PWM 
  TCCR1B|=(1<<WGM13)|(1<<WGM12)|(1<<CS11)|(1<<CS10); //PRESCALER=64 MODE=14(FAST PWM)
  //TCCR1B|=(1<<WGM13)|(1<<WGM12)|(1<<CS11); //PRESCALER=8 MODE=14(FAST PWM)
  DDRB|= (1<<PB1)|(1<<PB2);   //PWM Pins as Output
  //ICR1=39999;  //fPWM=50Hz
  ICR1=4999;  //fPWM=50Hz

void loop() {
  // put your main code here, to run repeatedly:
  OCR1AH = highByte(250);
  OCR1AL = lowByte(250);
  //OCR1A = 250;
  OCR1AH = highByte(375);
  OCR1AL = lowByte(375);
  OCR1AH = highByte(500);
  OCR1AL = lowByte(500);
  //OCR1A = 500;

It still didn't work.

and this:

  ICR1H = highByte(4999);
  ICR1L = lowByte(4999);

I could only heard some noise form the servo.

Explain what you think you're sending to the servo...

The output of OCR1A and OCR1B, is that right?

NO, I didn't set it this way, it just requested from the data sheet.

Tel us what signal you think is getting sent to the servos

There are some problems when setting the registers. You must not use the |= operator, because there are bits set from arduino initializing, that must be cleared. Therefore use the '=' operator to set the control registers.
With a prescaler of 64, TOP must be set to 5000 to get a 20ms refresh rate.
Using 16-bit registers is ok, the compiler handles this correctly.

Try this:

  TCCR1A=(1<<COM1A1)|(1<<COM1B1)|(1<<WGM11);        //NON Inverted PWM 
  TCCR1B=(1<<WGM13)|(1<<WGM12)|(1<<CS11)|(1<<CS10); //PRESCALER=64 MODE=14(FAST PWM)
  ICR1=5000;  //fPWM=50Hz

N.B. You can have a better resolution with a prescaler of 8 and a TOP value of 40000.

In remote-controlled model flight, PPM is a mutliplex signal from the receiver that contains the information for all connected servos. Each individual servo receives a pure PWM signal, whereby the servo position does not depend on the pulse-pause ratio, but on the absolute pulse length ( 1..2ms ) with a constant repetition rate of 20ms ( the repetition rate is less important and may vary a bit without changing the servo position ).
And that's exactly what the timer creates. The timer pulses are much more stable than the ISR created pulses of the servo library.

I found this code works:

            DDRB |= (1 << DDB1)|(1 << DDB2);
            // PB1 and PB2 is now an output

            ICR1 = 0xFFFF;
            // set TOP to 16bit

            OCR1A = 0x3FFF;
            // set PWM for 25% duty cycle @ 16bit

            OCR1B = 0xBFFF;
            // set PWM for 75% duty cycle @ 16bit

            TCCR1A |= (1 << COM1A1)|(1 << COM1B1); // |(1 << COM1A0)|(1 << COM1B0); // Add the extra code to make it inverting
            // set none-inverting mode

            TCCR1A |= (1 << WGM11);
            TCCR1B |= (1 << WGM12)|(1 << WGM13);
            // set Fast PWM mode using ICR1 as TOP
            TCCR1B |= (1 << CS10);
            // START the timer with no prescaler

            while (1);
                // we have a working Fast PWM

What's the deference between it and my code? I mean the setting pattern are the same.

Sorry, i tested it just now in Arduino IDE, it didn't work either, it just works in thinkercad simulator.

Don't knkow what you are doing. I tested on UNO and Leonardo. It worked perfectly on both boards.

Not for me. The '|=' is still the problem. If you don't use '=' or clear the register first, you don't know how the register is set at the end.
And the code doesn't creat a servo suitable signal. How do you check if it works?

See reply #8 for a full explanation of how to clear and set bits.

But you are not going to say. This could be an X-Y Problem

Yes, Micro, you are right, I found it didn't work now.

Thanks, Grumpy, I found the solution - alter to gcc chain in CMD, the initiation of servo are OK now.

Now, I found the block was in ADC, i am not sure it is suitable to attach the ADC code here or I should open a new thread. Since it was part of my question, so I just attach them here:

	if(knob == 1){
	ADMUX |= (1<<MUX0);
	ADMUX &= 0b11110001; 
	} else if(knob == 0){
	ADMUX &= 0b11110000;	
	int readingInterH = 0;
	int readingInterL = 0;
	int readingInter = 0;
	readingInterH = ADCH;
	readingInterL = ADCL;
	readingInter = (ADCH<<8)|ADCL;
	readingInter = readingInter>>6;
	reading = readingInter / 1023.0;

Is there any issue I lost?

Looks OK. But I haven't checked the ADCSRA register.