I am trying to implement message timeout for UART message, is it possible without global variables.
Two timeout detections i want to do, one is the complete message frame timeout within 10ms and another inter message byte timeout.
The frame format is
Start of Frame: 0x3A, Address, Data (this is variable but there is max limit of 10 Bytes), CRC, EOF.
As a starting point i want to detect the timeout of continuous messages, but the number of global variables are more, can i optimize or use some other method to avoid global variables.

uint8_t messagerxed  = FALSE;
uint16_t msgcounter=0;
uint8_t msgtimeout = FALSE;
int main(void)
{
 while(1);
}
/********* UART Interrupt ********/
UartInterrupt(void) {
messagerxed = TRUE;
msgcounter = 0;
msgtimeout = FALSE;
array[count++] = data;
}
/****** 1 ms Time Interrupt ********/
OnemsTimerInterrupt(void) {
if(messagerxed == TRUE) {
msgcounter++;
if(msgcounter == FIVEMS) {
 msgtimeout = TRUE;
messagerxed = FALSE; // reset the msgrxed flag
msgcounter = 0; // reset the msgcouter
}
}

 

You are committing a classic programming error in line 19:

if (msgcounter == FIVEMS) {

Do this instead:

if (msgcounter >= FIVEMS) {

This is an example where a program can pass all extensive testing and still have an unexpected bug waiting to happen.

Edit:
While I'm at it, here are some common examples where sleeping bugs lie:

1) comparing numbers with "is equal" operator (see above)
2) mistaking negative integers for positive integers and vice versa
3) integer underflow/overflow
4) divide by zero

 

As a starting point i want to detect the timeout of continuous messages, but the number of global variables are more, can i optimize or use some other method to avoid global variables.

How often do these messages arrive, and do you want to timeout before 1st message or only after?

Either way, a better approach, IMHO, is to set a timer running, say 5mS, then when each message starts or on each character, reset the timer. When the timer expires you have a 'message timeout' interrupt. No global variable needed except, of course, the handle for the timer, which could be local to the message handler class if implemented as a message object, or to the messagerxd task if implemented as a task.

 

I use UART interrupts to receive a character into a receive queue or transmit a character from a transmit queue. Some hardware can buffer more than one character at a time.

I use timer interrupts to count time, usually just one if high resolution is not required (as is the general case in this instance).

All other processes, packing/unpacking queues, timeouts, etc. are done in the asynchronous body of the program.

5 ms is eons for a modern MCU/CPU.

 

5 ms is eons for a modern MCU/CPU.

True, but at 9600bps, 5ms is roughly 5 characters and therefore a meaningful timeout. We don't know the baudrate (yet).

@Vihaan@123 On that note, to meet your timeout criteria, I'd start 2 timers, one for inter-character timeout at 1.5mS (or 1.5 to 2 x character time) and the second at 10mS for frame timeout. The first is reset every character, the second on receipt of the start of frame character.

If you need assistance in structuring that code, just ask

 

True, but at 9600bps, 5ms is roughly 5 characters and therefore a meaningful timeout. We don't know the baudrate (yet).

My point is there is plenty of time to process timeouts outside the interrupt.

So, we are in total agreement.

 

Thank you for the great inputs and i always get the exact advice i look for, so finally i have come up with the logic, request to please review, i am bit hesitant to Disable and Enable Uart Interrupts in while(1) and also tried to stop and start the timer. Regarding the baud rate i plan to go with 115200 if there are no issue else i will try to decrease it. I am assuming that the while(1) will be executed very fast and i can process all the messages before the next message arrives, so as of now holding the concept of circular buffer processing. Please suggest me for improvements / corrections, it is proto code only so i have not yet compiled. I have observed that there are multiple calls to starttimer() but i hope that will not be a problem.

#define MAX_MESSAGES             (10u)
#define MAX_CHARACTERS        (20u)

static volatile uint16_t timercharcounter=0; // Timeout variable for the character timeout
static volatile uint16_t timermsgcounter=0; // Timeout variable for the message timeout
static volatile uint8_t charcounter=0;   // Counter for characters received
static volatile uint8_t msgcounter=0;   // Counter for messages received
static volatile uint8_t FrameRxed = FALSE;

uint16_t Rs232Data[MAX_MESSAGES][MAX_CHARACTERS];  // Buffer for processing

int main(void)
{
   
    while(1) {
        if(FrameReceived() == TRUE){ // The framereceived function returns if the packet is successfully received
         DisbleUartInterrupt()  // To ensure while writing FrameRxed it shall not be updated in interrupt
                ResetFrameFlag();
         EnableUartInterrupt();   // Enable Uart Interrupt again.
            ProcessMessage();
            SendResponsePacket();
           
        }
           
    return 0;
}

UartInterrupt(void) {
    StartTimer();
    FrameData[charcounter] = TxBuf;
    ResetTimer_Char();
    if(FrameData[charcounter] == ':') {
        startPacket = TRUE;
    }
    if(startPacket == TRUE && FrameData[charcounter] == 0x0A) {
        if(FrameData[--charcounter] == 0x0D) {
            msgcounter++;
            FrameRxed = TRUE;
            ResetTimer_Frame();
            CopyToRs232Frame(FrameData, msgcounter,charcounter);
        }
       
    }
   
}

OneMsTimerInterrupt(void) {
    timercharcounter++;
    timermsgcounter++;
    if(timercounter >= ONEMS) {
        CharacterTimeout = TRUE;
        DisableTimer();
    }
    if(timermsgcounter >= FIVEMS) {
        CharacterTimeout = TRUE;
    }
}

void ResetTimer_Char(void) {
    timercharcounter=0;
}
void ResetTimer_Frame(void) {
    timermsgcounter=0;
}

void CopyToRs232Frame(FrameData, msgcounter,charcounter){
uint8_t index=0;
    while(!charcounter--) {
        Rs232Data[msgcounter][index] = FrameData[index];
        index++;
    }
}
uint8_t FrameReceived(void) {
    return FrameRxed;
}
void ResetFrameFlag(void) {
    FrameRxed = FALSE;
}

 

Thank you for the great inputs and i always get the exact advice i look for, so finally i have come up with the logic, request to please review, i am bit hesitant to Disable and Enable Uart Interrupts in while(1) and also tried to stop and start the timer. Regarding the baud rate i plan to go with 115200 if there are no issue else i will try to decrease it. I am assuming that the while(1) will be executed very fast and i can process all the messages before the next message arrives, so as of now holding the concept of circular buffer processing. Please suggest me for improvements / corrections, it is proto code only so i have not yet compiled. I have observed that there are multiple calls to starttimer() but i hope that will not be a problem.

Hmm , I can see where you are going with this but it's not quite what I was thinking. What is this going to run on?

 

Hmm , I can see where you are going with this but it's not quite what I was thinking. What is this going to run on?

It will be on dspic33EV controller (72MHz Cycle frequency). As you previously suggested i can run two timers.

 

It will be on dspic33EV controller (72MHz Cycle frequency). As you previously suggested i can run two timers.

That should be well fast enough. Before we go any further is this for a real application or is this a school/college assignment?

 

That should be well fast enough. Before we go any further is this for a real application or is this a school/college assignment?

It is a real application that is the reason I want to be very careful.

 

An interrupt service routine should do as little as possible.
Here is what the UART_RD_ISR needs to do:

1) capture the received character and store it in a buffer
2) test for end of message character and set data_available_flag
3) start hardware timer module

The timer module is configured to trigger an interrupt after M ms (or N timer clocks).
The timer interrupt sets timeout_flag.

Checking the two flags is done within the main process loop.

 

I'd start 2 timers...

3) start hardware timer module...

Dedicating 1 or 2 hardware timer resources to a single slow transmission channel is a huge waste of hardware, IMHO.

 

Dedicating 1 or 2 hardware timer resources to a single slow transmission channel is a huge waste of hardware, IMHO.

How do you time any process without a hardware timer?

 

How do you time any process without a hardware timer?

I use one free-running global system timer -- a heartbeat -- which can be quickly bit tested to time low-resolution events:

#include <xc.h>
#include "system_timer.h"
#include "system_timer_definitions.h"
#include "interrupts.h"
#include "power.h"

void __interrupt(irq(IRQ_TMR0), base(IVT_Base)) TMR0_ISR(void)
{
    TMR0IF = 0;                             //clear interrupt flag
    st_vtmrchg.bytev |= st_vtimer.bytev+1^(st_vtimer.bytev);  //accumlate changed bits
    st_vtimer.bytev++;                            //increment volatile timer
}

void ST_start(void)
{
    T0CON0bits.EN = 0;                      //turn off timer if running

    st_systim.wordv = 0;                    //reset timer registers
    st_tmrchg.wordv = 0;
    st_vtimer.bytev = 0;
    st_vtmrchg.bytev = 0;
  
    T0CON1 = system_timer_t0con1;           //set up oscillator/ sync / prescale
    T0CON0 = system_timer_t0con0;           //enable 8 bit timer and postscale
    TMR0H = system_timer_period;            //set tmr0 period
    TMR0L = 0;                              //clear any previous counts

    TMR0IP = 1;                             //set hi priority (for fast pid)
    TMR0IF = 0;                             //clear pending ints
    TMR0IE = 1;                             //and enable interrupts 
}

void ST_stop(void)
{
    T0CON0bits.EN = 0;                      //turn on timer
    TMR0IE = 0;                             //disable interrupts 

}

void ST_continue(void)
{
    T0CON0bits.EN = 1;                      //turn on timer   
    TMR0IE = 1;                             //and enable interrupts 
}

void ST_poll(void)
{
    st_tmrchg.bytev.hi = 0;
  
#ifdef TIMER0_SLEEP
  
//Sleep between interrupts
  
    di();
  
    if (PPFLAG_CAN_SLEEP && !st_vtmrchg.bytev)
    {
        Sleep();
        Nop();
    }

    PPFLAG_CAN_SLEEP = 1;
  
    ei();
  
#endif 
  
// Critical Section 

    TMR0IE = 0;

    st_systim.bytev.lo = st_vtimer.bytev;
    st_tmrchg.bytev.lo = st_vtmrchg.bytev;
    st_vtmrchg.bytev = 0;

    TMR0IE = 1;
  
// End Critical Section

    if (st_tmrchg.bitv.b7 && !st_systim.bitv.b7)
       st_tmrchg.bytev.hi = st_systim.bytev.hi^((st_systim.bytev.hi++)+1);
}

#ifndef SYSTEM_TIMER_H
#define    SYSTEM_TIMER_H

#ifdef    __cplusplus
extern "C" {
#endif

#include <xc.h>
#include "custom_types.h"
      
#define T0CON0_EN_Y             (0b1<<7)
#define T0CON0_EN_N             (0b0<<7)

#define T0CON0_MD16_16          (0b1<<4)
#define T0CON0_MD16_8           (0b0<<4)

#define T0CON0_POSTSCALE_16     0b1111
#define T0CON0_POSTSCALE_15     0b1110
#define T0CON0_POSTSCALE_14     0b1101
#define T0CON0_POSTSCALE_13     0b1100
#define T0CON0_POSTSCALE_12     0b1011
#define T0CON0_POSTSCALE_11     0b1010
#define T0CON0_POSTSCALE_10     0b1001
#define T0CON0_POSTSCALE_9      0b1000
#define T0CON0_POSTSCALE_8      0b0111
#define T0CON0_POSTSCALE_7      0b0110
#define T0CON0_POSTSCALE_6      0b0101
#define T0CON0_POSTSCALE_5      0b0100
#define T0CON0_POSTSCALE_4      0b0011
#define T0CON0_POSTSCALE_3      0b0010
#define T0CON0_POSTSCALE_2      0b0001
#define T0CON0_POSTSCALE_1      0b0000

#define T0CON1_CS_CLC1          (0b111<<5)
#define T0CON1_CS_SOSC          (0b110<<5)
#define T0CON1_CS_MFINTOSC      (0b101<<5)
#define T0CON1_CS_LFINTOSC      (0b100<<5)
#define T0CON1_CS_HFINTOSC      (0b011<<5)
#define T0CON1_CS_FOSC4         (0b010<<5)
#define T0CON1_CS_PIN_I         (0b001<<5)
#define T0CON1_CS_PINN_I        (0b000<<5)

#define T0CON1_ASYNC_N          (0b1<<4)
#define T0CON1_ASYNC_Y          (0b0<<4)

#define T0CON1_PRESCALE_32768   0b1111 
#define T0CON1_PRESCALE_16384   0b1110 
#define T0CON1_PRESCALE_8192    0b1101 
#define T0CON1_PRESCALE_4096    0b1100 
#define T0CON1_PRESCALE_2048    0b1011 
#define T0CON1_PRESCALE_1024    0b1010 
#define T0CON1_PRESCALE_512     0b1001 
#define T0CON1_PRESCALE_256     0b1000 
#define T0CON1_PRESCALE_128     0b0111 
#define T0CON1_PRESCALE_64      0b0110 
#define T0CON1_PRESCALE_32      0b0101 
#define T0CON1_PRESCALE_16      0b0100 
#define T0CON1_PRESCALE_8       0b0011 
#define T0CON1_PRESCALE_4       0b0010 
#define T0CON1_PRESCALE_2       0b0001 
#define T0CON1_PRESCALE_1       0b0000 
      
      
volatile ct_bitmap8 st_vtimer,st_vtmrchg;
ct_bitmap16      st_systim, st_tmrchg;

void ST_interrupt(void);  //Timer 0 interrupt handler
void ST_start(void);      //start system timer
void ST_stop(void);       //stop system timer
void ST_continue(void);   //continue timer after stop
void ST_poll(void);       //Main loop timer 0 polling call

#ifdef    __cplusplus
}
#endif

#endif    /* SYSTEM_TIMER_H */

#ifndef SYSTEM_TIMER_DEFINITIONS_H
#define    SYSTEM_TIMER_DEFINITIONS_H

#ifdef    __cplusplus
extern "C" {
#endif

#include "system_timer.h" 

#define TIMER0_SLEEP  //undefine to sleep between timer interrupts for power save 
  
// LFINTOSC -- 31Khz Base -- mod 31 -- 1ms time base

#define system_timer_t0con0 (T0CON0_EN_Y + T0CON0_MD16_8 + T0CON0_POSTSCALE_1) 
#define system_timer_t0con1 (T0CON1_CS_LFINTOSC + T0CON1_ASYNC_N + T0CON1_PRESCALE_1) 
#define system_timer_period (31-1);     //30 + 1 counts per ms

//To calculate time constants 
  
#define clctim(ms,msbase) ((unsigned long) ms*1000 / (unsigned long) msbase)


//Time Base Definitions [uS]

#define T1ms    1000
#define T2ms    2000
#define T4ms    4000
#define T8ms    8000
#define T16ms    16000
#define T32ms    32000
#define T64ms    64000
#define T128ms    128000

#define T256ms    256000
#define T512ms    512000
#define T1s     1024000
#define T2s     2048000
#define T4s     4096000
#define T8s     8192000
#define T16s    16384000
#define T32s    32768000


//Timer 0 Bit Constant Definitions

#define TB2ms    st_systim.bitv.b0 
#define TB4ms    st_systim.bitv.b1
#define TB8ms    st_systim.bitv.b2
#define TB16ms    st_systim.bitv.b3
#define TB32ms    st_systim.bitv.b4
#define TB64ms    st_systim.bitv.b5
#define TB128ms st_systim.bitv.b6
#define TB256ms    st_systim.bitv.b7

#define TB512ms    st_systim.bitv.b8
#define TB1s    st_systim.bitv.b9
#define TB2s    st_systim.bitv.b10
#define TB4s    st_systim.bitv.b11
#define TB8s    st_systim.bitv.b12
#define TB16s    st_systim.bitv.b13
#define TB32s    st_systim.bitv.b14
#define TB64s    st_systim.bitv.b15

//Timer 0 Change Constant Definitions

#define TC1ms    st_tmrchg.bitv.b0
#define TC2ms    st_tmrchg.bitv.b1
#define TC4ms    st_tmrchg.bitv.b2
#define TC8ms    st_tmrchg.bitv.b3
#define TC16ms    st_tmrchg.bitv.b4
#define TC32ms    st_tmrchg.bitv.b5
#define TC64ms    st_tmrchg.bitv.b6
#define TC128ms st_tmrchg.bitv.b7

#define TC256ms    st_tmrchg.bitv.b8
#define TC512ms    st_tmrchg.bitv.b9
#define TC1s    st_tmrchg.bitv.b10
#define TC2s    st_tmrchg.bitv.b11
#define TC4s    st_tmrchg.bitv.b12
#define TC8s    st_tmrchg.bitv.b13
#define TC16s    st_tmrchg.bitv.b14
#define TC32s    st_tmrchg.bitv.b15


#ifdef    __cplusplus
}
#endif

#endif    /* SYSTEM_TIMER_DEFINITIONS_H */

 

I would imagine that the system timer uses HW timer module and has huge SW overhead.
Using a timer module directly uses no software except for setting the timeout_flag. You cannot get more basic than that.

 

I would imagine that the system timer uses HW timer module and has huge SW overhead.
Using a timer module directly uses no software except for setting the timeout_flag. You cannot get more basic than that.

I've included the code. The entire timer interrupt consumes exactly 5 instructions every rollover period (in this case, 1 ms), and supports putting the processor to sleep automatically between events.

From this, I can time multiple events in userspace with multiple time-bases from 1ms to 32s with a simple one instruction bit test.

For example, blinky LED:

LED_LATCH_BIT = TB512ms;

TB512ms is 0 for 256 ms, 1 for 256 ms, etc. (512ms period).

To trigger events:

if (TC64ms) ...

TC64ms is set (and automatically cleared on the next loop) every 64ms.

Nearly zero overhead and incredibly efficient for low-resolution events in userspace.

Note that this does not work for those who use blocking hardware functions.

 

It seems like most of my abandoned blog entries disappeared (including the attachements), but this one summarizes my system timer -- the same timer I have used for (nearly?) 40 years of coding firmware.

 

Sure, you can measure time intervals by just registering clock ticks. But what the TS wants can be implemented with one simple timer. My STM32 has 17 timers so there are lots to spare.

 

Sure, you can measure time intervals by just registering clock ticks. But what the TS wants can be implemented with one simple timer. My STM32 has 17 timers so there are lots to spare.

Differences of opinion make a horse race.

Besides, I expect that my code ultimately will be doing much more than controlling a single communication channel.