James, The typo is fixed. I do that some times. Sorry! I don't understand volatile in this reference. Since this is a global variable isn't auto fine? These have been changed to hexadecimal. I thought binary was b'xxxxxxxx', where x =3D 0 or 1, but the compiler won't recognize it. Yes, I understand that the default is all inputs but I like to declare it. Just my style. The comment is wrong. It should have read //Set RC1 to high. Again the values are changed to hexadecimal. Again the comment was wrong. I know that the high bits aren't implemented but I still like to treat them as inputs. The comment should have read that RC0 is input. I used decimal values for the above because decimal values were used in the documentation where I got the settings for setting up the different ports and bits for digital or analog and for configuring the timer. The next four lines are incorrect in one manner or another. My decimal points are connected to RC1 to RC3 and the numbering system used is from 1 to 3. RC0 will be used for the ADC for controlling display time but not yet. So I had to correct the value of Reset and Gate so that RC1 would stay high and I could turn RC4 and 5 on and off with out disturbing RC1. This will be changed in later versions when I add the switches for controlling the Gating time and the DP position. But right now I just want to get the basics working. I have added comments to the code to try and clear this up. I have also added variables for controlling the gating time and the display time. It is true that you don't need a separate function for resetting the timer but again this is my style. In fact you could put 90+% of the code in to the main() function but would it be any better? I don't know. I established this style in HS and I have gotten high praise from instructors so I continue to use this style 40 years later. The compiler doesn't care. Everything from //Timer1 on down was given to me by the Microelektronika timer calculator except the PIR1bits.TMR1IF instruction. I actually changed the others to the shorter version. I missed this one. I will clean up the rest of this at a later time. The latest code is at the bottom of this reply. Thank-you everyone. I really do appreciate all of the help. I'm going to try and test the hardware later on today or tomorrow. And Yes, I do plan on changing the code to reset every 10mS but that will be later. Thanks a bunch, rich! On 7/22/2014 2:35 AM, James Cameron wrote: > On Tue, Jul 22, 2014 at 01:52:33AM -0500, Richard R. Pope wrote: >> James, >> I understand the confusion, so let's try this again. Here is all o= f >> the code except for the includes and configuration bits. >> I was able to change PIR1bits.TMR1IF to TMR1F. The timer tool from >> MicroElektronika doesn't use all of the proper flag names. This name was >> suggested to me by another person and since it works I used it. The >> other name works so I will use it because this is more consistent. >> You were correct about the void InitTimer1() not being called. I >> have added a call to this function. I also added a line showing the >> frequency of the crystal that I am going to use. > Thanks. I shall add review comments in the code, below each line that > causes my brain to explode. ;-) > >> Thanks, >> rich! >> >> // Crystal Freq =3D 20MHz > Typo in word Crystal. > >> //Global variables >> int count =3D 0; > This variable should be marked volatile. > >> void main(void) >> { >> >> PORTA =3D 0x00; // Turn porta off >> TRISA =3D 0xff; // Set Porta as all inputs > The decimal constant makes it harder to understand why all ports are > inputs; requires one to remember that 255 is 0xff. > > Both lines are redundant, the power on reset values for TRISA are all > bits set. > >> PORTC =3D 0x02; // Set RC1 to high > Comment does not match the constant. Only RC1 is high. > >> TRISC =3D 0xc1; // Only RC0 is input > 193 is 0xC1. Bits 7 and 6 are unimplemented, but you're writing them > high. So only RC0 is configured as input. > > Comment does not match the constant. Only RC0 is input. > >> CMCON0 =3D 7; // Turn off Comparators >> ANSEL =3D 16; // Set RC0/AN4 as analog >> InitTimer1(); // Set up the timer >> >> // Define commands >> int DP =3D 1; > This is RC0. > >> int Reset =3D 33 + DP; > This is 34 decimal, 0x22 hex, binary 100010. So by using this you are > setting RC1 and RC5. Very odd that you are adding 33 to 1 to get > this. Ask yourself what you intended to do here. > >> int GateOn =3D 17 + DP; > This is 18 decimal, 0x12 hex, binary 10010. So by using this you are > setting RC1 and RC4. Again, a very odd way to set a constant. > >> int GateOff =3D DP; > This is RC0. > >> while (1 =3D=3D 1) // Loop forever >> { >> PORTC =3D Reset; // Clear the Display >> PORTC =3D GateOn; // Start counting >> delay(1); // Count the frequency for one second =3D= HZ >> PORTC =3D GateOff; >> delay(15); // Display the frequency for 15 seconds >> >> } >> } // End NewControlCode.c >> >> void delay(int time) >> { >> int limit =3D time * 10 + 1; >> >> TMR1ON =3D 0; // Turn the timer off >> TMR1IE =3D 0; // Disable timer interrupts >> resettimer(); // Reset the timer to starting values >> count =3D 0; // Reset the timer interrupt counter >> TMR1IE =3D 1; // Enable the timer interrupts >> TMR1ON =3D 1; // Turn the timer on >> >> while(count < limit) >> { >> } >> return; >> } >> >> void resettimer(void) >> { >> TMR1H =3D 0x0B; // Reset the timer to roll over >> TMR1L =3D 0xDC; // at 100mS >> } > This doesn't really need to be in a separate function. > >> //Timer1 >> //Prescaler 1:8; TMR1 Preload =3D 3036; Actual Interrupt Time : 100 ms >> >> //Place/Copy this part in declaration section >> >> void InitTimer1(){ >> T1CON =3D 0x31; > The comment here should say you are setting the prescaler to 1:8 and > turning the timer on. > > While I agree that the prescaler should be set, I don't see why you > should turn the timer on here, since it is turned on in delay() function. > >> PIR1bits.TMR1IF =3D 0; > Another use of PIR1bits.TMR1IF that can be just TMR1IF. > >> TMR1H =3D 0x0B; >> TMR1L =3D 0xDC; > There's no reason to set these values here given that they are set by > resettimer() which is called by the delay() function. > >> TMR1IE =3D 1; > There's no need to set TMR1IE now, since it will be cleared and set in > the delay() function. You must choose where things are set, and > unless you have a good reason, set them in one place only. > >> INTCON =3D 0xC0; > When it boils down to it, InitTimer1() ends up being setting the > prescaler (T1CKPS0, T1CKPS1) and INTCON (GEI, PEIE). > >> } >> >> void Interrupt(){ >> if (TMR1IF){ >> TMR1IF =3D 0; >> TMR1H =3D 0x0B; >> TMR1L =3D 0xDC; > There's no need to reset the timer in the Interrupt() function, as it > will be reset in the delay() function. Clearing TMR1IF is enough. > >> count++; >> } >> } This is the latest version! // Crystal Freq =3D 20MHz // Global variables int count =3D 0; void main(void) { PORTA =3D 0x00; // Turn porta off TRISA =3D 0xff; // Set Porta as all inputs PORTC =3D 0x02; // Set RC1 to high TRISC =3D 0xc1; // Start with DP1 on, the rest of portc is inputs CMCON0 =3D 7; // Turn off Comparators ANSEL =3D 16; // Set RC0/AN4 as analog InitTimer1(); // Set up the timer // Define commands int DP =3D 1; // The decimal points are counted from 1 int Reset =3D 33 + DP; // Turn on RC5 int GateOn =3D 1 + DP; // Turn on RC4 int GateOff =3D DP; // Leave only the DP on int GateTime =3D 1; // The count time =3D 1 Second int DisplayTime =3D 15; // The display time =3D 15 seconds while (1 =3D=3D 1) // Loop forever { PORTC =3D Reset; // Clear the Display PORTC =3D GateOn; // Start counting delay(GateTime); // Count the frequency for one second =3D HZ PORTC =3D GateOff; delay(DisplayTime); // Display the frequency for 15 seconds } } // End NewControlCode.c void delay(int time) { int limit =3D time * 10 + 1; TMR1ON =3D 0; // Turn the timer off TMR1IE =3D 0; // Disable timer interrupts resettimer(); // Reset the timer to starting values count =3D 0; // Reset the timer interrupt counter TMR1IE =3D 1; // Enable the timer interrupts TMR1ON =3D 1; // Turn the timer on while(count < limit) { } return; } void resettimer(void) { TMR1H =3D 0x0B; // Reset the timer to roll over TMR1L =3D 0xDC; // at 100mS } //Timer1 //Prescaler 1:8; TMR1 Preload =3D 3036; Actual Interrupt Time : 100 ms //Place/Copy this part in declaration section void InitTimer1(){ T1CON =3D 0x31; TMR1IF =3D 0; TMR1H =3D 0x0B; TMR1L =3D 0xDC; TMR1IE =3D 1; INTCON =3D 0xC0; } void Interrupt(){ if (TMR1IF){ TMR1IF =3D 0; TMR1H =3D 0x0B; TMR1L =3D 0xDC; count++; } } -- http://www.piclist.com/techref/piclist PIC/SX FAQ & list archive View/change your membership options at http://mailman.mit.edu/mailman/listinfo/piclist .