patterncMinor
Ad hoc implementation of cruise control
Viewed 0 times
implementationcruisecontrolhoc
Problem
I've written a small C program that has ad hoc implementation of cruise control for my Altera DE2 FPGA. It works alright but there are too many global variables. Preferably there should be no global variables. How can I reduce the amount of global variables?
```
#include
#include "system.h"
#include "includes.h"
#include "altera_avalon_pio_regs.h"
#include "sys/alt_irq.h"
#include "sys/alt_alarm.h"
#include "altera_avalon_pio_regs.h"
#include "alt_types.h"
#include
#include
#include
#define N 8192
#define M 32
#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_3 0x0004 // 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];
OS_STK VehicleTask_Stack[TASK_STACKSIZE];
OS_STK WatchdogTask_Stack[TASK_STACKSIZE];
OS_STK DetectionTask_Stack[TASK_STACKSIZE];
// Task Priorities
#define STARTTASK_PRIO 5
#define VEHICLETASK_PRIO 10
#define CONTROLTASK_PRIO 12
#define DETECTIONTASK_PRIO 13
#define WATCHDOGTASK_PRIO 14
// Task Periods
#define CONTROL_PERIOD 300
#define VEHICLE_PERIOD 300
/*
* Definition of Kernel Objects
*/
// Mailboxes
OS_EVENT *Mbox_Throttle;
OS_EVENT *Mbox_Velocity;
OS_EVENT *Mbox_Writeok;
// Semaphores
OS_EVENT *aSemaphore;
OS_EVENT *aSemaphore2;
// SW-Timer
OS_TMR *SWTimer;
alt_u32 ticks;
alt_u32 time_1;
alt_u32 time_2;
alt_u32 timer_overhead;
/*
* Types
*/
enum active {on, off};
enum a
```
#include
#include "system.h"
#include "includes.h"
#include "altera_avalon_pio_regs.h"
#include "sys/alt_irq.h"
#include "sys/alt_alarm.h"
#include "altera_avalon_pio_regs.h"
#include "alt_types.h"
#include
#include
#include
#define N 8192
#define M 32
#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_3 0x0004 // 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];
OS_STK VehicleTask_Stack[TASK_STACKSIZE];
OS_STK WatchdogTask_Stack[TASK_STACKSIZE];
OS_STK DetectionTask_Stack[TASK_STACKSIZE];
// Task Priorities
#define STARTTASK_PRIO 5
#define VEHICLETASK_PRIO 10
#define CONTROLTASK_PRIO 12
#define DETECTIONTASK_PRIO 13
#define WATCHDOGTASK_PRIO 14
// Task Periods
#define CONTROL_PERIOD 300
#define VEHICLE_PERIOD 300
/*
* Definition of Kernel Objects
*/
// Mailboxes
OS_EVENT *Mbox_Throttle;
OS_EVENT *Mbox_Velocity;
OS_EVENT *Mbox_Writeok;
// Semaphores
OS_EVENT *aSemaphore;
OS_EVENT *aSemaphore2;
// SW-Timer
OS_TMR *SWTimer;
alt_u32 ticks;
alt_u32 time_1;
alt_u32 time_2;
alt_u32 timer_overhead;
/*
* Types
*/
enum active {on, off};
enum a
Solution
Wow, I don't know where to start. I think I'll just work on one function.
I'm not sure what the intent is here. Part of the problem is the completely obscure variable names and values. What is
Do you really mean to poll the status of
CONTINUING...
Can you post these files on pastebin, github, or some other textfile hosting site and include a link?
Why in the world do you
void SwitchIO(INT16S* current_velocity, int position)
{
int w[700];
int tmp = 600;
int x[13];
int a, b;
int size = M;
int i, j;
timer_overhead = 0;
if (switches_pressed()==1) {
engine = on;
top_gear=off;
show_position(position);
printf("The engine is turned on\n");
} else if (switches_pressed()==3) {
engine = on;
top_gear=on;
show_position(position);
}
else if (switches_pressed()==0) {
printf("switches_pressed()==0\n");
top_gear = off;
engine = off;
show_position(position);
}
else if (switches_pressed()==2) {
}
else if ((switches_pressed()-19)/16 >= 0) {
int number = (switches_pressed()-19)/16+1;
if (number > 50) {
number = 50;
}
printf("extra load %d\n", number);
for (i = 0; i < 10; i++) {
start_measurement();
stop_measurement();
timer_overhead = timer_overhead + time_2 - time_1;
}
initArray(w, 600);
initArray(x, 13);
start_measurement();
j=tmp+number*4;
for (i = 0; i < j; i++)
w[i]++;
stop_measurement();
printf("%5.2f us", (float) microseconds(ticks - timer_overhead));
printf("(%d ticks)\n", (int) (ticks - timer_overhead));
}
printf("switches_pressed: %d\n", switches_pressed());
printf("switches_pressed: %d\n", (switches_pressed()-19)/16);
}I'm not sure what the intent is here. Part of the problem is the completely obscure variable names and values. What is
w and why does it need to be of size 700? And so forth.Do you really mean to poll the status of
switches_pressed() multiple times, and then only take one action? That doesn't seem logical to me. If I were writing something similar, I'd save the value of switches_pressed() into a temporary variable, and then use chained ifs or a switch in order to take the appropriate action.CONTINUING...
#include "system.h"
#include "includes.h"
#include "altera_avalon_pio_regs.h"
#include "sys/alt_irq.h"
#include "sys/alt_alarm.h"
#include "altera_avalon_pio_regs.h"
#include "alt_types.h"Can you post these files on pastebin, github, or some other textfile hosting site and include a link?
Why in the world do you
#include "altera_avalon_pio_regs.h" twice?Code Snippets
void SwitchIO(INT16S* current_velocity, int position)
{
int w[700];
int tmp = 600;
int x[13];
int a, b;
int size = M;
int i, j;
timer_overhead = 0;
if (switches_pressed()==1) {
engine = on;
top_gear=off;
show_position(position);
printf("The engine is turned on\n");
} else if (switches_pressed()==3) {
engine = on;
top_gear=on;
show_position(position);
}
else if (switches_pressed()==0) {
printf("switches_pressed()==0\n");
top_gear = off;
engine = off;
show_position(position);
}
else if (switches_pressed()==2) {
}
else if ((switches_pressed()-19)/16 >= 0) {
int number = (switches_pressed()-19)/16+1;
if (number > 50) {
number = 50;
}
printf("extra load %d\n", number);
for (i = 0; i < 10; i++) {
start_measurement();
stop_measurement();
timer_overhead = timer_overhead + time_2 - time_1;
}
initArray(w, 600);
initArray(x, 13);
start_measurement();
j=tmp+number*4;
for (i = 0; i < j; i++)
w[i]++;
stop_measurement();
printf("%5.2f us", (float) microseconds(ticks - timer_overhead));
printf("(%d ticks)\n", (int) (ticks - timer_overhead));
}
printf("switches_pressed: %d\n", switches_pressed());
printf("switches_pressed: %d\n", (switches_pressed()-19)/16);
}#include "system.h"
#include "includes.h"
#include "altera_avalon_pio_regs.h"
#include "sys/alt_irq.h"
#include "sys/alt_alarm.h"
#include "altera_avalon_pio_regs.h"
#include "alt_types.h"Context
StackExchange Code Review Q#86468, answer score: 3
Revisions (0)
No revisions yet.