| View previous topic :: View next topic |
| Author |
Message |
marcus.couceiro
Joined: 04 Jan 2012
Posts: 12
Location: Brazil
|
| Help with Frequency Counter |
 Posted: Wed Jan 04, 2012 6:25 am |
 |
|
Dear All,
I am new to this PIC area so I would like to request your patience with my question.
I have been reading a lot of books regarding programming techniques and because of that tried to develop a small frequency counter myself.
I know we can find several programs around but my intention is learn the basics. So, here goes the problem:
Using PIC16F873 and Proteus simulator I have written the following code:
| Code: |
#include " 16F877A.h "
#use delay (clock=4000000)
int one_second;
long frequency;
#int_TIMER0
void ISR_tmr0
{ one_second=one_secod++;
clear_interrupt(INT_TIMER0);
}
void main ()
{
while (1)
setup_timer_0(T1_INTERNAL);
setup_timer_1(T1_EXTERNAL | DIV_BY_1);
enable_interrupts(GLOBAL);
enable_interrupts(INT_TIMER0);
if (one_second=3906)
{ frequency=get_timer1();
set_timer1(0);
set_timer0(0);
clear_interrupt(INT_TIMER0);
}
putc(254);putc(1);delay_ms(500);
printf("frequency: %d Hz", frequency);
}
|
Despite the typo you might find above, compiler run without problems. When simulating in Proteus, I can see no errors and nothing showed on display.
Can anyone point out what I did wrong?
Tks
_________________
Marcus Couceiro |
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9163
Location: Greensville,Ontario
|
|
| Posted: Wed Jan 04, 2012 6:58 am |
|
|
What's wrong ...
You are using Proteus !!
Honestly, get rid of it! It is full of bugs, errors and does NOT work properly as a PIC simulator. A quick search of this forum will find HUNDREDS of 'it doesn't work in simulation' and ALL errors are due to the Proteus software, which we obviously cannot debug let alone fix.
Get some real PICs, a breadboard, cut code,burn the PIC, run, observe and learn from doing.Yes, 'old school' but you will LEARN how the PIC works and HOW to cut better code.
After 20+ years of using PICs , I still use this as my developement style.If it works on a breadboard , it'll work in a PCB.
As for your code...
putc(254);putc(1);delay_ms(500);
printf("frequency: %d Hz", frequency);
...
It looks like you're trying to send data to a 'serial LCD' module, but you haven't 'setup' the PIC right by telling it which pin the LCD is on or speed,etc. This would be the use rs232(...) and you'ld have to tell the printf(...) where to send the data as well.
If you press F11 while your project is open, the CCS C onscreen help manual comes up. Instant access to 99% of what you need to know.The rest is shown in the examples found in the 'examples' folder. |
|
|
marcus.couceiro
Joined: 04 Jan 2012
Posts: 12
Location: Brazil
|
|
| Posted: Wed Jan 04, 2012 7:14 am |
|
|
Thank you for your help Temtronic,
Although not shown, the #use RS232 line was included in the code. I do not believe this is the problem.
As for Proteus, I have been simulating small programs with it and never had any issue. Maybe if the programs get larger they will come up.
I also like the "old school" but I also think that I need to evolve to new technologies. If we can simulate if the bridge will break, why build it first?
Could you have a look at the code and see what I did wrong?
Thank you for your help?
Marcus
_________________
Marcus Couceiro |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Wed Jan 04, 2012 8:05 am |
|
|
do it for real - dump proteus....
but errors??
you got em!!!
1- you re init timers every time thru the while loop
2- NO brace after while stmt - hence the display stmt will not execute
3- you dont need to clear the timer int in the ISR - compiler does it for you
4-LOSE delay_ms(500)
and MANY more ......
why not INDENT your code and use the code button here ??
it would help you see what is wrong
you did not write CODE - you wrote disaster ........ |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9163
Location: Greensville,Ontario
|
|
| Posted: Wed Jan 04, 2012 8:11 am |
|
|
The reason for 'building the bridge'( good example !) instead of simulating..
In every Proteus schematic that I've seen posted here, several glaring errors appear even though the 'code' kinda works.
These errors which Proteus ignores but in the real world would be catastorphic include
1) No power to PIC power Vdd and Vss pins,never run in the real world
2) No crystal or caps to PIC xtal pins,never oscillate in the real world
3) NO regard to mismatched Vcc levels of PIC(5 V) to devices(3V)(classic !)
4) _mclr NOT tied high, yet PIC 'runs '.
'Old School' design means proper schematics that would be handed to a tech to wireup and test, then debug.While a seasoned tech would wire up Vdd and Vss,etc. a rookie, fresh out of school would follow the schematic 100% (to impress the boss) but it won't work as shown.
Just a sample of errors that if you built the PIC as shown in the schematic, it will never,ever work.Having 'blind faith' in a simulator is not a good idea. Several people have died due to faulty logic,based on bad simulators.Worst here is a blown PIC, in the real world planes have crashed( I'm also a pilot).
re: the ISR. You do not have to clear it, compiler does that for you.
If you need more help with the program, use the /code button when posting and be sure it's the complete(but small) compilable program. You should also say which PIC and version of the compiler. |
|
|
marcus.couceiro
Joined: 04 Jan 2012
Posts: 12
Location: Brazil
|
|
| Posted: Wed Jan 04, 2012 8:50 am |
|
|
Once again Temtronic, thank you for your help.
Asmboy, maybe you could read the first sentence after "Dear all,".
I am trying to learn so maybe instead of criticize only you could use all the knowledge it seems you have and teach everyone else in the right way.
I did not know how to use the "code" button. I will pay more attention in the future.
Now, I will remove the Clear_INT in the ISR routine and put the last brace of the IF routine after the printF command.
Asmboy, I have seen other codes where people re init timers thru the while loop. I assume this is wrong.
What do you mean with LOSE delay_ms(500)?
Can you point the MANY more errors so I can try to change from "disaster" to code?
Your help is appreciated.
_________________
Marcus Couceiro |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Wed Jan 04, 2012 10:46 am |
|
|
my best advice :
1- lose= GET rid of 500 msec delay
2- learn to solder and build something real,
as simulation is kid stuff and teaches the
unfortunate lesson that "wishes=fishes" -
when in reality , design is far more complex.
Proteus forgives much and teaches little IMHO
btw: i believe you posted here in SEARCH of criticism.
So as the wise man said,
" be careful what you wish for ....."
best of luck
 |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Wed Jan 04, 2012 12:46 pm |
|
|
| Quote: | int one_second;
long frequency;
#int_TIMER0
void ISR_tmr0
{ one_second=one_secod++;
clear_interrupt(INT_TIMER0);
} |
The line in bold above has a variable which has not been declared.
If it's a typo, it's still the wrong way to increment a variable in C.
Just doing this is sufficient:
| Quote: |
int one_second;
if (one_second=3906)
{ frequency=get_timer1();
set_timer1(0);
set_timer0(0);
clear_interrupt(INT_TIMER0);
} |
Regarding the sections in bold above, in CCS, an 'int' is an unsigned 8-bit
integer. It's range is 0 to 255. If you want to go up to 3906, you need to
declare it as 'int16'.
I didn't look at your program design. Just at those coding bugs. |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Wed Jan 04, 2012 1:02 pm |
|
|
RE: STRUCTURE
| Code: |
unsigned int16 one_second=0;
unsigned int16 frequency=0;;
#int_TIMER0
void ISR_tmr0{
++one_second
}
void main () {
setup_timer_0(T1_INTERNAL);
setup_timer_1(T1_EXTERNAL | DIV_BY_1);
enable_interrupts(GLOBAL);
enable_interrupts(INT_TIMER0);
while (1){
if (one_second==3906) {
frequency=get_timer1();
set_timer1(0);
set_timer0(0);
clear_interrupt(INT_TIMER0);
putc(254);putc(1);
printf("frequency: %d Hz", frequency);
}
putc(254);putc(1);delay_ms(500);
printf("frequency: %d Hz", frequency);
} |
that is STRUCTURE fixed a bit
one_second must be a misnomer ---
But w/o knowing the prescalar for timer 0 -??
you should specify it in the setup call--
this much i know - it is sure that the ISR is actually much faster
than "seconds" based on seat of pants math
( timer 0 ISR will be taken much too often 1/sec) |
|
|
marcus.couceiro
Joined: 04 Jan 2012
Posts: 12
Location: Brazil
|
|
| Posted: Wed Jan 04, 2012 2:42 pm |
|
|
Hi Guys,
Thank you so much for your help.
PCM programmer, I realized that if one_second was supposed to count until 3096 I would have to change its type but thanks anyway. I already implemented the changes you proposed below mixing them with the proposed changes from asmboy.
Asmboy,
Before seeing your post, I was trying to change the code so it could look better and easier to understand. Your comments were really helpful. I have also changed the variable one_second to flag (no problems with that).
| Code: |
#include <16F873.h> // Specifies the uP to be used
#FUSES NOWDT // Disable WatchDog Timer
#FUSES XT // Use Crystal Oscillator
#FUSES PUT // Enable Power On Timer
#FUSES NOPROTECT // EPROM Not Protected
#FUSES NOLVP // Disable Low Voltage programming
#FUSES NOCPD // No Code Protection
#use delay(clock=4000000) // Set Crystal Frequency
#use RS232(baud=9600,xmit=PIN_C6,rcv=PIN_C7) // Set RS232 Port Parameters
unsigned int16 flag=0; // Variable to count the number of TIMER0 Interruptions, initially set to zero
unsigned int16 frequency=0; // variable to hold the value from Timer1 set to zero
#int_TIMER0 // Timer0 Interruption
void isr_timer0() // Interrupt Service Request for Timer0
{ ++flag; // Add one to flag variable
}
void main ()
{
setup_TIMER_0(RTCC_INTERNAL | RTCC_DIV_1); // Define TIMER0 as Internal timer (Fosc/4)
setup_TIMER_1(T1_EXTERNAL | T1_DIV_BY_1); //Define Timer1 as External, no pre-scaler
enable_interrupts(GLOBAL); // Enable GLOBAL Interruptions
enable_interrupts(INT_TIMER0); // Enable TIMER0 Interruption
set_TIMER0(0); // Reset TIMER0
set_TIMER1(0); // Reset TIMER1
while(1) //*****************************************Endless Loop
{
IF (flag==3921); // Count approx. 1s @ 4MHz (0.999855s)
{
frequency=get_timer1(); // Stores the number of clocks received at TIMER1
flag=0; // Reset flag so it can start count to 3921 again
set_TIMER0(0); // Reset TIMER0
clear_interrupt(INT_TIMER0); // Clear INT_TIMER0
set_TIMER1(0); // Reset TIMER1
putc(254);putc(1);delay_ms(10); // Prepare display to receive data
printf("frequency: %Ld Hz", frequency); // Show frequency
}
}
}
|
Although it's not working yet, and already apologizing for using Proteus to simulate the program, I could see 145 Hz on the display (I am using 5Mhz as T1CKl).
I supposed that if you assign TIMER0 as RTCC_INTERNAL it would already be configured as Fosc/4 but after reading the Reference manual, I saw a note saying:
Note: To achieve a 1:1 prescaler assignment for the TMR0 register, assign the prescaler to the Watchdog Timer.
I have no idea on how to do that so I included a line stating that at setup_timer_0.
Once again thank you for your help. I will be waiting for your comments.
_________________
Marcus Couceiro |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Wed Jan 04, 2012 2:48 pm |
|
|
Just a comment. The programming concept of a "flag" is a variable that
shows either of two binary states (True and False), or it's a byte that
has up to 8 flag bits in it (each one of those is True or False).
'Flag' is not an appropriate name for a variable that holds a count. |
|
|
marcus.couceiro
Joined: 04 Jan 2012
Posts: 12
Location: Brazil
|
|
| Posted: Thu Jan 05, 2012 3:59 am |
|
|
Hi PCM Programmer,
Thank you for the explanation. This was changed in the new version I will post when I get back home.
I was able to make the program work. The only limitation is that it can count until 35 kHz. Above that nothing changes.
I have changed the concept a little bit. Used TIMER0 to count the pulses and TIMER1 to count to 0.1s. When 0.1s reached there will be overflows on TIMER0 stored in another variable so I get the value at TIMER0, sum with the number of times we had overflow and multiply by 10.
Something like frequency=10((factor*255)+get_timer0()) where factor is the number of times we had overflow.
It worked but only till 35 kHz. I guess it's related to timing. Could anyone try to explain me the reason?
I will post the code by the end of the day.
Thank you all for your help.
_________________
Marcus Couceiro |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
|
marcus.couceiro
Joined: 04 Jan 2012
Posts: 12
Location: Brazil
|
|
| Posted: Sat Jan 07, 2012 9:31 am |
|
|
Gentlemen,
Sorry for the delay. Busy Days!
Here is the code you helped me to write.
| Code: | #include <16F873.h> // Specifies the uP to be used
#FUSES NOWDT // Disable WatchDog Timer
#FUSES XT // Use Crystal Oscillator
#FUSES PUT // Enable Power On Timer
#FUSES NOPROTECT // EPROM Not Protected
#FUSES NOLVP // Disable Low Voltage programming
#FUSES NOCPD // No Code Protection
#use delay(clock=4000000) // Set Crystal Frequency
#use RS232(baud=9600,xmit=PIN_C6,rcv=PIN_C7) // Set RS232 Port Parameters
unsigned int1 flag=0; // Variable to indicate if 0.1 s a counted, initialy set to zero
unsigned int16 frequency=0, factor=0; // variables to hold the value from Timer0 and multiplier factor, initially set to zero
#int_TIMER1 // Timer1 Interruption
void isr_timer1() // Interrupt Service Request for Timer1
{ flag=1; // Add one to flag variable
}
#int_TIMER0
void isr_timer0() // Interrupt Service Request for Timer1
{ factor++; // add one to multiplier factor
}
void main ()
{
setup_TIMER_0(RTCC_DIV_1|RTCC_EXT_L_TO_H); // Define TIMER0 as external counter
setup_TIMER_1(T1_INTERNAL | T1_DIV_BY_8); //Define Timer1 as Internal, pre-scaler set to 8
enable_interrupts(GLOBAL); // Enable GLOBAL Interruptions
enable_interrupts(INT_TIMER1); // Enable TIMER1 Interruption
enable_interrupts(INT_TIMER0); // Enable TIMER0 Interruption
set_TIMER1(53035); // count 0.1s
while(1) //*****************************Endless Loop
{
IF (flag==1) // IF 0.1s have passed....
{
frequency=10*((factor*255)+get_TIMER0()); // Reads the value from Timer0, add the extra pulses and multiply by 10 (0.1s*10= 1s)
flag=0; // Reset flag so it can start count again
set_TIMER0(0); // Reset TIMER0
factor=0; // Reset Multiplier Factor
set_TIMER1(53035); // Count 0.1s again
putc(254);putc(1);putc(0);delay_ms(1); // Prepare display to receive data
printf("freq.: %Lu Hz", frequency); // Show frequency
}
}
}
|
PCM programmer, thank you for the links. Using CCP seems much better. I will try to understand that. The code above counts up to 35K Hz but in the end I have 30 Hz errors (instead of 35000 it shows 34970).
Just for knowledge, does anyone know why it goes up to 35000 and not further?
Thank you all for sharing and helping me understand a little bit of C.
Regards
_________________
Marcus Couceiro |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Sat Jan 07, 2012 10:10 am |
|
|
comment
because you set flag in an ISR and the structure of your loop in MAIN --
essentially you are just polling "flag" but with the extra latency of of the ISR added on.
i think there would be less latency overhead if you declared
#Bit TMR1IF =gentenv(SFR:Tmr1if)
and then in your main loop simply made your conditional be
if (TMR1IF) {...
and add TMR1IF=0 after you re-init timer1 in the main conditional branch
i'm thinking - this IS a counter right? and any excess overhead is an error
oh yeah - enable global ints after ALL the individual INT enables -
NOT beforehand -
as to your max count limit ?
the way its coded makes my head hurt already .....
 |
|
|
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
