 |
 |
| View previous topic :: View next topic |
| Author |
Message |
rovtech
Joined: 24 Sep 2006
Posts: 262
|
| Interrupt does not work with different PIC |
 Posted: Thu Feb 18, 2016 2:52 pm |
 |
|
The following program works as shown with a PIC16F722 chip.
If I physically change the PIC to a 16F1938 in my hardware, change the first line to #include <16F1938>, and run the Project Wizard to change the chip to 16F1938 and recompile, the program does not work and seems to hang in the ISR, the LED stops blinking, and the “Keypress=x” never appears. The Welcome appears and the LED blinks until I press a key on the 4x4 keypad so there is an interrupt.
If I reverse the procedure back to a 16F722 then the program works again.
PCM 5.019, Win7, ICD3
I am starting to suspect the compiler but I am no expert so any suggestions would be appreciated.
| Code: | //////////////////////////////////////////////////////////////////////////////////////////////
// File test.c //
// Digole Serial Adapter for 128x64 Graphic LCD. //
// Port B is used for keyboard data entry, Port A for Analog input and control, //
// Port C drives the display //
// RA4 drives an LED that is toggled every pass //
// Purpose is to test the Keyboard interrupt ISR //
// 18 Feb 2016 //
//////////////////////////////////////////////////////////////////////////////////////////////
/* Pre-processor directives */
#include <16F1938.H>
// #include <math.h>
#fuses INTRC_IO, NOWDT, PUT, NOPROTECT, BROWNOUT, MCLR
#use delay (clock=2000000)
#use I2C (master, SCL=PIN_C3, SDA=PIN_C4)
// #byte porta = 5
#byte portb = 6
#use fast_io(B)
#byte portc = getenv("SFR:PORTC")
// global variables
static char key; // character entered
// define I2C address
#define LCD_WRT_ADDR 0X4E // LCD display
#define buff_size 22 // characters per line plus one
// Function prototypes
void clear_LCD (void); // Clear LCD
void text_position (int line, int column); // set start for next text
void set_font (char size); // set font size
void send_str(char buff[buff_size]); // Send string to LCD
// Interrupt Service Routine
#INT_RB // any change on B4 to B7
void RB_isr(void)
{
// declare variables
byte KeyPad [4] [4] = {'1','2','3','U', // keypad array, row, col
'4','5','6','D',
'7','8','9','A',
'C','0','H','E'};
int key_press, new_kp, old_kp, row, col, count;
// debounce key
new_kp = portb ^ 0xF0; // get and mask new keypress
old_kp = new_kp; // store
if (new_kp == 0) // if not pressed
{
key = 0;
return; // return false
}
// adjust debounce here
for (count = 1; count <= 10; count++) // else confirm key still pressed
delay_cycles (250); // after 5 ms delay
new_kp = portb ^ 0xF0; // get and mask new keypress again
if (old_kp != new_kp) // if not legit
{
key = 0;
return; // return false
}
key_press = old_kp;
// Disable interrupts so release does not clear "key"
disable_interrupts (INT_RB); // until enabled in main
// Find the column. A switch in the matrix is holding one of the RB7 thru RB4
// inputs low by connecting to one of the RB3 thru RB0 outputs. Find which of
// RB7 thru RB4 is low.
switch (key_press)
{
case 0x10:
col = 0;
break;
case 0x20:
col = 1;
break;
case 0x40:
col = 2;
break;
case 0x80:
col = 3;
break;
default:
col = 0;
break;
}
// FIND THE ROW. Now find which of RB3 thru RB0 is connected by the switch.
// Each of RB3 thru RB0 are brought high in turn to find the row.
// The interrupts-on-change must be disabled or this ISR will be triggered
// by actions in the following.
output_high (PIN_B0); // scan for row 0
new_kp = portb ^ 0xF1;
if (new_kp == 0)
row = 0;
output_low (PIN_B0);
output_high (PIN_B1); // scan for row 1
new_kp = portb ^ 0xF2;
if (new_kp == 0)
row = 1;
output_low (PIN_B1);
output_high (PIN_B2); // scan for row 2
new_kp = portb ^ 0xF4;
if (new_kp == 0)
row = 2;
output_low (PIN_B2);
output_high (PIN_B3); // scan for row 3. Pin_B3 will be set low
new_kp = portb ^ 0xF8; // by the output_b(0xF0) statement below
if (new_kp == 0)
row = 3;
key = KeyPad [row] [col]; // Get character from array. Key = True
output_b (0xF0); // must leave outputs low
return;
}
// ************************************************************
// The main function
void main(void)
{
// setup ports
set_tris_b (0xF0); // 11110000
set_tris_c (0x00); // all outputs
output_b(0xF0); // outputs low
// setup interrupts
clear_interrupt(INT_RB);
enable_interrupts(INT_RB);
enable_interrupts(global);
port_b_pullups(0xF0); // pullups on inputs
// declare variables
char buff[buff_size];
// int i;
// setup display with welcome screen
clear_LCD(); // clear the LCD, sets default size,
sprintf(buff, "Welcome"); // place text string in buffer
text_position(3,0); // start first line, center
send_str(buff); // display text array
// Turn on LED
output_low(PIN_A4); // LOW = relay ON
// START Endless loop
while (1)
{
delay_ms (500);
output_toggle (pin_A4); // flash for testing
// Check for key press
if (key)
{
text_position(0,3); // start fourth line
sprintf (buff, "Keypress= %c", key); // send key to buffer
send_str(buff); // display text array
key = false; // clear the flag
delay_ms(500);
enable_interrupts(INT_RB);
enable_interrupts(global);
}
} // end of while loop
} // end of main function
// Functions
// Clear Display
void clear_LCD (void)
{
I2C_START (); // start I2C
I2C_WRITE (LCD_WRT_ADDR); // addr of LCD
I2C_WRITE ('C'); // C CL to clear display
I2C_WRITE ('L'); // L
I2C_STOP (); // stop I2C
}
// set position of next text
void text_position(int line, int column)
{
I2C_START (); // start I2C
I2C_WRITE (LCD_WRT_ADDR); // addr of LCD
I2C_WRITE ('T'); // T, TP set text position
I2C_WRITE ('P'); // P
I2C_WRITE (line); // line position
I2C_WRITE (column); // column position
I2C_STOP (); // stop I2C
}
// Set Font Size
void set_font (char size)
{
I2C_START (); // start I2C
I2C_WRITE (LCD_WRT_ADDR); // addr of LCD
I2C_WRITE ('S');
I2C_WRITE ('F');
I2C_WRITE (size);
I2C_STOP (); // stop I2C
}
// send string to LCD
void send_str(char buff[buff_size])
{
int i;
I2C_START (); // start I2C
I2C_WRITE (LCD_WRT_ADDR); // addr of LCD
I2C_WRITE('T'); // send TT for text coming
I2C_WRITE('T');
for (i=0; i<buff_size; i++)
{
I2C_WRITE(buff[i]); // start with a Z
}
I2C_WRITE(0);
I2C_STOP (); // stop I2C
}
// end[code][/code] |
|
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9216
Location: Greensville,Ontario
|
|
| Posted: Thu Feb 18, 2016 3:08 pm |
|
|
this...
#byte portb = 6
.. is wrong.
At least according to the datasheet I have PORTB is located at 00Dh, table 3-3 page 26.
What's _interesting_ to me is you have this...
#byte portc = getenv("SFR:PORTC")
... which is the preferred method of addressing addresses.
As you've just found out, not all PICs share a common, same register set.
What is happening is that your proram is NOT accessing PORTB instead it's looking at FSR1L.
Jay |
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Thu Feb 18, 2016 4:16 pm |
|
|
Wow! Ports A, B, and C have been on 5, 6, and 7 for all the chips I have ever used.
You are right but using #byte portb = getenv("SFR:PORTB") does not fix the problem.
It is strange that the analog and digital I/O on port A have been working with #byte porta = 5. I was trying to add an interrupt controlled keypad to a working project using the 16F1938 because it has EEPROM. I have noticed a drift in an ADC input.....
Any other suggestion? |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Thu Feb 18, 2016 4:36 pm |
|
|
beyond the obvious - i fear that the NEWish 16F1938 chip
and your older compiler may be an imperfect fit..
its the complex INIT for all the IO functions would be my bet -
something extra needing to be done for the 1938 .... |
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Thu Feb 18, 2016 5:05 pm |
|
|
| Thanks, I was thinking it was time to upgrade my compiler. I will let you know what happens. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9216
Location: Greensville,Ontario
|
|
| Posted: Thu Feb 18, 2016 7:00 pm |
|
|
re:
... I have noticed a drift in an ADC input.....
You should post a small program with JUST the ADC problem...
general comments though
ADC reference MUST be a stable supply( so NOT VDD)
The greater the bits the more 'unstable' the readings unless you're really good at PCB layout
Noise on VDD, supply lines can affect ADC readings if you use VDD as the ref.
EMI from bad PCB layout, no decoupling, wrong wiring/connections
Controlling relays(spikes, back EMF) cause bad reading
wrong selection for sample/hold capacitor
input sample to ADC is NOT stable.
'drift'? always one way ?? linear ?? based on time?? sampling rate??
Yeesh I could write a book on ADC design.Generally it's a process of elimination from observing a scope and following chip makers design specs( like bypass caps AT the chip ).
Jay |
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Thu Feb 18, 2016 7:41 pm |
|
|
I was just musing because I had the wrong address for port a but maybe it was never needed. I suspect the drift is the AD623 instrumentation Amp looking at a 30 amp shunt. They were bought cheap from China and I did a quick check that all showed zero by trying one at a time in the circuit without changing the output offset. They seemed good but occasionally I get an offset however the circuit is on a breadboard and wiggling wires sometimes eliminates it.
They are nice chips to use with a PIC because you can drive them from +/- 5v, feed them +/- mV (current) and have the output, in my case, as -30amps for 0v, 0amps for 2.5v, and +30amps for +5v so the ADC can handle it. The gain is set by a single resistor. You can't feed negative voltage into most PIC ADCs, at least not the ones I use. I use a resistor and the PIC diodes to protect the input but the input never gets to below zero anyway.
I trimmed pages of program to find out why the interrupt was hanging and just kept the minimum of code to simplify. The drift I see is not programming because it worked for years with a two op-amp version of the AD623A.
This is not my concern right now. I bought an update to my compiler but I can't download until tomorrow. I hope that solves the problem. If you got this far maybe you can explain a few things.
Why do I have to declare the address of a port? Why doesn't the compiler recognize something like portb or Port_B?
Why do I have to have enable_interrupts(INT_RB); as well as enable_interrupts(global); The program seems to need it but I really don't understand why. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19479
|
|
| Posted: Fri Feb 19, 2016 12:05 am |
|
|
This is one of the problems of 'confused edges' to the PIC families.
You have the 'nominal' boundaries (PIC12, 16, 18 etc.), but there are chips that blur over these edges. So there are PIC16's, that have the more limited PIC12 instruction set, and also newer PIC16's, that have extended abilities, and as a result have a lot more basic registers,and some of these result in ones being relocated.
With all of the PIC's, if you are locating registers yourself, the old line 'read the data sheet' applies.....
The compiler does recognise the ports without you having to locate anything. The input functions reads the port. It is just that you are bypassing the compiler's functions, and talking to the port directly.
However as has already been pointed out the compiler will allow you to setup the #byte commands using a port name, and then the compiler will automatically locate these for you.
On the interrupts, the chip has two control bits. There is an individual bit for each interrupt, and a 'master' (GLOBAL) one for all the interrupts. Both have to be set to enable the interrupts.
The reason. Well imagine you have 10 interrupts all enabled, and you wanted to disable them all for a particular operation. If you only had the individual enables, you would have to turn them all off one by one. Instead disabling the 'GLOBAL' bit turns them all off in one operation.
However then imagine you just had the GLOBAL control. If there were no individual enables, turning this on, would enable every interrupt, including ones you don't want.
So there are separate 'individual' enables, and then a 'master' enable as well.
There is also another thing. If (for instance) you were using interrupts to wake from sleep. Here the global enable does not actually have to be 'on'. Any interrupt which has it's individual enable on, can wake the chip. If you leave GLOBAL off, this will then wake the chip, but won't result in an interrupt handler actually being called. Saves a lot of wasted time. |
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Fri Feb 19, 2016 1:05 pm |
|
|
Thanks to all above.
I now have the latest compiler installed. It does not solve the problem but I now get two warnings. I cannot see the problem. Here they are with the relevant code from the complete program above (with the addition of the correct address for portb)
>>> Warning 240 "test.c" Line 143(12,16): Pointer types do not match
>>> Warning 240 "test.c" Line 158(14,18): Pointer types do not match
| Code: | #byte portb = getenv("SFR:PORTB")
139 // setup display with welcome screen
140 clear_LCD(); // clear the LCD, sets default size,
141 sprintf(buff, "Welcome"); // place text string in buffer
142 text_position(3,0); // start first line, center
143 send_str(buff); // display text array
154 if (key)
155 {
156 text_position(0,3); // start fourth line
157 sprintf (buff, "Keypress= %c", key); // send key to buffer
158 send_str(buff); // display text array
159 key = false; // clear the flag
160 delay_ms(500);
161 enable_interrupts(INT_RB);
enable_interrupts(global);
}
|
I am now very suspicious of my use of portb and wonder if it is being read correctly. It would account for the program getting lost in the ISR that uses portb 6 times. Should I be using input_b() as in:
| Code: | | new_kp = input_b() ^ 0xF1; |
if that is the correct way to do it? I don't think the pointer warnings are the problem but I would like to resolve them. |
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Fri Feb 19, 2016 1:16 pm |
|
|
OK I replaced the 6 portb with input_b() and the program now works (with the warnings).
An explanation of why portb does not work even when defined properly would be nice. Maybe the compiler still has a bug but I am more inclined to suspect my programming. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9216
Location: Greensville,Ontario
|
|
| Posted: Fri Feb 19, 2016 1:38 pm |
|
|
simply dump the listings for both the working and nonworking programs! Since you only made a slight change it should be obvious what is different.
Your program was hard for me to follow, thing like naming an array and a variable the same name (buff) is 'cloudy coding'. Self describing variable names makes it a lot easier. Also when using fast_io(), I like to have the tris() statements next in line though for 99% of the programs we've seen here standard_IO() works perfectly fine.
Also it might be better to put the port_b_pullups() before enabling the interrupt, maybe with say a 50ms delay, so to stabilze the inputs.
Jay |
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Fri Feb 19, 2016 2:01 pm |
|
|
Thanks Jay,
Where did I name buff a variable?
Should it always have the dimension attached as in
| Code: | | send_str(buff[buff_size]); |
somehow that did not seem right. I will try your suggestions and see if I can figure the portb problem.
I am not a programmer and appreciate the help on this forum.
The functions talk to an LCD display. |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Fri Feb 19, 2016 2:04 pm |
|
|
Change it to this, and the pointer warnings will go away:
| Code: |
void send_str(char *buff);
void send_str(char *buff)
{
}
|
|
|
|
rovtech
Joined: 24 Sep 2006
Posts: 262
|
|
| Posted: Fri Feb 19, 2016 4:43 pm |
|
|
Thanks PCM Programmer,
The pointer to the array. I had forgotten about that.
Yes that cured that problem and I will try to get to the other later although with input_b() it is probably the way I should be doing it and it works.
I did not think using buff[] to show it was an array would work. |
|
|
temtronic
Joined: 01 Jul 2010
Posts: 9216
Location: Greensville,Ontario
|
|
| Posted: Fri Feb 19, 2016 7:32 pm |
|
|
this...
143 send_str(buff); // display text array
I took 'buff' to be a single variable not an array of data
As PCM P pointed out the * pointer makes all the difference !
I like longer descriptive names like 'buffer_array' hoping to keep it organized in my mind as to what it is . I started programming when variable could only be 2 characters long and every line had a comment,even obvious ones!
Jay |
|
|
|
|
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
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
