Implementing a timeout "wrapper"?

Go To Last Post
8 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm trying to clean up some code on an AVR project that uses a lot of timeouts for communication over both a CAN bus and a UART connection. The timeout code is blocking and pretty simple:

void func(void) {
    uint16_t tmr = TIMEOUT_DURATION_IN_MS;
    bool terminating_condition = false;

    // Wait for reply with timeout
    while (tmr && !terminating_condition) {
        // Do stuff (check Serial, poll CAN controller, etc.)
        // Set terminating_condition once proper reply is received

        // 1 ms delay
        _delay_ms(1);

        // Decrement timeout counter
        tmr--;
    }

    // Check if we timed out
    if (!tmr) {
        // Handle timeout accordingly
    }
}

Since I have these timeout code blocks all over my program, things have gotten pretty bloated/messy and I'm trying to see if there's a simple/elegant way to create a "wrapper" of sorts (maybe with function pointers, macros, etc.) to clean things up a bit. This is what I've come up with so far:

/**
 * @param condition termination condition to exit loop
 * @param duration  timeout duration in milliseconds
 */
#define TIMEOUT_START(condition, duration) \
    uint16_t tmr = duration; \
    while(tmr && !condition) {

/**
 * @param result true if a timeout occurred
 *               false otherwise
 */
#define TIMEOUT_END(result) \
        _delay_ms(1); \
        tmr--; \
    } \
    result = (tmr == 0);

// Example usage
void func(void) {
    bool terminating_condition = false;
    bool res;

    TIMEOUT_START(condition, TIMEOUT_DURATION_IN_MS)

    // Do stuff (check Serial, poll CAN controller, etc.)
    // Set terminating_condition once proper reply is received

    TIMEOUT_END(res)

    if (res) {
        // Handle timeout accordingly
    }
}

While this code does work, it feels kinda hacky and I don't think it's the best way to simplify all my timeout code blocks. Is there a better way to achieve what I'm trying to do?

This topic has a solution.
Last Edited: Mon. Aug 17, 2020 - 03:35 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Greetings and Welcome to AVR Freaks -

 

Since your timeouts are all in the range of multiple milliseconds, I would be tempted to make a "1ms ticker" with a timer and a little state machine that manages the timeouts. That way, there would not be any blocking, which surely must be one of your challenges.

 

Jim

 

Until Black Lives Matter, we do not have "All Lives Matter"!

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the quick reply Jim!

 

Right now, the blocking isn't a huge issue because my program is pretty sequential. Basically my AVR micro is coordinating the actions of several peripheral devices over CAN and UART as they occur in real time in an automated system. While the micro waits for a peripheral to complete its task (i.e. run a motor, poll a sensor, etc.), it doesn't need to be doing anything. The purpose of the timeouts is mostly to detect if a peripheral has unexpectedly gone offline or if there's a problem with the communication infrastructure.

 

I just don't like setting up these big timeout while loops every time I need to wait on a reply from a peripheral. It's making my code bloated and hard to follow. I was hoping there was some way to condense this into a reusable code snippet since the structure and variables are the same for every timeout code block.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would resist creating defines/macros as you end up in the same boat as before, only now you cannot even see what is taking place. If you do not have a system time of some sort, I guess the _delay_ms is going to be needed (wasting time doing nothing is not ideal, but may not matter). You could also use _delay_us to keep the time between waiting and doing useful code to a minimum, if it matters (or just use a raw number for the count without delays, and switch to a uint32_t if needed).

 

bool timeout(uint16_t ms){ //0=check, !0=set
    static uint16_t tmr;
    if( ms ) tmr = ms; //if !0, set tmr
    if( tmr ){ tmr--; _delay_ms(1); } //dec, delay
    return tmr == 0; //timeout?
}

 

void func(void) {
    timeout(TIMEOUT_DURATION_IN_MS);

    bool done = false;
    while( !timeout(0) && !done ){
        //do stuff, set done to escape
    }
    if( !done ) { /* was timeout */ }
}

 

Last Edited: Sun. Aug 16, 2020 - 08:58 PM
This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Another possible approach to is to have a generic 'check_external_status' function that accepts two arguments, (1) a pointer to a helper function that is used to determine whether some external task has completed or not, and (2) the timeout in ms.

uint8_t check_external_status_with_timeout(bool (*check_func)(void), uint32_t timeout_ms);

You could set the return value to an enum of possibilities (task completed within timeout, task timed out, error, etc, etc).

 

You can then write a number of these helper functions, one for each external status you need to check.

bool is_task_complete_1(void);
bool is_task_complete_2(void);

Change bool to uint8_t if you want to signal more than a simple yes/no complete.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

>Since I have these timeout code blocks all over my program,

 

And maybe rethink why all the blocking is needed. If you are polling for everything, then maybe there is no choice but maybe there are alternatives.

 

 

int16_t readUsart0(){ return (UCSR0A & 0x80) ? UDR0 : -1; }

 

int16_t readUsart0_timeout(uint16_t ms){

   timeout(ms);

   int16_t ret;

   while( ret = readUsart0(), !timeout(0) && (ret < 0) );

   return ret;

}

 

//now functions can choose a timeout version to call

void func(){

  int16_t c = readUsart0_timeout(TIMEOUT_DURATION_IN_MS);

  if( c < 0 ){ /* timeout */ }

}

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

However you do it (and i think I wouldn't bother trying to over think it, simple is usually good),

looking at the logic in post #1,

once the terminating condition is true, why wait another ms before exiting the loop ?

if the terminating condition is true, but tmr == 1, tmr will be decremented before exiting the loop and look like it timed out.

Likwise, if the terminating condition isn't true, and tmr == 1, you are then waiting another ms just to exit the loop ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for everyone's replies! I really liked obdevel's suggestion of using a ' check_external_status' function that uses function pointers and timeouts, so I'll give that a try and see where it takes me.

 

I think I may have to do a good bit of restructuring to my code before things get too out of hand though. I'm now realizing there are instances where I could end up with nested timeouts where a timeout loop for one communication link (e.g. CAN) gets held up by communication on another link (e.g. UART), which is far from ideal... So looks like it's back to the drawing board!

 

Thanks again everyone!