Split from: External clock maximum frequency

Go To Last Post
31 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

HI,

I want the timer of Atmega 328p to use the output frequency of TCS3200(light - frequency sensor) as its external clock source.I expect the clock to stop at every 1s and print the output.

But the code doesn't seem to work.

Can anyone kindly point out the mistake.

#include <avr/io.h>
#include <avr/interrupt.h>
#define S0 2
#define S1 3
#define S2 6
#define S3 5
#define sensorOut 4
void sensor() {
  pinMode(S0, OUTPUT);
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(S3, OUTPUT);
  pinMode(sensorOut, INPUT);
   
  digitalWrite(S0,HIGH);
  digitalWrite(S1,LOW);//20% scaling
  digitalWrite(S2,LOW);
  digitalWrite(S3,LOW);//RED
  //digitalWrite(S2,HIGH);
  //digitalWrite(S3,HIGH);//Green
  //digitalWrite(S2,LOW);
  //digitalWrite(S3,HIGH); //Blue
}
void timer_1()

      OCR1A = 0x3D08;
      TCCR1B |= (1 << WGM12);// Mode 4, CTC on OCR1A
      TIMSK1 |= (1 << OCIE1A);//Set interrupt on compare match
      TCCR1B |= (1 << CS12) | (1 << CS10);// set prescaler to 1024 and start the timer
      sei();// enable interrupts
      while (1)
       {
          //working Timer
       }
}
void setup(void)
{   
    Serial.begin(9600);
    void sensor();
    void timer_1();
}

 ISR (TIMER1_COMPA_vect)
{
   
    Serial.print("  ");
    Serial.print( TCNT0);// prints every 1 sec
    Serial.print("  ");
    timer_0();
   
}
void timer_0()
{
  TCNT0=0;
  TCCR0A |=(0<<COM0A1)|(0<<COM0A0)|(0<<COM0B1)|(0<<COM0B0);//normal mode
  TCCR0A |=(0<<WGM02)|(0<<WGM01)|(0<<WGM00);//normal mode
  TCCR0B |=(1<<CS02)|(1<<CS01)|(0<<CS00);//external source on T0 pin
   
 }
 void loop(void){};

This topic has a solution.
Last Edited: Tue. Aug 22, 2017 - 11:26 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Shouldn't the pinMode for sensorOut be input?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But if I give it as an input how will the sensor provide output frequency to the timer?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Errr ... if the sensor is to provide frequency to the timer then, at the timer, that is an input - isn't it?

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

How to properly post code: http://www.avrfreaks.net/comment... (also images)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

yes, you are right!

 

I did the following changes,  but I am not getting any output in the serial port.

#include <avr/io.h>
#include <avr/interrupt.h>
#define S0 2
#define S1 3
#define S2 6
#define S3 5
#define sensorOut 4
void sensor() {
  pinMode(S0, OUTPUT);
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(S3, OUTPUT);
  pinMode(sensorOut, INPUT);
   
  digitalWrite(S0,HIGH);
  digitalWrite(S1,LOW);//20% scaling
  digitalWrite(S2,LOW);
  digitalWrite(S3,LOW);//RED
  //digitalWrite(S2,HIGH);
  //digitalWrite(S3,HIGH);//Green
  //digitalWrite(S2,LOW);
  //digitalWrite(S3,HIGH); //Blue
}
void timer_1()

      OCR1A = 0x3D08;
      TCCR1B |= (1 << WGM12);// Mode 4, CTC on OCR1A
      TIMSK1 |= (1 << OCIE1A);//Set interrupt on compare match
      TCCR1B |= (1 << CS12) | (1 << CS10);// set prescaler to 1024 and start the timer
      sei();// enable interrupts
      while (1)
       {
          //working Timer
       }
}
void setup(void)
{   
    Serial.begin(9600);
    void sensor();
    void timer_1();
}

 ISR (TIMER1_COMPA_vect)
{
    Serial.print("  ");
    Serial.print( TCNT0);// prints every 1 sec
    Serial.print("  ");
    timer_0();
   
}
void timer_0()
{
  TCNT0=0;
  TCCR0A |=(0<<COM0A1)|(0<<COM0A0)|(0<<COM0B1)|(0<<COM0B0);//normal mode
  TCCR0A |=(0<<WGM02)|(0<<WGM01)|(0<<WGM00);//normal mode
  TCCR0B |=(1<<CS02)|(1<<CS01)|(0<<CS00);//external source on T0 pin
   
 }
 void loop(void){};

 

 

 

Last Edited: Thu. Aug 10, 2017 - 08:48 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Using | with 0<< is NOT the way to clear bits and the answer to "if (TOV0 == 1)" is never going to change.

 

I'm not sure what TOV0 is defined as for this micro but it'll just be some number between 0 and 7 and that will never change so it either will be ==1 or it won't be for all time.

 

You need to read the register that bit occurs in then use an & mask to pick out the single bit and test whether that is non-0 or not.

Last Edited: Thu. Aug 10, 2017 - 08:55 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Geeth wrote:
the code doesn't seem to work.

Is uninformative!

 

Tell us:

  • What you expected it to do;
  • What it actually does;
  • What testing and debugging you have done to investigate the problem
  • What you have learned from your testing and debugging 

 

http://www.avrfreaks.net/comment/2173461#comment-2173461

 

Show your schematic (see instructions above for posting images) ?

 

It would also be helpful to provide a datasheet link for the TCS3200

 

Have you checked the manufacturer's website for support materials on the TCS3200 - application notes, examples, etc?

 

Have you googled for 3rd-party materials - in particular, Arduino ... ?

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have edited the code. Can you please checkk

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
that will never change so it either will be ==1 or it won't be for all time

Did you not get a compiler warning about that ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Geeth wrote:
I have edited the code.

Don't do that!!

 

angry

 

It makes the discussion impossible to follow.

 

Add a reply with the modifications - don't edit existing posts.

 

And pay attention to the instructions for posting code!

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

1. I expect it to print the value of the counter(TCNT0) for every 1sec.

2.But it does not print any value in the serial port.

3.The timer1 provides a 1 sec window where I can print the value of timer0(for every 1 sec.)

 the following are the reference material that I used:<http://www.mouser.com/catalog/sp...

<http://www.theorycircuit.com/col... datasheet of  ARDUINO 328p

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

oh ok, I am sorry ,I am new to this forum.I will make sure not to edit the posted codes

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Geeth wrote:
1. I expect it to print the value of the counter(TCNT0) for every 1sec.

2.But it does not print any value in the serial port.

 

How do you know that it's not "printing" to the serial port? How do you know it's not just a fault between the serial port and whatever you're using to observe the output ... ?

 

What have you done to find out why you don't see anything?

 

Start by thinking of possible reasons - I've given one above.

 

Again, you need to adopt a debugging process: http://www.avrfreaks.net/comment/2173461#comment-2173461

 

Image result for ibm think

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am using Arduino IDE and it has a serial monitor in it. And when I write a simple command Serial.print("Arduino"),it writes ARDUINO into the serial monitor. So that confirms that the serial port is working good.

That is the debugging method I tried.I think the reason it does not print into the serial port is because it does not enter into  (ISR (TIMER1_COMPA_vect) function.

Last Edited: Thu. Aug 10, 2017 - 09:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Geeth wrote:
I am using Arduino IDE and it has a serial monitor in it.

You need to remember that we can't see you, or what you have, or what you're doing - the only thing we have to go on is what you write in your posts!

 

And when I write a simple command Serial.print("Arduino"),it writes ARDUINO into the serial monitor. So that confirms that the serial port is working good.

Excellent - that is the way to proceed: check & verify one thing at a time...

 

I think the reason it does not print into the serial port is because it does not enter into  (ISR (TIMER1_COMPA_vect) function.

Good. That's a hypothesis - now think how you might prove that ...

 

http://www.8052.com/faqs/120313

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The Arduino runtime preconfigures all of the timers.  While you may think that you are configuring TIMER0 and TIMER1 from scratch, you are actually modifying the configuration in ways you have not understood.

 

Avoid the use of |= when configuring your timer registers.  Configure all bits from scratch using = instead.  Note also that the order in which you write to the timer registers may be important depending on their previous state.  Hint:  in your case you will need to write to TCCR1A/B before writing to OCR1A.  Bonus:  why?

 

Further, TIMER0 is used by the Arduino runtime to count milliseconds.  This may interfere with your needs, so you should disable the TIMER0 interrupts.

 

Question:  Since you're using the Arduino sketch paradigm, why are you trying to re-invent the wheel?  millis() already provides you with a time-base.  Use it.  Configure TIMER1 for external clocking instead, and read TCTN1 after 1000 ms have elapsed.

 

 

 

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Hi Geeth,

 

I have made some notes in your code.

Do you realise that it can count only to 255 Hz.

 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>

#define S0 2
#define S1 3
#define S2 6
#define S3 5
#define sensorOut 4

//---------------------------------------
void sensor() {
  pinMode(S0, OUTPUT);
  pinMode(S1, OUTPUT);
  pinMode(S2, OUTPUT);
  pinMode(S3, OUTPUT);
  pinMode(sensorOut, INPUT);

  digitalWrite(S0,HIGH);
  digitalWrite(S1,LOW);//20% scaling
  digitalWrite(S2,LOW);
  digitalWrite(S3,LOW);//RED
  //digitalWrite(S2,HIGH);
  //digitalWrite(S3,HIGH);//Green
  //digitalWrite(S2,LOW);
  //digitalWrite(S3,HIGH); //Blue
}

//---------------------------------------
void timer_1()
{
      //OCR1A = 0x3D08;                      ///// place this on the end of timer configuration
      TCCR1A = 0;                            ///// added, never believe that a register is not changed by Arduino
      //TCCR1B |= (1 << WGM12);              ///// for the first writing to a register use "=" , not "|="
      TCCR1B = (1 << WGM12);                 // Mode 4, CTC on OCR1A
      TCCR1B |= (1 << CS12) | (1 << CS10);   // set prescaler to 1024 and start the timer
      TIMSK1 = (1 << OCIE1A);                // Set interrupt on compare match
      OCR1A = 0x3D08;
      //sei();// enable interrupts           ///// this should be on the end of setup()

     //while (1)                             ///// error, program will never get out of this loop
       //{
          //working Timer
       //}
}

//---------------------------------------
 ISR (TIMER1_COMPA_vect)                       ///// using print() in an ISR is a bad practice
{
    //Serial.print(" ");                       ///// use Serial.println
    Serial.println( TCNT0);                    // prints every 1 sec
    TCNT0 = 0;                                 ///// clear counter
    //Serial.print(" ");
    //timer_0();                               ///// this should be in setup()

}

//---------------------------------------
void timer_0()
{
  TCNT0=0;
  //TCCR0A |=(0<<COM0A1)|(0<<COM0A0)|(0<<COM0B1)|(0<<COM0B0);//normal mode     ///// why not simply TCCR0A = 0;
  //TCCR0A |=(0<<WGM02)|(0<<WGM01)|(0<<WGM00);//normal mode
  TCCR0A = 0;
  TCCR0B =(1<<CS02)|(1<<CS01)|(0<<CS00);//external source on T0 pin         ///// for the first writing to a register use "=" , not "|="
}

//=======================================
void setup(void)
{
    Serial.begin(9600);
    //void sensor();                        ///// should be  sensor();  we call functions without "void"
    sensor();
    //void timer_1();
    timer_1();
    timer_0();
    sei();
}   

 //---------------------------------------
 void loop(void){};

//=======================================

 

Last Edited: Fri. Aug 11, 2017 - 02:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

HI,

I found that using arduino IDE might result in bug sometimes, so I re- wrote the code for AVR studio.

Here I used a color sensor output  as clock source to run timer_0 of atmega 168.Timer_1 of atmega 168 uses the internal clock source ( 8MHZ)and for every 1 sec prints the timer_0 value.

But timer_0 value is not changing based on the color sensor value.Timer_0 gives a constant output.

I tested the color sensor using oscilloscope and it gives correct  output.To debug I used a PWM to run the clock source of timer_0 instead of using color sensor.Yet I got the same constant value.

So I think the mistake is in initializing the timer_0 to accept the external clock source.But in the data sheet the following is mentioned:  

And I made the same setting.If you could find the mistake please highlight.

#include <avr/io.h>
#include <avr/interrupt.h>
#include <string.h>
#define F_CPU 8000000UL
#define BAUD 9600
#include <util/delay.h>
#define MYUBBR F_CPU/16/BAUD-1


void uart_init(unsigned int ubrr)
{
	// Setting the baud rate to ubrr
	UCSR0A = 0<<U2X0;
	UBRR0H = (unsigned char)(ubrr >> 8);
	UBRR0L = (unsigned char)ubrr;
	//Setting the frame format to 8 data bit, no parity bit, 1 stop bit
	UCSR0C = 3<<UCSZ00;
	UCSR0B = (1<<TXEN0)|(1<<RXEN0);
}
void USART_Transmit( unsigned char data )
{
	while (!(UCSR0A & (1<<UDRE0)));
	UDR0 = data;
}
void transmit_string(char data)
{
		USART_Transmit(data);
}

void timer_0()
{
  TCNT0=0;
  TCCR0B =(1<<CS00)|(1<<CS01)|(0<<CS00);//external source on T0 pin
 };
 void sensor() 
{
  DDRD|=(1<<DDB4);//S0 output
  DDRD|=(1<<DDB5);//S1 output
  DDRD|=(1<<DDD2);//S2 output
  DDRD|=(1<<DDD3);//S3 output
  DDRD|=(0<<DDD4);// input
  PORTD=(1<<PB4)|(0<<PB5)/*20% scaling*/|(0<<PD2)|(0<<PD3);//RED
};
int main(void)
{
	CLKPR = (1 << CLKPCE);// Setting system clock to 8MHz
	CLKPR = 0;
	uart_init(MYUBBR);
	sensor();
  	OCR1A = 0x1E83;
	TCCR1B = (1 << WGM12);// CTC on OCR1A
	TIMSK1 |= (1 << OCIE1A);//Set interrupt on compare match
    TCCR1B |= (1 << CS12) | (1 << CS10);// set prescaler to 1024 and start the timer
	sei();// enable interrupts
	while (1)   {// we have a working Timer
				}
}

ISR (TIMER1_COMPA_vect)
{
		transmit_string(TCNT0);
		timer_0();
		_delay_ms(100);

	
}








 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A delay in an ISR? That immediately says "bad design".

 

Also if you ever find yourself writing to TCNT in a timer ISR it is almost certainly saying "should have used CTC mode".

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I used the delay in ISR so that when I check the output in cool term it prints slowly.

And I called the timer_0() function in ISR so as to re-initialize timer_0 after printing the value.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

I found that using arduino IDE might result in bug sometimes, so I re- wrote the code for AVR studio.

The bugs were yours, not Arduino's.  Did you understand what I said in #17?  Did you look at Visovian's code in #18?

 

  DDRD |= (1<<DDB4);  //S0 output
  DDRD |= (1<<DDB5);  //S1 output
  DDRD |= (1<<DDD2);  //S2 output
  DDRD |= (1<<DDD3);  //S3 output
  DDRD |= (0<<DDD4);  // input

Looks like you've got a copy/paste error there.  I assume the first two need to be DDRB?

 

Again here:

  PORTD = (1<<PB4) | (0<<PB5) /*20% scaling*/ | (0<<PD2) | (0<<PD3);  //RED

 

I think you also need to have a look at the tutorials section on bit manipulation.  Or-ing a register with zero does nothing at all.  In this case, there's no harm because the default state of the DDR registers is zero anyway, so all pins are inputs after a reset.

 

void transmit_string(char data) {
  USART_Transmit(data);
}

You have misunderstood what a C string is.  This will only send one byte.  Your call to:

transmit_string(TCNT0);

... will send one byte, the lower byte of TCNT0.  Since you're calling this from the compare match interrupt, the value of TCNT0 should be the same value as OCR1A, or 0x1E83.  The lower byte of that is 0x83.  That is not a part of the standard ASCII character set.  What your terminal does with it will depend on it's settings.  It might do nothing.  It might print 'â', it might print a graphic block.

 

  TCCR0B = (1<<CS00) | (1<<CS01) | (0<<CS00); //external source on T0 pin

Incorrect.  Look at it again:

  TCCR0B = (1<<CS00) | (1<<CS01) | (0<<CS00); //external source on T0 pin

You are setting a prescaler of /64.

 

Please, please, please, put some effort into formatting your code for readability, especially if you're asking others to look at it to try to find an error.

#define F_CPU 8000000UL
#define BAUD 9600



#include <avr/io.h>
#include <avr/interrupt.h>
#include <string.h>
#include <util/delay.h>



#define MYUBBR F_CPU/16/BAUD-1



void uart_init(unsigned int ubrr) {
  // Setting the baud rate to ubrr
  UCSR0A = 0<<U2X0;
  UBRR0H = (unsigned char)(ubrr >> 8);
  UBRR0L = (unsigned char)ubrr;
  //Setting the frame format to 8 data bit, no parity bit, 1 stop bit
  UCSR0C = (3<<UCSZ00);
  UCSR0B = (1<<TXEN0) | (1<<RXEN0);
}



void USART_Transmit( unsigned char data ) {
  while (!(UCSR0A & (1<<UDRE0)));
  UDR0 = data;
}



void transmit_string(char data) {
  USART_Transmit(data);
}



void timer_0() {
  TCNT0  = 0;
  TCCR0B = (1<<CS00) | (1<<CS01) | (0<<CS00); //external source on T0 pin
 };



void sensor() {
  DDRD |= (1<<DDB4);  //S0 output
  DDRD |= (1<<DDB5);  //S1 output
  DDRD |= (1<<DDD2);  //S2 output
  DDRD |= (1<<DDD3);  //S3 output
  DDRD |= (0<<DDD4);  // input
  PORTD = (1<<PB4) | (0<<PB5) /*20% scaling*/ | (0<<PD2) | (0<<PD3);  //RED
};



int main(void) {
  CLKPR = (1 << CLKPCE);                // Setting system clock to 8MHz
  CLKPR = 0;
  uart_init(MYUBBR);
  sensor();
  OCR1A = 0x1E83;
  TCCR1B  = (1 << WGM12);               // CTC on OCR1A
  TIMSK1 |= (1 << OCIE1A);              // Set interrupt on compare match
  TCCR1B |= (1 << CS12) | (1 << CS10);  // set prescaler to 1024 and start the timer
  sei();                                // enable interrupts
  while (1) {}                          // we have a working Timer
}



ISR (TIMER1_COMPA_vect) {
  transmit_string(TCNT0);
  timer_0();
  _delay_ms(100);
}

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Fri. Aug 18, 2017 - 05:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
Please, please, please, put some effort into formatting your code for readability, especially if you're asking others to look at it to try to find an error.

+10

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Geeth wrote:
I found that using arduino IDE might result in bug sometimes

I have compiled the code in #18 with Arduino 1.8.1.

Then I tested on Arduino nano board (with 10 cm wire on D4 pin).

See result in arduino serial monitor (net freq = 50 Hz).

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have made the copy/paste error twice and I have corrected it .while initializing timer_0 I have made a mistake again, thank you for pointing that out.I apologize for not formatting the code well.

Yes, I looked into Visovian's code and avoided the use of  |=  when configuring timer registers.

 

Can you please explain this :

"

... will send one byte, the lower byte of TCNT0.  Since you're calling this from the compare match interrupt, the value of TCNT0 should be the same value as OCR1A, or 0x1E83.  The lower byte of that is 0x83.  That is not a part of the standard ASCII character set.  What your terminal does with it will depend on it's settings.  It might do nothing.  It might print 'â', it might print a graphic block.

 

"

TCNT0 is an 8 bit reg(1 byte), so if I have to send the entire 1 byte of information what should I do?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

Did the values change based on the color sensor input?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Whoops.  My mistake.  In my haste I'd conflated TIMER1 and TIMER0.

 

I'm still confused why you have a function called transmit_string(), which simply calls USART_Transmit() once, which simply sends one byte.  What are you hoping to accomplish?

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That was a mistake from my end ;creating a transmit_string() was unnecessary  because USART_Transmit()  does the same function.So I corrected that and now the code has only  USART_Transmit().

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Did the values change based on the color sensor input?

 

I have not got a complete stock of IC's at home, so I could not test with the sensor.

 

The program measures frequency, so if the sensor changes frequency then the value will change for sure.

 

I warn again:

Full-scale frequency of the sensor is min. 10 kHz while your code can measure up to 255 Hz.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Geeth wrote:
 if I have to send the entire 1 byte of information what should I do?

The UART always sends 1 complete byte.

 

You really should get solid UART comms working before you move on to anything else!

 

The ATmega328P also has on-chip debug - you should learn to use it.

 

ADDENDUM

 

http://www.avrfreaks.net/comment...

Last Edited: Tue. Aug 22, 2017 - 08:52 AM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks joeymorin.I made the changes that you had pointed out and my program works.It produces the expected output for every 1sec.