![](templates/subSilver/images/CCSLogo.jpg) |
![CCS C Software and Maintenance Offers](templates/subSilver/images/forumAd6.jpg) |
View previous topic :: View next topic |
Author |
Message |
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Fri Feb 18, 2005 12:48 am |
|
|
I received my 16F88 and I was able to duplicate the problem.
It's a RMW problem. When the comparators are configured
in Mode 2, all the input pins (RA0-RA3) are set as analog inputs.
It doesn't matter what state the CIS bit is in, they are still configured
as analog inputs. You can override part of this by setting pin A3
as a digital pin with the ANSEL register. But the input latch for
pin A3 has a logic '0' on it, due to the analog-input configuration
and you can't change it. Because of this, if you read pin A3 you
will always read a logic '0'.
This causes a problem because when you write code like this,
you are doing a RMW on all pins of Port A. The PIC reads all
Port A pins into a temporary register. Then it modifies the
VHF_MOD bit (it sets bit 4 = logic high) in that register. Finally,
it writes the entire register to the Port A output latches.
But because it read a 0 on pin A3, it writes a 0 back to it.
That's why calling the vhf_chirp() function causes CS_GPS to go low.
Both of the lines of code, VHF_MOD = HIGH or VHF_MOD = LOW
do the RMW and will cause CS_GPS to go low.
The solution is to use a shadow register to hold the Port A bit values.
You update the shadow register and then write its contents to Port A.
I stripped your program down to the minimum necessary to show the
problem. In the code below, I have redefined the #bit variables, so
they are in a shadow register, instead of being directly on Port A.
I also declare a shadow register and initialize it to 0. There is also
a macro which I defined, in order to make it easier to update the code.
Every place in your code where you access a bit on Port A, must be
changed to use the new "update_Port_A()" function (macro).
For example, if you have
then you must change it to this:
Code: | update_Port_A(VHF_MOD, HIGH); |
You have to do this everywhere you access a Port A pin.
If you miss even one place, it'll do the RMW and set CS_GPS low.
Code: | #include <16F88.H>
#fuses INTRC_IO,NOWDT,PUT,NOPROTECT,NOBROWNOUT,MCLR,NOLVP,WRT
#use delay( clock=4000000 )
#ignore_warnings 203
#byte PORT_A = 0x05
int8 Shadow_Port_A = 0;
#bit REED = Shadow_Port_A.0 // 470K pullup.
#bit AD_IN = Shadow_Port_A.1 // Wired to COMP_IN
#bit COMP_IN = Shadow_Port_A.2 // and has a 1Meg pullup.
#bit CS_GPS = Shadow_Port_A.3
#bit VHF_MOD = Shadow_Port_A.4
#bit MCLR = Shadow_Port_A.5
#bit MORT = Shadow_Port_A.6 // pullup
#bit CS_RTC_READ = Shadow_Port_A.7
// This macro updates Port A bits via a shadow register.
#define update_Port_A(bit ,value) bit = value; PORT_A = Shadow_Port_A
#byte OSC_CON = 0x8F
#byte TRIS_A = 0x85
#byte COMREF = 0x9D
#byte ANSEL = 0x9B
#byte CMCON = 0x9C
//=========================================
void main()
{
OSC_CON = 0x62;
setup_comparator( A0_VR_A1_VR );
//set the voltage reference for about 1/2 way.
COMREF = 0b10000111;
TRIS_A = 0b01100111;
setup_adc_ports(sAN0 | sAN1 | sAN2);
update_Port_A(CS_GPS, 1);
//CS_GPS = 1;
delay_ms(3000);
update_Port_A(VHF_MOD, 1);
//VHF_MOD = 1; // This line makes CS_GPS go low
delay_ms(5000);
update_Port_A(CS_GPS, 1);
// CS_GPS = 1;
while(1);
} |
-----------------------------
I found a few other things in your code:
1. You have several places where you're doing this
Code: | PORT_A = PORT_A;
PORT_B = PORT_B; |
I'm not sure what your intention is, but the compiler optimizes that
code away. No assembly language instructions are generated.
You can see this in the .LST file.
2. According to the 16F88 data sheet, in the comparator ISR
you need to read the CMCON register and clear the CMIF bit
before you exit the ISR. The compiler inserts code to clear
the CMIF bit, but it doesn't take care of reading CMCON, so
you need to do that. The isr should be modified as shown
in bold below.
#byte CMCON = 0x9C // Put this above main()
#int_comp
void magnetic_interference()
{
char c;
OSC_CON = 0b01100010;
//The comparator has just been triggered. Set the MAGNET_ON
//to sync with the comparator. The reed switch is on A0, and that
//is comparator one. The pin will go low when the reed switch is
//activated, so the negative rail of the comparator will go below
//the positive rail, making C1 out low.
MAGNET_ON = C1OUT;
//CS_GPS = FALSE; (this was commented out by you)
c = CMCON; // Read the CMCON register before exiting
}
3. In your initialize() function, you have one line like this:
Code: | TRIS_A = 0b00100101; |
That's setting pin A1 to an output. But pin A1 is the Vin- pin
for Comparator #1. So I think it should be:
Code: | TRIS_A = 0b00100111; |
I did note that CCS is still using the obsolete TRIS instruction in
their built-in function, set_tris_a(), so I think it's a good idea to
set it manually as you're doing. I mean, TRIS may work, but
it's not in the published instruction set in the data sheet, so why
take the chance ? I've told them about this, but they don't
seem to want to change it. |
|
![](templates/subSilver/images/spacer.gif) |
theMagni
Joined: 21 May 2004 Posts: 48 Location: Victoria, BC
|
|
Posted: Fri Feb 18, 2005 11:48 am |
|
|
Well, wow.
Where did you learn that? That's certainly a solution I couldn't have come up with. That's just... fantastic. Thank you very much. Can I send you some beer or something?
As for the PORTA = PORTA thing, the intent was to let the external wiring set the logic levels on the pins after I write a value to them. In the interrupts, the intent is to clear the interrupt flag. The manual told me that any write or read of the port clears the flag. Those lines were in the old code that I inherited, and I assumed that that was why they were there. I graduated in June and was hired to sort out and organize all the code here. My assembly is really, really, really poor, so I don't check the LST files. If it doesn't generate code then I'll take the lines out.
The reason I set the A1 pin to an output was because I made a mistake. Thanks for catching that.
All this time I thought you worked for CCS. Huh. _________________ In the 90's, I promised myself I'd never use a so-called "smiley icon". I hate what I've become. ;) |
|
![](templates/subSilver/images/spacer.gif) |
|
|
You cannot post new topics in this forum You cannot reply to topics in this forum You cannot edit your posts in this forum You cannot delete your posts in this forum You cannot vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|