 |
 |
| View previous topic :: View next topic |
| Author |
Message |
Jean FOUGERON
Joined: 30 Nov 2012
Posts: 110
Location: France
|
| UART stops answering (with Timer and INT_EXT) |
 Posted: Mon Jan 07, 2013 4:59 am |
 |
|
Hi friends
I wish you all a very good year 2013.
I have a problem with my project, that I do not know how to solve.
I use a PIC18F2520 running at 20Mhz
My application shares datas under ModBus protocol and I use INT_RDA (38400 bdx) for it.
It also is linked to a set, measuring delays between positive and negative edges on INT_EXT (# 14ms).
At least it also uses INT_TIMER1 (58ms) and INT_TIMER3 (250 µs) to proceed a reference Time base
The RS232 works OK, until ... it stops. I mean the INT_RDA does not fire after either 2mn or 1 hour, without explaination !
I am afraid of a coincidence betwen INT_EXT and _INT_RDA, but I'm not sure
I have added (via the Timer3) a watchdog which empties the RS232 buffer and reset my input routine, but no result.
Have anyone an idea to help me find a solution ? My customer of course seems in a hurry (always !)
Thanks for all |
|
 |
Jean FOUGERON
Joined: 30 Nov 2012
Posts: 110
Location: France
|
|
| Posted: Mon Jan 07, 2013 5:11 am |
|
|
Oh, I have forgotten to mention that I have a priority order :
#PRIORITY TIMER3,EXT
Timer3 to have a precise time base (it is not so precise anyway) and EXT to correctly measure.
I suppose that if a character arrives during these INTs, it is store in the 3 char buffer of the PIC and will not be lost ? |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Mon Jan 07, 2013 5:48 am |
|
|
Have you got 'ERRORS' in your RS232 declaration?.
This is _required_, unless you are handling UART errors yourself. Without it, if a UART overrun occurs, or a byte is received that is 'malformed', the UART _will_ stop receiving, till the error is cleared.
There is only a 1.9999 character buffer, not '3 characters'. The byte actually being received, and one stored character, till the 'moment' the received character is complete and wants to transfer to the buffer.
PRIORITY, only affects the order the interrupts are tested when the handler is called. It does not allow one interrupt to interrupt another (for this you need HIGH_INTS=TRUE, and to flag one interrupt as HIGH. However you can't use this, since you are using INT_EXT, and if this is enabled, INT_EXT is always a high priority interrupt (feature of the hardware....).
It almost certainly implies that a combination of one or more of the interrupts is taking too long in the handler for the RS232 to be handled. So long as your code can recover from a single character being lost, adding ERRORS should fix it.
Best Wishes |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Mon Jan 07, 2013 7:59 am |
|
|
As a further comment, if you are designing the hardware, then move whatever is connected to INT_EXT, to INT_EXT2.
Then add the #DEVICE HIGH_INTS=TRUE, and set timer3 as HIGH.
Then modify your priority statement (or the order you declare your interrupts, which does the same thing) to have INT_RDA first (don't include TIMER3 in this), so it'd be: #PRIORITY INT_RDA,INT_EXT,INT_TIMER1. Add ERRORS, and the chances of things working goes up a lot.
Look five times at the code in your interrupts. Repeat the mantra 'keep handlers short', for a couple of days, and see if you can reduce the timings involved. There are several 'caveat' ones that may get you - arithmetic avoid anything other than addition and subtraction in the interrupts. If you are using '%' in the serial handler, unless your buffer is a 'binary' size like 16 bytes, don_t. Use a test and load instead (several threads here about this). Be aware of how slow const accesses are, and array handling. Sometimes quite small changes can make big differences to execution. For instance, a 'switch' statement without a 'default', with several values, is much faster than the same statement with a default (the former uses a jump table).
Best Wishes |
|
|
Jean FOUGERON
Joined: 30 Nov 2012
Posts: 110
Location: France
|
|
| Posted: Mon Jan 07, 2013 8:25 am |
|
|
Thanks Ttelmah for your quick answer.
I already have #DEVICE HIGH_INTS=TRUE
As I need a precise counting with Timer3 I have #PRIORITY TIMER3,EXT
And #INT_TIMER3 high and #INT_EXT high (that you told me is unusefull, I maybe can suppress the "high")
The modbus query is 9 bytes long, includinc CRC16, it is not so long.
/**********************************************/
The INT_EXT duration is 250 s (it only set orn reset flags which are used in main afterward)
| Code: | #INT_EXT high
void Boucle()
{
L_Compteur_Flanc=L_Compteur;
Flanc_Pulse();
} |
with Flanc_Pulse :
| Code: | void Flanc_pulse()
{
if(I_Pas_de_Signal)
{
if(I_Pas_de_Signal==CYCLE_No_Signal) SFLS.Changed=TRUE;
I_Pas_de_Signal=0;
}
if(B_Rising) // flanc montant
{
if (Proximite(L_Compteur,I_Intervalle_14))
{
if(I_LED_en_cours<SFLS.length)
{
I_LED_en_cours++;
}
else
{
I_LED_en_cours=1; //pour si il n'y avait pas de pulse de 5 ms
}
LED(I_LED_en_cours, TRUE, VERT);
Clear_Alarme(I_Led_en_cours);
//SFLS.duree[I_LED_en_cours]=L_Compteur;
}
else
{
if(I_LED_en_cours>=SFLS.length)
{
// on est au début a SFLS.length, on le spécifiera à 5ms
I_LED_en_cours=0;
}
else
{
if(L_Compteur>I_Intervalle_14)
{
if(I_LED_en_cours<SFLS.length) //Soit c'est un défaut, soit on n'a pas vu le 15ms ?
{
int I,J,K;
// soit c'est une erreur
// ceux d'avant sont rouges
I=(L_Compteur+4)/I_Intervalle_14-1;
//I est le nombre de LED rouge à allumer
J=SFLS.length-I_LED_en_cours;
if (I>J) I=J;
//Affiche un nombre I de led rouge à partir de la led en cours
for(K=1;K<=I;K++)
{
Set_Alarme(++I_LED_en_cours);
//SFLS.duree[I_LED_en_cours]=I_Intervalle_14; // on simule un temps pour la moyenne
}
// celui là est vert à priori
LED(++I_LED_en_cours,TRUE,VERT);
}
}
else
{
//bizarre, plus court !
}
}
}
L_Compteur=0;
ext_int_edge(0,H_TO_L); //prochain mode d'interruption : descendant
B_Rising=FALSE;
set_timer3(L_Period); // réinitialise le compteur
}
else // flanc descendant
{
LED (I_LED_en_cours, FALSE, VERT); //éteindre la verte
if (Proximite(L_Compteur,I_Intervalle_5))
{
I_Intervalle_5=L_Compteur;
I_LED_en_cours=0; // même si on n'avait pas vu SFLS.last
}
else
{
if (Proximite(L_Compteur,I_Intervalle_10))
{
// normal
I_Intervalle_10=L_Compteur;
}
else
{
if (Proximite(L_Compteur,I_Intervalle_15))
{
//dernier
I_Intervalle_15=L_Compteur;
I_LED_en_cours=SFLS.length;
}
else
{
//erreur >15 ne devrait pas exister
}
}
}
ext_int_edge(0,L_TO_H); //prochain mode d'interruption : montant
B_Rising=TRUE;
}
L_Compteur_Flanc=0;
} |
/*********************************************/
The INT_RDA duration is 4µs
| Code: | #INT_RDA
void Receive()
{
char c;
c=getc();
if(I_UART_IN_POS==0 && c==I_NumeroEsclave) // on commence à prendre en compte si c'est bien la bonne adresse
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=FALSE;
}
else if(I_UART_IN_POS<UART_MAX)
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=(I_UART_IN_POS>=UART_MAX);
}
L_Cycle_UART=1;
} |
/******************************************************/
The INT_Timer1 is 440 µs
| Code: | #INT_TIMER1
void Affiche_Defaut()
{
int i;
Boolean B_Alarme;
if(++I_Timer1>2)
{
I_Timer1=0;
if(!I_Pas_de_Signal)
{
B_Alarme=FALSE;
for (i=0; i<SFLS.length;i++)
{
if(SFLS.defaut[i]>0) //défaut
{
if(SFLS.defaut[i]<I_DELAI_LAMPE_ALARME_MAX)
{
SFLS.defaut[i]++; // délai d'affichage
}
else
{
SFLS.defaut[i]=I_DELAI_LAMPE_ALARME_MAX;
LED(i+1, TRUE, ROUGE); // affichage
B_Alarme=TRUE;
}
}
else
{
LED(i+1, FALSE, ROUGE); // extinction
}
}
if(B_Alarme)
{
if(I_Delai_Alarme<I_DELAI_ALARME_MAX)
{
I_Delai_Alarme++;
if(SFLS.Alarm) {SFLS.Alarm=FALSE; SFLS.Changed=TRUE;}
}
else
{
I_Delai_Alarme=I_DELAI_ALARME_MAX;
if(!SFLS.Alarm) {SFLS.Alarm=TRUE; SFLS.Changed=TRUE;}
}
}
else
{
I_Delai_Alarme=0;
if(SFLS.Alarm) {SFLS.Alarm=FALSE; SFLS.Changed=TRUE;}
}
}
}
} |
/*****************************************************/
The INT_Timer3 (critical regularity) is 5µs
| Code: | #INT_TIMER3 high
void Comptage()
{
int I;
//doit donc battre le quart de milliseconde
L_Compteur++;
if(L_Cycle_UART && I_UART_IN_POS) L_Cycle_UART++;
if (L_Cycle_UART>CYCLE_250) // au bout de 250ms
{;
while(kbhit()) getc();
L_Cycle_UART=0;
I_UART_IN_POS=0; //répare l'UART
B_UART_OK=FALSE;
}
if(++L_Cycle_Seconde>CYCLE_1000) // toute les secondes
{
L_Cycle_Seconde=0;
if(++I_Cycle_5_Secondes>CYCLE_5) // au bout de 5 secondes
{
I_Cycle_5_Secondes=0;
}
if(++I_Pas_de_Signal>CYCLE_No_Signal) // Au bout de 3 secondes
{
I_Pas_de_Signal=CYCLE_No_Signal;
B_Pas_de_Signal=!B_Pas_de_Signal;
if (B_Pas_de_Signal)
{
for(I=2; I<SFLS.Length; I++) //problème si on commence I à 1
{
SFLS.defaut[I]=B_Pas_de_Signal;
LED(I++,B_Pas_de_Signal,ROUGE);
}
}
else Efface_Tout();
SFLS.Alarm=TRUE;
SFLS.Changed=TRUE;
}
}
set_timer3(L_Period);
} |
I will add ERRORS, try to understand how it works, and come back to you
Regards |
|
|
Jean FOUGERON
Joined: 30 Nov 2012
Posts: 110
Location: France
|
|
| Posted: Mon Jan 07, 2013 8:26 am |
|
|
| The INT_EXT duration is 250 µs (it only set orn reset flags which are used in main afterward) SORRY |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Mon Jan 07, 2013 9:34 am |
|
|
There is a potential logic problem I can see with:
| Code: |
while(kbhit()) getc();
|
Problem is that if kbhit has gone 'true' the INT_RDA flag _will_ have been set. When Timer3 then exits, INT_RDA will be called, and no character will be available.
I'd add:
| Code: |
while(kbhit()) {
getc();
clear_interrupt(INT_RDA);
}
|
Best Wishes |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Mon Jan 07, 2013 10:14 am |
|
|
Just a minor remark on your Timer3: | Code: | | set_timer3(L_Period); |
This line is at the end of your interrupt routine. Up till this point your ISR has executed a variable number of instructions, depending on the program flow. Also there is the fixed delay of about 30 instruction times required for saving all working registers before your interrupt handler is entered. And then there is the possible extra delay when another (high level) interrupt is already active and has to finish first.
All-in-all, the fixed time you set before the next timer interrupt is fired is not as exact as you believe it is.
Much better: | Code: | | set_timer3(get_timer3() + L_Period); |
Another tiny comment: | Code: | else if(I_UART_IN_POS<UART_MAX)
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=(I_UART_IN_POS>=UART_MAX); |
The last line is equal to |
|
|
Jean FOUGERON
Joined: 30 Nov 2012
Posts: 110
Location: France
|
|
| Posted: Mon Jan 07, 2013 10:48 am |
|
|
I understand the Timer3 duration topic
I so set_timer3(get_timer3() + L_Period);, I put set_timer3(L_Period); in the beginning of the procedure (I will re-calculate the correct value) Am I right ?
Regarding The last line is equal to
I think it is not the case. It is TRUE when I_UART_IN_POS>=UART_MAX
Suppose I_UART_IN_POS=2, I_UART_IN_POS++ << UART_MAX (which is 9)
So B_UART_OK will be TRUE when all the bytes will be received
No ?
Thanks and good evening |
|
|
asmboy
Joined: 20 Nov 2007
Posts: 2128
Location: albany ny
|
|
| Posted: Mon Jan 07, 2013 1:18 pm |
|
|
I might add that Ttelmahs suggestion is excellent based on what you show,
| Code: |
while(kbhit()) {
getc();
clear_interrupt(INT_RDA);
}
|
BUT!!!
IMHO is is a HUGE logical mistake to service the uart inside the TIMER ISR
and even with the fix proposed above -
this aspect alone could have hard to predict consequences, though my bet would be that THIS exact aspect is why the RDA ISR eventually fails.
Why DO you attempt to read the UART from the timer isr anyway?? is there that much latency in MAIN -if you try to take a high level action based on a uart mediated vector /command ? |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Mon Jan 07, 2013 1:39 pm |
|
|
| Jean FOUGERON wrote: | I understand the Timer3 duration topic
I so set_timer3(get_timer3() + L_Period);, I put set_timer3(L_Period); in the beginning of the procedure (I will re-calculate the correct value) Am I right ? |
If you did calculate the value for L_Period the first time instead of trial & error, then your original value will be OK. No need for recalculation, that's part of the beauty of this solution.
For example you had calculated a period time of 5000 ticks, then L_Period would have become 65536 - 5000 = 60536.
It doesn't matter where in your ISR you do set the Timer3 value. If it is early in the code, then for example the value of Timer3 will have increased to 10. Putting it at the end of the ISR the Timer3 value could have increased, for example, to 15 but this doesn't matter. Because you add your L_Period to the timer value it will overflow a bit sooner, but that is exactly what you want to.
| Quote: | Regarding The last line is equal to
I think it is not the case. It is TRUE when I_UART_IN_POS>=UART_MAX
Suppose I_UART_IN_POS=2, I_UART_IN_POS++ << UART_MAX (which is 9)
So B_UART_OK will be TRUE when all the bytes will be received
No ? |
You are right and I was wrong. B_UART_OK will be set to TRUE when you have received UART_MAX characters.
| asmboy wrote: | IMHO is is a HUGE logical mistake to service the uart inside the TIMER ISR
and even with the fix proposed above - |
I think this code is there only to restart the receiver in the case of a message timeout. A timer is started when the StartOfMessage character is seen and then 250ms later he restarts the receiver state machine. A little bit of inline documentation would have helped here. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Mon Jan 07, 2013 2:05 pm |
|
|
Key is that all of his interrupts take significantly longer than he thinks. It takes perhaps 7uSec to actually arrive at the interrupt code (for the 'high' interrupts), and at least a uSec longer for the 'low' ones. Then he gives 'times' for the interrupts, but this varies with the logic tests, and in some cases could be longer than he thinks.
The there are also some odd logic values that seem inverted. For instance:
| Code: |
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=FALSE;
}
else if(I_UART_IN_POS<UART_MAX)
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=(I_UART_IN_POS>=UART_MAX);
}
L_Cycle_UART=1;
|
Which sets 'B_UART_OK' to 'true' when the buffer is overflowing?....
Seems rather the opposite of logic.
There also doesn't seem to be anything actually ensuring the buffer does not physically overflow.
I also don't like a lot of his names. INSTR, is a function name in several languages to find one string in another, while he uses upper case in some places for variable names, and in others for presumably #defined values. There is a 'standard' in C, to reserve 'ALL CAPS' for #defined values. Minor, but it makes understanding the code harder.
Best Wishes |
|
|
Jean FOUGERON
Joined: 30 Nov 2012
Posts: 110
Location: France
|
|
| Posted: Tue Jan 08, 2013 1:59 am |
|
|
Dear Ttelmah
| Quote: | | it makes understanding the code harder |
: OK I will do an effort to satisfy
| Quote: | | Seems rather the opposite of logic. |
I do not understand what you mean. This routine works to | Quote: | | B_UART_OK will be set to TRUE when you have received UART_MAX characters |
. No ?
Yes ckielstra, | Quote: | | this code is there only to restart the receiver in the case of a message timeout |
because I cannot take the risk of no communication with the set.
The solution of Ttelmah to do | Code: | | #use RS232(BAUD=38400,BITS=8,PARITY=N,STOP=1,ENABLE=PIN_C5, UART1, ERRORS) |
is a super solution, as I tried this night and only had 105 retry of query in 15h26mn (1 query every 1s). Before I had a blocking situation (continuously retry) after 30' or 1h !
So maybe this makes the routine in Timer3 unusefull ?
I today will try without
| Code: | while(kbhit())
{
getc();
clear_interrupt(INT_RDA);
} |
expecting that the ERRORS parameter does the same (resetting UART)
(I also will do my best to improve | Quote: | | inline documentation |
)
Thanks to all of you for your help |
|
|
ckielstra
Joined: 18 Mar 2004
Posts: 3680
Location: The Netherlands
|
|
| Posted: Tue Jan 08, 2013 2:02 am |
|
|
| Ttelmah wrote: | | There also doesn't seem to be anything actually ensuring the buffer does not physically overflow. |
Yes, there is a protection against overflow but this is not marked as a possible error.
I do agree there is a strange logic in the code. Looking again at the INT_RDA, it will miss some messages. | Code: | c=getc();
if(I_UART_IN_POS==0 && c==I_NumeroEsclave) // on commence à prendre en compte si c'est bien la bonne adresse
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=FALSE;
}
else if(I_UART_IN_POS<UART_MAX)
{
INStr[I_UART_IN_POS++]=c;
B_UART_OK=(I_UART_IN_POS>=UART_MAX);
}
|
When it is out of sync and the first received character doesn't match the expected slave address (I_NumeroEsclave), then it will still save the received byte and increment I_UART_IN_POS. From that moment on all data will be read until the buffer is full or 250ms have passed.
Because of the 250ms time out I guess the system will recover eventually, but that requires a larger than 250ms gap. On a busy system there will be messages getting lost.
The message receive state machine needs to be redesigned. And it would be advisable to have the messages start with a unique start character so it will be easier to get 'in sync' again. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19338
|
|
| Posted: Tue Jan 08, 2013 3:29 am |
|
|
Yes. My problem is that without knowing the sizes involved, several things could potentially go wrong with the tests as implemented. Depends on the relationship between UART_MAX, the size of I_UART_IN_POS, and the size of the buffer. I have the nasty visualisation of buffers at 256 bytes, and then the counter will have wrapped before the test.....
The logic on B_UART_OK, is that in fact it is nothing to do with the UART, but in fact a 'message received' flag.
Best Wishes |
|
|
|
|
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
