 |
 |
| View previous topic :: View next topic |
| Author |
Message |
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
| INT_RDA fail to receive randomly |
 Posted: Sat Nov 29, 2014 1:06 pm |
 |
|
Hi,
In the code everything compile and work fine but I have a problem with the serial interrupt.
Often it miss chars. Shouldn't be like this.
I always keep my ISR the KISS way. No delays/long routine inside them whatsoever.
| Code: |
#include <18F2550.h>
#fuses HSPLL,PLL5,CPUDIV1,NOMCLR,NOLVP,BROWNOUT,BORV43,PUT,NOWDT,NOVREGEN
#use delay(clock=48000000)
#use rs232(baud=9600, xmit=PIN_C6,rcv=PIN_C7,errors)
#define runled PIN_A0
#define dataled PIN_A1
#define faultled PIN_A2
#define ONE_WIRE_PIN PIN_A4
#define door_pin PIN_A5
#define waste_pin PIN_A3
#define lasereye_pin PIN_C0
#include "ds1820.h" //temp sensor stuff
int1 sendflag = 0;
signed int8 temp;
unsigned int8 dummy,count,savedcount;
unsigned int1 door,waste,lasereye;
//*****************************************************************************
char version[16] = "1.0 29/11/2014"; /// change when releasing a new version !!!!!! <<<---
//*****************************************************************************
#int_timer0
void timer0_isr()
{
output_toggle(runled);
if (sendflag == false){sendflag = true;}
count++;
output_low(dataled);
}
#int_rda
void serial_isr()
{
dummy=getc();
output_toggle(dataled);
}
void sendpacket() //Start char $ and 0x02, sensor temp,door status,waste status,laser status, 8-bit CRC
{
unsigned int8 CRC;
CRC= temp^door^waste^lasereye; //xor everything
putc('$');
putc(0x02);
putc(temp);
putc(door);
putc(waste);
putc(lasereye);
putc(crc);
}
void main()
{
setup_timer_0(T0_INTERNAL|T0_DIV_128);
enable_interrupts(INT_TIMER0);
enable_interrupts(INT_RDA);
enable_interrupts(GLOBAL);
while(true){
if (sendflag == true) { // packet sending
temp = ds1820_read();
door = input(door_pin);
waste = !input(waste_pin);
lasereye = !input(lasereye_pin);
sendpacket();
sendflag = false;
}
switch (dummy)
{
case '0' : output_b(0x00);//turn all used relay pins in a Low state
dummy = 0;
break;
case 'A' :output_high(pin_b0); // High Relay K0
dummy = 0;
break;
case 'a' :output_low(pin_b0);
dummy = 0;
break;
case 'B' :output_high(pin_b1); // High Relay K1
dummy = 0;
break;
case 'b' :output_low(pin_b1);
dummy = 0;
break;
case 'C' :output_high(pin_b2); // High Relay K2
dummy = 0;
break;
case 'c' :output_low(pin_b2);
dummy = 0;
break;
case 'D' :output_high(pin_b3); // High Relay K3
dummy = 0;
break;
case 'd' :output_low(pin_b3);
dummy = 0;
break;
case 'E' :output_high(pin_b4); // High Relay K4
dummy = 0;
break;
case 'e' :output_low(pin_b4);
dummy = 0;
break;
case 'F' :output_high(pin_b5); // High Relay K5
dummy = 0;
break;
case 'f' :output_low(pin_b5);
dummy = 0;
break;
case 'G' :output_high(pin_b6); // High Relay K6
dummy = 0;
break;
case 'g' :output_low(pin_b6);
dummy = 0;
break;
case 'H' :output_high(pin_b7); // High Relay K7
dummy = 0;
break;
case 'h' :output_low(pin_b7);
dummy = 0;
break;
case '?' :printf("$!*"); //keep alive
savedcount = count;
dummy = 0;
break;
case '@' :printf("$%s*",version); // version info
dummy = 0;
break;
}
//! if ((count - savedcount) > 3 ) //if keep alive fail reset relay
//! {
//! output_b(0x00);
//! output_high(faultled);
//! count = 0;
//! savedcount = 0;
//! }
}
}
|
I don't know why the line temp = ds1820_read(); make the ISR miss somes rx chars.
When I commented this line I receive 100% of the serial chars.
I don't understand why the ds1820 routine make the serial ISR go bad.
Delays in the main loop should NOT affect the ISR in any way but clearly in this case the few delay_us required for the DS1820 driver make the ISR miss some chars.
Do I need a buffer or something for a single character check (dummy switch)?
Thank you very much!
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
 |
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Sat Nov 29, 2014 2:12 pm |
|
|
The problem is simple.
Time.
Look at how long the DS1820 routines take. They are _slow_. Problem is that all the time you are handling this, if _two_ characters arrive, the first will be lost with your code. The character _is_ received fine, but gets overwritten by the second character...
Go fractionally more complex in your service routine, and use a small ring buffer (just a handful of characters). |
|
|
newguy
Joined: 24 Jun 2004
Posts: 1903
|
|
| Posted: Sat Nov 29, 2014 3:47 pm |
|
|
Ttelmah is right. The DS1820 is very slow to read/interact with, particularly if you wrote the interface code in a blocking manner.
I use that sensor on all the boards I've designed and I've come up with a method to read that chip that doesn't block. I can't share the code but I can tell you how to do it.
When you want to read temperature, initialize a timer and simultaneously set the DIO line low. Set the timer to correspond to the 1820's initial reset pulse length. When the timer's interrupt fires, set the line high and reload the timer for the next time quanta in the grand scheme. Proceed in this manner for the entire interaction sequence, substituting a DIO read instead of a set when you're reading the sensor. At the end of the entire sequence, disable the timer.
Voila. A non-blocking DS1820 interaction scheme. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Sun Nov 30, 2014 2:27 am |
|
|
Agreed.
In his case though the DS code is not actually 'blocking', but his receive interrupt has no buffering. So if a second character is received before he handles the first, 'data loss'.... :(
This is where small buffers are so useful.  |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Sun Nov 30, 2014 8:52 am |
|
|
One coda to that.
Everything is fine if you use _binary_ sizes for the buffer (2,4,8,16 etc..). However there is a problem if you want a 'non binary' size (3,5,6,7 etc.). With the binary size, the compiler codes x % y, using the & operator, so it only takes a single instruction. However if you use a non binary size, it has to use a division. This results in divisions having interrupts disabled in the main code, and is relatively large/slow.
So for 'non binary' buffer sizes, change this:
| Code: |
in = (++in % size);
|
to:
| Code: |
if (++in==size)
in=0;
|
and the same when incrementing the 'out'.
This is small (only one instruction more than the '&' solution), and works for all buffer sizes.
It's a thing that CCS ought to warn about in the example. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
| Posted: Sun Nov 30, 2014 9:24 am |
|
|
Thanks Ttelmah!
I'll stick to binary size buffer (easier for computing).
I don't see any empty slot in this example.
A true circular buffer must have an empty slot to determine end / start location right?
Otherwise it's hard to know when the buffer is full or empty...
| Quote: | | The CCS code handles this, by setting 'in' back to the last value if this happens, throwing away the newest character. |
I find that odd that it overwrite by default the newest values. Isn't the point of a ring buffer to make a circle and erase older values first?
_________________
Regards,
Laurent
-----------
Here's my first visual theme for the CCS C Compiler. Enjoy! |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Sun Nov 30, 2014 10:17 am |
|
|
You don't have to have an empty slot.
If the increment will make the 'in' == 'out', then the buffer is full.
If 'in' == 'out' then the buffer is empty.
You can code to throw away the oldest. If the buffer is full, you increment 'next_out', to lose the oldest character. Realistically the point is that if the buffer is sensibly sized you shouldn't overflow.
If you want, you don't even have to code the buffer. You can let the compiler do it 'for you':
| Code: |
#use rs232(BAUD=9600,UART1 ,ERRORS, RECEIVE_BUFFER=8)
//Then don't code an INT_RDA, or enable it (the compiler will do this)
//Then replace the switch, with:
if (kbhit())
{
switch (getc())
//and test as required
}
else
{
//here do your 'no character' stuff
}
|
The compiler will automatically generate a receive ISR, enable it, and automatically change 'kbhit' to test the status of this, instead of the physical port. |
|
|
ELCouz
Joined: 18 Jul 2007
Posts: 427
Location: Montreal,Quebec
|
|
|
jeremiah
Joined: 20 Jul 2010
Posts: 1322
|
|
| Posted: Mon Dec 01, 2014 9:26 am |
|
|
As a note:
Unless it has changed in the last half year or so, the built in buffer that Ttelmah mentioned does have a noteable downfall in that if you ever fill the buffer it completely starts over, losing the current contents of the buffer. Instead of doing the pre increment check for next_in == next_out, it increments it regardless, resetting the buffer size back to 0 (and then to 1 for the next byte that comes in). |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Mon Dec 01, 2014 10:09 am |
|
|
It still does.
I pointed this out here, and to CCS right when they first launched it.
It is a downside, and rather silly.... |
|
|
|
|
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
