6
\$\begingroup\$

My background for this review is a university course in embedded systems using MicroC and this question. Now my program appears to run ok, but I'd like to know what you think can be improved or if the solution I've done so far is not acceptable for some reason?

#include <stdio.h>
#include "system.h"
#include "includes.h"
#include "altera_avalon_pio_regs.h"
#include "sys/alt_irq.h"
#include "sys/alt_alarm.h"

#define DEBUG 1

#define HW_TIMER_PERIOD 100 /* 100ms */

/* Button Patterns */

#define GAS_PEDAL_FLAG      0x08
#define BRAKE_PEDAL_FLAG    0x04
#define CRUISE_CONTROL_FLAG 0x02
/* Switch Patterns */

#define TOP_GEAR_FLAG       0x00000002
#define ENGINE_FLAG         0x00000001

/* LED Patterns */

#define LED_RED_0 0x00000001 // Engine
#define LED_RED_1 0x00000002 // Top Gear

#define LED_GREEN_0 0x0001 // Cruise Control activated
#define LED_GREEN_2 0x0002 // Cruise Control Button
#define LED_GREEN_4 0x0010 // Brake Pedal
#define LED_GREEN_6 0x0040 // Gas Pedal

/*
 * Definition of Tasks
 */

#define TASK_STACKSIZE 2048

OS_STK StartTask_Stack[TASK_STACKSIZE]; 
OS_STK ControlTask_Stack[TASK_STACKSIZE]; 
OS_STK VehicleTask_Stack[TASK_STACKSIZE];

// Task Priorities

#define STARTTASK_PRIO     5
#define VEHICLETASK_PRIO  10
#define CONTROLTASK_PRIO  12

// Task Periods

#define CONTROL_PERIOD  300
#define VEHICLE_PERIOD  300

/*
 * Definition of Kernel Objects 
 */

// Mailboxes
OS_EVENT *Mbox_Throttle;
OS_EVENT *Mbox_Velocity;


// Semaphores
OS_EVENT *aSemaphore;
OS_EVENT *aSemaphore2;
// SW-Timer
OS_TMR *SWTimer;
OS_TMR *SWTimer1;
BOOLEAN status;
/*
 * Types
 */
enum active {on, off};

enum active gas_pedal = off;
enum active brake_pedal = off;
enum active top_gear = off;
enum active engine = off;
enum active cruise_control = off; 

/*
 * Global variables
 */
int delay; // Delay of HW-timer 
INT16U led_green = 0; // Green LEDs
INT32U led_red = 0;   // Red LEDs

int sharedMemory=1;
void TimerCallback(params)
{
    // Post to the semaphore to signal that it's time to run the task.
    OSSemPost(aSemaphore); // Releasing the key
}
void ContextSwitch()
{  
    printf("ContextSwitch!\n"); 
    sharedMemory=sharedMemory*-1;
}
int buttons_pressed(void)
{
  return ~IORD_ALTERA_AVALON_PIO_DATA(DE2_PIO_KEYS4_BASE);    
}

int switches_pressed(void)
{
  return IORD_ALTERA_AVALON_PIO_DATA(DE2_PIO_TOGGLES18_BASE);    
}

/*
 * ISR for HW Timer
 */
alt_u32 alarm_handler(void* context)
{
  OSTmrSignal(); /* Signals a 'tick' to the SW timers */

  return delay;
}

void release()
{
  OSSemPost(aSemaphore2); 
}

static int b2sLUT[] = {0x40, //0
                 0x79, //1
                 0x24, //2
                 0x30, //3
                 0x19, //4
                 0x12, //5
                 0x02, //6
                 0x78, //7
                 0x00, //8
                 0x18, //9
                 0x3F, //-
};

/*
 * convert int to seven segment display format
 */
int int2seven(int inval){
    return b2sLUT[inval];
}

/*
 * output current velocity on the seven segement display
 */
void show_velocity_on_sevenseg(INT8S velocity){
  int tmp = velocity;
  int out;
  INT8U out_high = 0;
  INT8U out_low = 0;
  INT8U out_sign = 0;

  if(velocity < 0){
     out_sign = int2seven(10);
     tmp *= -1;
  }else{
     out_sign = int2seven(0);
  }

  out_high = int2seven(tmp / 10);
  out_low = int2seven(tmp - (tmp/10) * 10);

  out = int2seven(0) << 21 |
            out_sign << 14 |
            out_high << 7  |
            out_low;
  IOWR_ALTERA_AVALON_PIO_DATA(DE2_PIO_HEX_LOW28_BASE,out);
}

/*
 * shows the target velocity on the seven segment display (HEX5, HEX4)
 * when the cruise control is activated (0 otherwise)
 */
void show_target_velocity(INT8U target_vel)
{
}

/*
 * indicates the position of the vehicle on the track with the four leftmost red LEDs
 * LEDR17: [0m, 400m)
 * LEDR16: [400m, 800m)
 * LEDR15: [800m, 1200m)
 * LEDR14: [1200m, 1600m)
 * LEDR13: [1600m, 2000m)
 * LEDR12: [2000m, 2400m]
 */
void show_position(INT16U position)
{
}

/*
 * The function 'adjust_position()' adjusts the position depending on the
 * acceleration and velocity.
 */
 INT16U adjust_position(INT16U position, INT16S velocity,
                        INT8S acceleration, INT16U time_interval)
{
  INT16S new_position = position + velocity * time_interval / 1000
    + acceleration / 2  * (time_interval / 1000) * (time_interval / 1000);

  if (new_position > 24000) {
    new_position -= 24000;
  } else if (new_position < 0){
    new_position += 24000;
  }

  show_position(new_position);
  return new_position;
}

/*
 * The function 'adjust_velocity()' adjusts the velocity depending on the
 * acceleration.
 */
INT16S adjust_velocity(INT16S velocity, INT8S acceleration,  
               enum active brake_pedal, INT16U time_interval)
{
  INT16S new_velocity;
  INT8U brake_retardation = 200;

  if (brake_pedal == off)
    new_velocity = velocity  + (float) (acceleration * time_interval) / 1000.0;
  else {
    if (brake_retardation * time_interval / 1000 > velocity)
      new_velocity = 0;
    else
      new_velocity = velocity - brake_retardation * time_interval / 1000;
  }

  return new_velocity;
}

/*
 * The task 'VehicleTask' updates the current velocity of the vehicle
 */
void VehicleTask(void* pdata)
{ 
  INT8U err;  
  void* msg;
  INT8U* throttle; 
  INT8S acceleration;  /* Value between 40 and -20 (4.0 m/s^2 and -2.0 m/s^2) */
  INT8S retardation;   /* Value between 20 and -10 (2.0 m/s^2 and -1.0 m/s^2) */
  INT16U position = 0; /* Value between 0 and 20000 (0.0 m and 2000.0 m)  */
  INT16S velocity = 0; /* Value between -200 and 700 (-20.0 m/s amd 70.0 m/s) */
  INT16S wind_factor;   /* Value between -10 and 20 (2.0 m/s^2 and -1.0 m/s^2) */

  printf("Vehicle task created!\n");

    // Create a periodic software timer which calls TimerCallback()
    // when it expires.


  SWTimer1 = OSTmrCreate(0,
  CONTROL_PERIOD/(4*OS_TMR_CFG_TICKS_PER_SEC),
  OS_TMR_OPT_PERIODIC,
  TimerCallback,
  NULL,
  NULL,
  &err);
     if (err == OS_ERR_NONE) {
  /* Timer was created but NOT started */
  printf("SWTimer1 was created but NOT started \n");
  }

  status = OSTmrStart(SWTimer1,
    &err);
  if (err == OS_ERR_NONE) {
  /* Timer was started */
    printf("SWTimer1 was started!\n");
  }

  while(1)
    {
      OSSemPend(aSemaphore, 0, &err); // Trying to access the key   
      err = OSMboxPost(Mbox_Velocity, (void *) &velocity);

      //OSTimeDlyHMSM(0,0,0,VEHICLE_PERIOD); 

      /* Non-blocking read of mailbox: 
       - message in mailbox: update throttle
       - no message:         use old throttle
      */
      msg = OSMboxPend(Mbox_Throttle, 1, &err); 
      if (err == OS_NO_ERR) 
         throttle = (INT8U*) msg;

      /* Retardation : Factor of Terrain and Wind Resistance */
      if (velocity > 0)
         wind_factor = velocity * velocity / 10000 + 1;
      else 
         wind_factor = (-1) * velocity * velocity / 10000 + 1;

      if (position < 4000) 
         retardation = wind_factor; // even ground
      else if (position < 8000)
          retardation = wind_factor + 15; // traveling uphill
        else if (position < 12000)
            retardation = wind_factor + 25; // traveling steep uphill
          else if (position < 16000)
              retardation = wind_factor; // even ground
            else if (position < 20000)
                retardation = wind_factor - 10; //traveling downhill
              else
                  retardation = wind_factor - 5 ; // traveling steep downhill

      acceleration = *throttle / 2 - retardation;     
      position = adjust_position(position, velocity, acceleration, 300); 
      velocity = adjust_velocity(velocity, acceleration, brake_pedal, 300); 
      printf("Position: %dm\n", position / 10);
      printf("Velocity: %4.1fm/s\n", velocity /10.0);
      printf("Throttle: %dV\n", *throttle / 10);
      show_velocity_on_sevenseg((INT8S) (velocity / 10));
      //OSSemPost(aSemaphore); // Releasing the key

    }
} 

/*
 * The task 'ControlTask' is the main task of the application. It reacts
 * on sensors and generates responses.
 */

void ControlTask(void* pdata)
{
  INT8U err;
  INT8U throttle = 40; /* Value between 0 and 80, which is interpreted as between 0.0V and 8.0V */
  void* msg;
  INT16S* current_velocity;

  printf("Control Task created!\n");

  while(1)
    {
      OSSemPend(aSemaphore2, 0, &err); // Trying to access the key   
      msg = OSMboxPend(Mbox_Velocity, 0, &err);
      current_velocity = (INT16S*) msg;
      printf("Control Task!\n");
      err = OSMboxPost(Mbox_Throttle, (void *) &throttle);
    }
}

/* 
 * The task 'StartTask' creates all other tasks kernel objects and
 * deletes itself afterwards.
 */ 

void StartTask(void* pdata)
{
  INT8U err;
  void* context;

  static alt_alarm alarm;     /* Is needed for timer ISR function */

  /* Base resolution for SW timer : HW_TIMER_PERIOD ms */
  delay = alt_ticks_per_second() * HW_TIMER_PERIOD / 1000; 
  printf("delay in ticks %d\n", delay);

  /* 
   * Create Hardware Timer with a period of 'delay' 
   */
  if (alt_alarm_start (&alarm,
      delay,
      alarm_handler,
      context) < 0)
      {
          printf("No system clock available!n");
      }

  /* 
   * Create and start Software Timer 
   */

  SWTimer = OSTmrCreate(0,
  CONTROL_PERIOD/(4*OS_TMR_CFG_TICKS_PER_SEC),
  OS_TMR_OPT_PERIODIC,
  release,
  NULL,
  NULL,
  &err);
     if (err == OS_ERR_NONE) {
  /* Timer was created but NOT started */
  printf("SWTimer was created but NOT started \n");
  }

  status = OSTmrStart(SWTimer,
    &err);
  if (err == OS_ERR_NONE) {
  /* Timer was started */
    printf("SWTimer was started!\n");
  }
  /*
   * Creation of Kernel Objects
   */

  // Mailboxes
  Mbox_Throttle = OSMboxCreate((void*) 0); /* Empty Mailbox - Throttle */
  Mbox_Velocity = OSMboxCreate((void*) 0); /* Empty Mailbox - Velocity */

  /*
   * Create statistics task
   */

  OSStatInit();

  /* 
   * Creating Tasks in the system 
   */


  err = OSTaskCreateExt(
       ControlTask, // Pointer to task code
       NULL,        // Pointer to argument that is
                    // passed to task
       &ControlTask_Stack[TASK_STACKSIZE-1], // Pointer to top
                             // of task stack
       CONTROLTASK_PRIO,
       CONTROLTASK_PRIO,
       (void *)&ControlTask_Stack[0],
       TASK_STACKSIZE,
       (void *) 0,
       OS_TASK_OPT_STK_CHK);

  err = OSTaskCreateExt(
       VehicleTask, // Pointer to task code
       NULL,        // Pointer to argument that is
                    // passed to task
       &VehicleTask_Stack[TASK_STACKSIZE-1], // Pointer to top
                             // of task stack
       VEHICLETASK_PRIO,
       VEHICLETASK_PRIO,
       (void *)&VehicleTask_Stack[0],
       TASK_STACKSIZE,
       (void *) 0,
       OS_TASK_OPT_STK_CHK);

  printf("All Tasks and Kernel Objects generated!\n");

  /* Task deletes itself */

  OSTaskDel(OS_PRIO_SELF);
}

/*
 *
 * The function 'main' creates only a single task 'StartTask' and starts
 * the OS. All other tasks are started from the task 'StartTask'.
 *
 */

int main(void) {

  printf("Cruise Control 2014\n");
  aSemaphore = OSSemCreate(1); // binary semaphore (1 key)  
  aSemaphore2 = OSSemCreate(0); // binary semaphore (1 key)    
  OSTaskCreateExt(
     StartTask, // Pointer to task code
         NULL,      // Pointer to argument that is
                    // passed to task
         (void *)&StartTask_Stack[TASK_STACKSIZE-1], // Pointer to top
                             // of task stack 
         STARTTASK_PRIO,
         STARTTASK_PRIO,
         (void *)&StartTask_Stack[0],
         TASK_STACKSIZE,
         (void *) 0,  
         OS_TASK_OPT_STK_CHK | OS_TASK_OPT_STK_CLR);

  OSStart();

  return 0;
}

The output appears to switch using the timers:

Cruise Control 2014
delay in ticks 100
SWTimer was created but NOT started 
SWTimer was started!
All Tasks and Kernel Objects generated!
Vehicle task created!
SWTimer1 was created but NOT started 
SWTimer1 was started!
Control Task created!
Position: 0m
Velocity:  0.4m/s
Throttle: 3V
Control Task!
Position: 0m
Velocity:  0.9m/s
Throttle: 4V
Control Task!
Position: 0m
Velocity:  1.4m/s
Throttle: 4V
Control Task!
Position: 0m
Velocity:  1.9m/s
Throttle: 4V
Control Task!
Position: 1m
Velocity:  2.4m/s
Throttle: 4V
Control Task!
Position: 1m
Velocity:  2.9m/s
Throttle: 4V
Control Task!
Position: 2m
Velocity:  3.4m/s
Throttle: 4V
Control Task!
Position: 3m
Velocity:  3.9m/s
Throttle: 4V
Control Task!
Position: 4m
Velocity:  4.4m/s
Throttle: 4V
Control Task!
Position: 6m
Velocity:  4.9m/s
Throttle: 4V
Control Task!
Position: 7m
Velocity:  5.4m/s
Throttle: 4V
Control Task!
Position: 9m

Modified program version 2 using 1 semaphore + 1 timer

The modified program also can switch tasks and now uses 1 semaphore and 1 timer.

#include <stdio.h>
#include "system.h"
#include "includes.h"
#include "altera_avalon_pio_regs.h"
#include "sys/alt_irq.h"
#include "sys/alt_alarm.h"

#define DEBUG 1

#define HW_TIMER_PERIOD 100 /* 100ms */

/* Button Patterns */

#define GAS_PEDAL_FLAG      0x08
#define BRAKE_PEDAL_FLAG    0x04
#define CRUISE_CONTROL_FLAG 0x02
/* Switch Patterns */

#define TOP_GEAR_FLAG       0x00000002
#define ENGINE_FLAG         0x00000001

/* LED Patterns */

#define LED_RED_0 0x00000001 // Engine
#define LED_RED_1 0x00000002 // Top Gear

#define LED_GREEN_0 0x0001 // Cruise Control activated
#define LED_GREEN_2 0x0002 // Cruise Control Button
#define LED_GREEN_4 0x0010 // Brake Pedal
#define LED_GREEN_6 0x0040 // Gas Pedal

#define TASK_STACKSIZE 2048

OS_STK StartTask_Stack[TASK_STACKSIZE]; 
OS_STK ControlTask_Stack[TASK_STACKSIZE]; 
OS_STK VehicleTask_Stack[TASK_STACKSIZE];

// Task Priorities

#define STARTTASK_PRIO     5
#define VEHICLETASK_PRIO  10
#define CONTROLTASK_PRIO  12

// Task Periods

#define CONTROL_PERIOD  300
#define VEHICLE_PERIOD  300

/*
 * Definition of Kernel Objects 
 */

// Mailboxes
OS_EVENT *Mbox_Throttle;
OS_EVENT *Mbox_Velocity;


// Semaphores
OS_EVENT *aSemaphore;

// SW-Timer
OS_TMR *SWTimer;

/*
 * Types
 */
enum active {on, off};

enum active gas_pedal = off;
enum active brake_pedal = off;
enum active top_gear = off;
enum active engine = off;
enum active cruise_control = off; 

/*
 * Global variables
 */
int delay; // Delay of HW-timer 
INT16U led_green = 0; // Green LEDs
INT32U led_red = 0;   // Red LEDs

void TimerCallback(params)
{
    // Post to the semaphore to signal that it's time to run the task.
    OSSemPost(aSemaphore); // Releasing the key
}

int buttons_pressed(void)
{
  return ~IORD_ALTERA_AVALON_PIO_DATA(DE2_PIO_KEYS4_BASE);    
}

int switches_pressed(void)
{
  return IORD_ALTERA_AVALON_PIO_DATA(DE2_PIO_TOGGLES18_BASE);    
}

/*
 * ISR for HW Timer
 */
alt_u32 alarm_handler(void* context)
{
  OSTmrSignal(); /* Signals a 'tick' to the SW timers */

  return delay;
}

void release()
{
  OSSemPost(aSemaphore); 
} 

static int b2sLUT[] = {0x40, //0
                 0x79, //1
                 0x24, //2
                 0x30, //3
                 0x19, //4
                 0x12, //5
                 0x02, //6
                 0x78, //7
                 0x00, //8
                 0x18, //9
                 0x3F, //-
};

/*
 * convert int to seven segment display format
 */
int int2seven(int inval){
    return b2sLUT[inval];
}

/*
 * output current velocity on the seven segement display
 */
void show_velocity_on_sevenseg(INT8S velocity){
  int tmp = velocity;
  int out;
  INT8U out_high = 0;
  INT8U out_low = 0;
  INT8U out_sign = 0;

  if(velocity < 0){
     out_sign = int2seven(10);
     tmp *= -1;
  }else{
     out_sign = int2seven(0);
  }

  out_high = int2seven(tmp / 10);
  out_low = int2seven(tmp - (tmp/10) * 10);

  out = int2seven(0) << 21 |
            out_sign << 14 |
            out_high << 7  |
            out_low;
  IOWR_ALTERA_AVALON_PIO_DATA(DE2_PIO_HEX_LOW28_BASE,out);
}

/*
 * shows the target velocity on the seven segment display (HEX5, HEX4)
 * when the cruise control is activated (0 otherwise)
 */
void show_target_velocity(INT8U target_vel)
{
}

/*
 * indicates the position of the vehicle on the track with the four leftmost red LEDs
 * LEDR17: [0m, 400m)
 * LEDR16: [400m, 800m)
 * LEDR15: [800m, 1200m)
 * LEDR14: [1200m, 1600m)
 * LEDR13: [1600m, 2000m)
 * LEDR12: [2000m, 2400m]
 */
void show_position(INT16U position)
{
}

/*
 * The function 'adjust_position()' adjusts the position depending on the
 * acceleration and velocity.
 */
 INT16U adjust_position(INT16U position, INT16S velocity,
                        INT8S acceleration, INT16U time_interval)
{
  INT16S new_position = position + velocity * time_interval / 1000
    + acceleration / 2  * (time_interval / 1000) * (time_interval / 1000);

  if (new_position > 24000) {
    new_position -= 24000;
  } else if (new_position < 0){
    new_position += 24000;
  }

  show_position(new_position);
  return new_position;
}

//The task SwitchIO creates the signals ENGINE
//8
//and TOP_GEAR, while the task ButtonIO creates the signals CRUISE_CONTROL,
//GAS_PEDAL and BRAKE_PEDAL. Use the red LEDs to indicate that a switch is active
//and the green LEDs to indicate that a button is active, as specified in Table
//1.

/*
 * The function 'adjust_velocity()' adjusts the velocity depending on the
 * acceleration.
 */
INT16S adjust_velocity(INT16S velocity, INT8S acceleration,  
               enum active brake_pedal, INT16U time_interval)
{
  INT16S new_velocity;
  INT8U brake_retardation = 200;

  if (brake_pedal == off)
    new_velocity = velocity  + (float) (acceleration * time_interval) / 1000.0;
  else {
    if (brake_retardation * time_interval / 1000 > velocity)
      new_velocity = 0;
    else
      new_velocity = velocity - brake_retardation * time_interval / 1000;
  }

  return new_velocity;
}

void ButtonIO()
{
}

void SwitchIO()
{
    engine = on;
    top_gear = on;
}

/*
 * The task 'VehicleTask' updates the current velocity of the vehicle
 */
void VehicleTask(void* pdata)
{ 
  INT8U err;  
  void* msg;
  INT8U* throttle; 
  INT8S acceleration;  /* Value between 40 and -20 (4.0 m/s^2 and -2.0 m/s^2) */
  INT8S retardation;   /* Value between 20 and -10 (2.0 m/s^2 and -1.0 m/s^2) */
  INT16U position = 0; /* Value between 0 and 20000 (0.0 m and 2000.0 m)  */
  INT16S velocity = 0; /* Value between -200 and 700 (-20.0 m/s amd 70.0 m/s) */
  INT16S wind_factor;   /* Value between -10 and 20 (2.0 m/s^2 and -1.0 m/s^2) */

  printf("Vehicle task created!\n");

  while(1)
    {
      OSSemPend(aSemaphore, 0, &err); // Trying to access the key   
      err = OSMboxPost(Mbox_Velocity, (void *) &velocity);

      /* Non-blocking read of mailbox: 
       - message in mailbox: update throttle
       - no message:         use old throttle
      */
      msg = OSMboxPend(Mbox_Throttle, 1, &err); 
      if (err == OS_NO_ERR) 
         throttle = (INT8U*) msg;

      /* Retardation : Factor of Terrain and Wind Resistance */
      if (velocity > 0)
         wind_factor = velocity * velocity / 10000 + 1;
      else 
         wind_factor = (-1) * velocity * velocity / 10000 + 1;

      if (position < 4000) 
         retardation = wind_factor; // even ground
      else if (position < 8000)
          retardation = wind_factor + 15; // traveling uphill
        else if (position < 12000)
            retardation = wind_factor + 25; // traveling steep uphill
          else if (position < 16000)
              retardation = wind_factor; // even ground
            else if (position < 20000)
                retardation = wind_factor - 10; //traveling downhill
              else
                  retardation = wind_factor - 5 ; // traveling steep downhill

      acceleration = *throttle / 2 - retardation;     
      position = adjust_position(position, velocity, acceleration, 300); 
      velocity = adjust_velocity(velocity, acceleration, brake_pedal, 300); 
      printf("Position: %dm\n", position / 10);
      printf("Velocity: %4.1fm/s\n", velocity /10.0);
      printf("Throttle: %dV\n", *throttle / 10);
      show_velocity_on_sevenseg((INT8S) (velocity / 10));

    }
} 

/*
 * The task 'ControlTask' is the main task of the application. It reacts
 * on sensors and generates responses.
 */

void ControlTask(void* pdata)
{
  INT8U err;
  INT8U throttle = 40; /* Value between 0 and 80, which is interpreted as between 0.0V and 8.0V */
  void* msg;
  INT16S* current_velocity;

  printf("Control Task created!\n");

  while(1)
    {
      OSSemPend(aSemaphore, 1, &err); // Trying to access the key   
      msg = OSMboxPend(Mbox_Velocity, 0, &err);
      current_velocity = (INT16S*) msg;
      printf("Control Task!\n");
      err = OSMboxPost(Mbox_Throttle, (void *) &throttle);
    }
}

/* 
 * The task 'StartTask' creates all other tasks kernel objects and
 * deletes itself afterwards.
 */ 

void StartTask(void* pdata)
{
  INT8U err;
  void* context;

  static alt_alarm alarm;     /* Is needed for timer ISR function */

  /* Base resolution for SW timer : HW_TIMER_PERIOD ms */
  delay = alt_ticks_per_second() * HW_TIMER_PERIOD / 1000; 
  printf("delay in ticks %d\n", delay);

  /* 
   * Create Hardware Timer with a period of 'delay' 
   */
  if (alt_alarm_start (&alarm,
      delay,
      alarm_handler,
      context) < 0)
      {
          printf("No system clock available!n");
      }

  /* 
   * Create and start Software Timer 
   */

  SWTimer = OSTmrCreate(0,
  CONTROL_PERIOD/(4*OS_TMR_CFG_TICKS_PER_SEC),
  OS_TMR_OPT_PERIODIC,
  release,
  NULL,
  NULL,
  &err);
     if (err == OS_ERR_NONE) {
  /* Timer was created but NOT started */
  printf("SWTimer was created but NOT started \n");
  }

  BOOLEAN status = OSTmrStart(SWTimer,
    &err);
  if (status > 0 && err == OS_ERR_NONE) {
  /* Timer was started */
    printf("SWTimer was started!\n");
  }
  /*
   * Creation of Kernel Objects
   */

  // Mailboxes
  Mbox_Throttle = OSMboxCreate((void*) 0); /* Empty Mailbox - Throttle */
  Mbox_Velocity = OSMboxCreate((void*) 0); /* Empty Mailbox - Velocity */

  /*
   * Create statistics task
   */

  OSStatInit();

  /* 
   * Creating Tasks in the system 
   */

  err = OSTaskCreateExt(
       ControlTask, // Pointer to task code
       NULL,        // Pointer to argument that is
                    // passed to task
       &ControlTask_Stack[TASK_STACKSIZE-1], // Pointer to top
                             // of task stack
       CONTROLTASK_PRIO,
       CONTROLTASK_PRIO,
       (void *)&ControlTask_Stack[0],
       TASK_STACKSIZE,
       (void *) 0,
       OS_TASK_OPT_STK_CHK);

  err = OSTaskCreateExt(
       VehicleTask, // Pointer to task code
       NULL,        // Pointer to argument that is
                    // passed to task
       &VehicleTask_Stack[TASK_STACKSIZE-1], // Pointer to top
                             // of task stack
       VEHICLETASK_PRIO,
       VEHICLETASK_PRIO,
       (void *)&VehicleTask_Stack[0],
       TASK_STACKSIZE,
       (void *) 0,
       OS_TASK_OPT_STK_CHK);

  printf("All Tasks and Kernel Objects generated!\n");
  /* Task deletes itself */

  OSTaskDel(OS_PRIO_SELF);
}

int main(void) {

  printf("Cruise Control 20141010\n");
  aSemaphore = OSSemCreate(1); // binary semaphore (1 key)    
  OSTaskCreateExt(
     StartTask, // Pointer to task code
         NULL,      // Pointer to argument that is
                    // passed to task
         (void *)&StartTask_Stack[TASK_STACKSIZE-1], // Pointer to top
                             // of task stack 
         STARTTASK_PRIO,
         STARTTASK_PRIO,
         (void *)&StartTask_Stack[0],
         TASK_STACKSIZE,
         (void *) 0,  
         OS_TASK_OPT_STK_CHK | OS_TASK_OPT_STK_CLR);

  OSStart();

  return 0;
}

enter image description here

\$\endgroup\$
  • 1
    \$\begingroup\$ Please do not greatly modify a question after receiving answers. If you'd like to ask something separate, post a new question. \$\endgroup\$ – Jamal Oct 10 '14 at 15:51
  • \$\begingroup\$ @Jamal My requirement wasn't clear from my question. Is it best to leave this question complete without updating it or could I update the question with the new version of the program + the requirement without modifying my original question? I could also try be more specific in a new question. I had some problem understand the requirement and I got good help from the information that the program can be improved and use less resources. Thank you for the help. \$\endgroup\$ – Niklas Oct 10 '14 at 16:23
  • 1
    \$\begingroup\$ Just as long as the answer isn't invalidated in any way or adds code based on improvements from the answer. It was hard to tell for sure if this was done. However, if the answerer was unsure about your requirements, then that should've been clarified in comments first. \$\endgroup\$ – Jamal Oct 10 '14 at 16:26
  • \$\begingroup\$ Probably just me being old fashioned, but I've never been able to make any sense of those sorts of bubble and arrow diagrams. In your diagram, can it be true that "Brake Pedal" doesn't feed in to the "Control Law" bubble? And there is no way for the cruise control to change gear - is that right (maybe CC only acts in top gear, I've never used one)? \$\endgroup\$ – William Morris Oct 10 '14 at 17:33
4
\$\begingroup\$

Niklas, I'll make a few comments.

Firstly the code is rather chaotic. It is difficult to make out what is going on and what the requirement is. I'd expect to see some sort of comment at the top that summarises the purpose of the overall system. I cannot compile it because of all the headers, so I don't know how cleanly it builds. Do you have plenty of compiler warnings turned on?

I don't follow exactly what is required of you. If your 2 tasks are just alternating then one semaphore and one timer would seem to be all you need, not two of each. The timer wakes up the first task via the semaphore; the first task waits on the semaphore, sends a message to the 2nd then waits on a reply from the 2nd; the 2nd task waits on a message queue from the 1st task, does its stuff then sends a reply to the first.

Some specific comments:

  • Error handling is lacking. Often you don't check errors or you check for success but continue on failure. Error handling in an embedded system is difficult. As this is an exercise, I suggest that on error you should turn on all LEDs or make them say ERR or something like that and then hang in an infinite loop. A real system would want to be able to recover (eg restart via a watchdog timer), but I think that is beyond the scope of your project.

  • Some of your variable and function names are not so good. aSemaphore and aSemaphore2 give no clue as to their function and are hardly different. Ditto, SWTimer amd SWTimer2. TimerCallback is unhelpful when there are 2 timers and release is equally opaque.

  • There are many flags and some functions and variables that are unused. When submitting code for review your should omit such parts. This also applies to commented-out lines of code which give the reader a poor impression of the completeness/readiness of your code.

  • Global variable status should be deleted. Make it local and check its value.

  • Your b2sLUT should be const and should be integrated into and interrogated by show_velocity_on_sevenseg. int2seven seems redundant.

  • You have numeric constants scattered throughout your code. This is generally considered bad practice, (although there can be occasions where it makes sense).

  • Your comment on show_position refers to 4 LEDs but lists 6.

  • You should be very careful about your integer arithmetic to avoid overflow or loss of precision. Integer dividing x by 1000 for example will give a result of 0 for x == 999. It can be better to multiply first, then divide in order not to lose precision.

  • Your small types (8 and 16 bit) types may be inappropriate. Or they may be ok. It depends upon your target processor.

  • Your adjust_velocity obscures global brake_pedal with its local variable of the same name. Try to avoid globals if at all possible, but if you must use them, don't do this. Also note that if globals are being shared between tasks they need to be declared volatile.

  • Your task startup code is messy. I don't see why you need main to start StartTask and then the latter to start the two persistent tasks and die. And I don't see why VehicleTask creates its own timer yet ControlTask has its timer created for it by StartTask (but see above on the questionable necessity for two timers/semaphore).

  • Splitting the job into separate files for each persistent task and a main might be preferable.

|improve this answer|||||
\$\endgroup\$
  • \$\begingroup\$ Thank you for the help. Now I've updated the program using only one timer and only one semaphore and it's task-switching. I also added more info about what I'm trying to do. My target system is the Altera DE2 FPGA. I use Quartus and Nios 2 IDE. My code was sloppy (but it compiled and ran). Now I hope it's better and more cleanly addresses the problem. I don't get any warnings or compilers errors (it's using gcc and has many settings that I didn't look at yet). \$\endgroup\$ – Niklas Oct 10 '14 at 15:42
  • \$\begingroup\$ My requirement was The skeleton program uses the statement OSTimeDlyHMSM to implement periodic tasks, which will not give an exact period. Use instead soft timers for this purpose and connect them with semaphores to your tasks. Use the same period for the soft timers as in the original skeleton program. \$\endgroup\$ – Niklas Oct 10 '14 at 16:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.