 |
 |
| View previous topic :: View next topic |
| Author |
Message |
duncangray
Joined: 27 Apr 2007
Posts: 14
Location: UK
|
| Strange #int_ext behaviour |
 Posted: Fri Aug 19, 2016 3:24 pm |
 |
|
Good evening CCS forum.
I seem to write CCS code in small bursts every 6 months and, every time I do I end up using features I had never used before - and sometimes I get caught out!
The following code is being used to count precision 32.768kHz pulses (Macetech Chronodot) in timer1 - no overhead. There is an external interrupt int_ext(H_TO_L) generated by a Rigol DG4062 Function Generator (99% duty cycle high) on B0. To improve precision only every 20th (or 200th) interrupt is used. Timer1 will overflow but that doesn't matter - it's only the repeatability of the overflow I am interested in.
To minimise time spent in the int_ext service routine I started by using a print_flag which was set to TRUE in the isr every 20th time around. In "main" the programme is marking time but prints a single line with all the counters then resets the print_flag.
This code is interrupted at 10Hz dividing the interrupts by 20 correctly and toggles an LED on C4 properly but it frequently prints lines between the ones it ought to. int_count should always print as zero but, this version can print out several times on each line_count. There is no apparent pattern except that it always prints the int_count = 0 line - and it is the same if I interrupt at 100Hz and divide by 2000 as in example 2. It looks like the print_flag is being set to TRUE somewhere other than in the conditional part of the isr.
int_count = 0 line_count = 12 pulse_ count = 65535
int_count = 3 line_count = 12 pulse_ count = 65535
int_count = 9 line_count = 12 pulse_ count = 65535
int_count = 13 line_count = 12 pulse_ count = 65535
int_count = 0 line_count = 13 pulse_ count = 65535
| Code: | /* 7170 Calibrate - use Timer1 as counter of 32kHz pulses - count max_int_count
pulses of 7170 interrupt on port B. Print Timer 1 value to serial then
reset timer1 value and interrupt count value & start again.
*/
#include <16F767.h>
#fuses HS, NOWDT, BROWNOUT, BORV42, PUT // ,
#use delay(internal=8M)
#use rs232 (baud=9600, xmit=PIN_C6, rcv=PIN_C7, PARITY=N, BITS=8)
int8 dummy, line_count, print_flag;
int16 int_count; // interrupt count
int16 max_int_count = 20;
int16 timer1;
#int_ext
void ext_isr()
{
disable_interrupts(GLOBAL);
print_flag = FALSE;
dummy = input_b();
input(PIN_B0); // don't need both
int_count++; // counts the number of interrupts - rolls over at 255
if(int_count >= max_int_count){
output_toggle(PIN_C4); // just to prove it is interrupting
timer1 = get_timer1();
set_timer1(0);
line_count++;
int_count = 0;
print_flag = TRUE;
}
enable_interrupts(GLOBAL);
}
void main(void)
{
setup_timer_1(T1_EXTERNAL | T1_DIV_BY_1 );
ext_int_edge(H_TO_L);
enable_interrupts(INT_EXT);
int_count = line_count = 0;
timer1 = 0;
set_timer1(0);
print_flag = FALSE;
enable_interrupts(GLOBAL);
printf("\n\r");
while(1)
{
if(print_flag){
printf("int_count = %4lu line_count = %3u pulse count = %lu \n\r", int_count, line_count, timer1);
print_flag = FALSE;
}
}
}
|
The second version is performing the same function but the print line is in the isr - every 200th time the isr is entered. I thought that, as this means execution is in the isr for longer it was a bad idea but this code executes properly. It only prints 1 line every 2000 interrupts (at 100Hz) so printed lines read - 1 line every 20 seconds.
int_count = 2000 line_count = 90 pulse_ count = 1637
int_count = 2000 line_count = 91 pulse_ count = 1637
int_count = 2000 line_count = 92 pulse_ count = 1638
| Code: | /* 7170 Calibrate - use Timer1 as counter of 32kHz pulses - count max_int_count
pulses of 7170 interrupt on port B. Print Timer 1 value to serial then
reset timer1 value and interrupt count value & start again.
*/
#include <16F767.h>
#fuses HS, NOWDT, BROWNOUT, BORV42, PUT // ,
#use delay(internal=8M)
#use rs232 (baud=9600, xmit=PIN_C6, rcv=PIN_C7, PARITY=N, BITS=8)
int8 dummy, line_count, print_flag;
int16 int_count;
int16 max_int_count = 2000;
int16 timer1;
#int_ext
void ext_isr()
{
disable_interrupts(GLOBAL);
dummy = input_b();
input(PIN_B0); // don't need both
int_count++;
if(int_count == max_int_count){
output_toggle(PIN_C4); // just to prove it is interrupting
timer1 = get_timer1();
set_timer1(0);
line_count++;
printf("int_count = %4lu line_count = %3u pulse count = %lu \n\r", int_count, line_count, timer1);
int_count = 0;
}
enable_interrupts(GLOBAL);
}
void main(void)
{
setup_timer_1(T1_EXTERNAL | T1_DIV_BY_1 );
ext_int_edge(H_TO_L);
enable_interrupts(INT_EXT);
int_count = line_count = 0;
timer1 = 0;
set_timer1(0);
printf(" \n\r");
enable_interrupts(GLOBAL);
while(1)
{
if(print_flag){
print_flag = FALSE;
}
}
}
|
Sorry about the code formatting. I don't know how to get the indentation to work.
I appear to have done something stupid in implementing the print_flag but I have not been able to find it.
Please can someone with a fresh pair of eyes put me out of my misery.
Thanks very much in advance.
Duncan |
|
 |
temtronic
Joined: 01 Jul 2010
Posts: 9229
Location: Greensville,Ontario
|
|
| Posted: Fri Aug 19, 2016 4:42 pm |
|
|
1) use the code button(between quote and list ) before you paste code into your 'message body'. That should allow your code to look 'normal' and ident will work.
++++++++++++++
Fixed.
- Forum Moderator
++++++++++++++
2) no need to disable interrupts within and ISR, CCS takes care of that
3) never use printf within an ISR. It take way too much time, same holds true for doing 'math'. simply set 'flags' and get out fast.
4) I didn't see which compiler version you're using...
Jay |
|
|
duncangray
Joined: 27 Apr 2007
Posts: 14
Location: UK
|
|
| Posted: Fri Aug 19, 2016 4:54 pm |
|
|
Jay,
Compiler is 5.062. I realised almost as soon as I had posted that I had omitted that info.
Thanks for your comments. 1 & 2 were both new to me. I was aware that printf took loads of time in an isr but that is the version that works. My problem is that the print_flag version isn't working.
Duncan |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19518
|
|
| Posted: Sat Aug 20, 2016 1:46 am |
|
|
enable_interrupts(GLOBAL) in an ISR, is an absolute 'no no' on a PIC!....
Nothing to do with CCS. 
On the PIC, the _hardware_ automatically disables the interrupts when the ISR is called. There is a special 'return from interrupt' instruction (RETFIE - return from interrupt interrupt enable), which enables the interrupts the instruction _after_ the return. Problem is that if you enable the interrupts while you are still inside the handler, if the interrupt flag is set, the interrupt will be called from inside itself. Result corruption of the stored original registers. Ugh.....
Now on some processors, you do have to enable interrupts at the end of the handler. On these they work differently, and have it that the processor doesn't actually enable the interrupts till the _next_ instruction has completed. Hence the return is done before the enable.
Now some further comments:
dummy = input_b();
input(PIN_B0); // don't need both
Doesn't need _either_.....
Reading the port, is needed on INT_RB (interrupt on change), not on INT_EXT.
Now, the big thing that is problematical, is things changing.
Think about it. What is different about the 'in ISR' version?. The big thing is that the ISR can't trigger while you are in the ISR!...
Now problem is that all the values you are printing, are 'dynamic'. int_count, for example will change every time the interrupt triggers. Now printf, takes a long time, both to format the data, and to send it. So straight away, there are 12 characters printed, before the printf, even starts to format the int_count value. Basically 12mSec of time. Now if 'int_count', then changes (or any of the other variables), during the print, the result will be garbage... 
So:
| Code: |
* 7170 Calibrate - use Timer1 as counter of 32kHz pulses - count max_int_count
pulses of 7170 interrupt on port B. Print Timer 1 value to serial then
reset timer1 value and interrupt count value & start again.
*/
#include <16F767.h>
#fuses NOWDT, BROWNOUT, BORV42, PUT //'HS' is the external HS clock
#use delay(internal=8M)
#use rs232 (baud=9600, UART1, PARITY=N, BITS=8)
int1 print_flag=FALSE;
struct timer_struct {
int8 line_count;
int16 timer1;
int16 int_count;
} counts;
#define REQUIRED_COUNT 19
//Don't waste a variable for the required value
#int_ext
void ext_isr(void)
{
if (counts.int_count) //(non zero)
--counts.int_count;
else
{
counts.int_count=REQUIRED_COUNT;
output_toggle(PIN_C4); // just to prove it is interrupting
counts.timer1 = get_timer1();
set_timer1(0);
counts.line_count++;
print_flag = TRUE;
}
}
void main(void)
{
struct timer_struct local;
//duplicate set of timer variables.....
setup_timer_1(T1_EXTERNAL | T1_DIV_BY_1 );
ext_int_edge(H_TO_L);
enable_interrupts(INT_EXT);
counts.timer1=counts.line_count = 0;
counts.int_count=REQUIRED_COUNT;
set_timer1(0);
enable_interrupts(GLOBAL);
printf("\n\r");
while(TRUE)
{
if(print_flag)
{
//Now at this point, we need to 'snapshot' the registers
disable_interrupts(GLOBAL);
local=counts; //this copies all three counter variables
print_flag=FALSE;
enable_interrupts(GLOBAL);
//Now print the local copies....
printf("int_count = %4lu line_count = %3u pulse count = %lu \n\r", local.int_count, local.line_count, local.timer1);
}
}
}
|
Now the point here is that the instant 'print_flag' is seen as TRUE, the code disables the interrupts, and copies all three counter variables, into a 'local' copy. Then re-enables the interrupts. It is these 'local' copies that are then printed.
There are other ways of doing the copy (you can copy 'till' a variable doesn't change), but the point is that you have to ensure the variables are not going to change during the print....
Using a structure, so all three variables can be copied in just one code line, is just an easy way to do it. 
Using a count 'down' is just a rather easy way of doing counters like this.
There is another comment. Your 'supposed' printout from the 'in interrupt' version cannot be right. You never reset 'line_count', so it must increment between successive calls. The interrupt code can't be called again, unless line_count has incremented....
Line count should be going up between calls.
So your code is not actually running as you think. 
Probably what is actually happening, is the chip is resetting, because of the unbalanced stack caused by enabling the interrupt inside the interrupt, so the variables then get reset, and line_count goes back to zero..... |
|
|
duncangray
Joined: 27 Apr 2007
Posts: 14
Location: UK
|
|
| Posted: Sun Aug 21, 2016 1:16 pm |
|
|
Ttelmah,
Thank you very much for your help. I believe my disable interrupts & enable interrupts habit goes back to my Z80 assembler days. Point taken but it took some finding in the "Embedded C Programming" book. I still haven't found the bit about reading port B being unnecessary but I took those lines out as you suggested anyway - and will have to keep reading the book.
I removed the disable/enable in my print_flag version and it works exactly as your (much more elegant) code does but I was confused about your last few lines so I set out to be a bit more thorough.
In my 'in interrupt' version line_count does increment and print but your suggestion that the code was getting in a mess didn't ring true as, over 2000 interrupts (20 seconds@100Hz) of counting i.e. timer1 total of 655360, the overflow was completely repeatable at 1637/1638 i.e. a jitter of 1 in 2/3 of a million.
int_count = 2000 line_count = 68 pulse count = 1638
int_count = 2000 line_count = 69 pulse count = 1638
int_count = 2000 line_count = 70 pulse count = 1638
int_count = 2000 line_count = 71 pulse count = 1638
int_count = 2000 line_count = 72 pulse count = 1638
int_count = 2000 line_count = 73 pulse count = 1638
int_count = 2000 line_count = 74 pulse count = 1637
int_count = 2000 line_count = 75 pulse count = 1638
int_count = 2000 line_count = 76 pulse count = 1638
Could it be that the code was working as I had hoped but the length of the counting time is 2000 interrupts PLUS the time for the printf because I reset timer1 before the printf? I changed the baud rate and the overflow count changed proportionally i.e.38400 Baud had a quarter of the 1638 overflow that 9600 had. With a bit of arithmetic the printf takes roughly 50ms at 9600 Baud and 12ms at 38400.
So my count was wrong because the timing was slightly wrong but the Pic does not appear to be resetting or getting confused in any way that I can make out despite another interrupt occurring while it is in the interrupt service routine - with interrupts disabled. I assume normal operation starts again once it leaves the isr and interrupts are enabled again, once by me and once by the compiler!
You will be pleased to hear that your code worked perfectly at my first attempt and gave an overflow of zero (or 65535) and that the overflow increases/decreases by 5 or 6 for every 0.001Hz change in the interrupt frequency. It is extremely stable and will do exactly what I wanted for the job it was intended for. From here on - NO printing in the isr!
Once again - thanks very much for your (& Jay's) help and please keep up your advice to less experienced Pic programmers. I'm sure many would have given up with duff code if it wasn't for your timely help.
Duncan |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19518
|
|
| Posted: Sun Aug 21, 2016 1:44 pm |
|
|
You only showed three cycles of the 'working' code, and it did not look as if the line was incrementing as I'd expect.
Key place to look for details about reading the port, is the chip's data sheet.
Reading order needs to be something like:
1) A C book.
2) The chip data sheet.
3) The CCS manual.
4) The header file for your chip.
5) Then 'fuses.txt'. for what the fuses here mean.
6) The examples.
Updates to add the 'fuses' line. |
|
|
|
|
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
