ROBOTC Jeopardy program

Question

I have tried to comment my code as much as I can. If you have any questions about it, please feel free to ask. The code itself should have a lot of details and the comments mostly explain what is going on. The task description is in the code.

#pragma config(Sensor, dgtl1,  allowButton,    sensorTouch)
#pragma config(Sensor, dgtl2,  resetButton,    sensorTouch)
#pragma config(Sensor, dgtl3,  redLED1,        sensorLEDtoVCC)
#pragma config(Sensor, dgtl4,  contestantButton1, sensorTouch)
#pragma config(Sensor, dgtl5,  redLED2,        sensorLEDtoVCC)
#pragma config(Sensor, dgtl6,  contestantButton2, sensorTouch)
#pragma config(Sensor, dgtl7,  redLED3,        sensorLEDtoVCC)
#pragma config(Sensor, dgtl8,  contestantButton3, sensorTouch)
#pragma config(Sensor, dgtl9,  resetLED,       sensorLEDtoVCC)
#pragma config(Sensor, dgtl10, greenLEDStart,  sensorLEDtoVCC)
//*!!Code automatically generated by 'ROBOTC' configuration wizard               !!*//

/*
Project Title: Jeopardy Game
Team Members: 
Date: 11/12/13
Section:

Task Description:
The Jeopardy game should be programmed to perform the following functions:
1.  Alex has two buttons at his podium; a button that enables players to ring in once he has read the question or to ring in after someone misses the question (the green LED should light up to indicate the button has been pressed), and the other to reset the program.
2.  Each contestant has their own signalling button when they ring in, their red light flashes with the flashes getting closer and closer together until it stays lit showing they are out of time.
3.  After a contestant rings in, they should not be able to ring in again if they miss the question.
4.  If no one answers the question or if someone gets the question right, the reset button should be hit to reset the program.
5.  Wowing the teacher can be achieved if the green light acts as a timer for any contestant to ring in and if a player is locked out if they hold down their signalling button when Alex presses his enable button.
Hint: A thorough knowledge of functions is required.

Pseudo code:
After the question is read, Alex hits allowButton, this enables the contestants buttons
timer starts
Contestants hits their button,their button is disabled and red light starts flashing
reset button resets all the buttons and timers
*/
bool contestantsAllow;
bool badContestant1;
bool badContestant2;
bool badContestant3;
int y = 500;

void blinkLED(tSensors sensorPort)
{
    ClearTimer(T2);
    while(time1[T2] < 10000) //Performs  body for 10 seconds (10000 Milliseconds
    {

        turnLEDOn(sensorPort);
        waitInMilliseconds (y);
        turnLEDOff(sensorPort);
        waitInMilliseconds (y);
        y = y *(5/10); //cuts wait time in half 

    }
}

task resetAll
{
    while (1) //Keeps task running at all times
    {
            if (SensorValue[resetButton] == 1)//Resets all Sensors and Booleans
        {
            turnLEDOff (redLED1);
            turnLEDOff (redLED2);
            turnLEDOff (redLED3);
            turnLEDOff (greenLEDStart);
            SensorValue[contestantButton1] = 0;
            SensorValue[contestantButton2] = 0;
            SensorValue[contestantButton3] = 0;
            SensorValue[allowbutton] = 0;
            badContestant1 = false;
            badContestant2 = false;
            badContestant3 = false;
            contestantsAllow = false;
            turnLEDOn(resetLED);
            wait (.5);
            turnLEDOff(resetLED);
        }
    }
}
task main()
{
    while(1)
    {
    StartTask(resetAll); //Starts the reset task so it is running in parallel with the main task
    while (SensorValue[allowButton] == 0) //makes sure that the allowbutton isn't pressed
    {
        if (SensorValue[contestantButton1] == 1) //Checks to see if they have tried to answer before the question was finished
        {
            turnLEDOn(redLED1); //Visual that they can no longer answer
            badContestant1 = true; //They are not allowed to answer if true
        }
        if (SensorValue[contestantButton2] == 1) //Checks to see if they have tried to answer before the question was finished
        {
            turnLEDOn(redLED2); //Visual that they can no longer answer
            badContestant2 = true; //They are not allowed to answer if true
        }

        if (SensorValue[contestantButton3] == 1) //Checks to see if they have tried to answer before the question was finished
        {
            turnLEDOn(redLED3); //Visual that they can no longer answer
            badContestant3 = true; //They are not allowed to answer if true
        }
    }
    contestantsAllow = true; //Sets value to true so that contestants can answer the question

    turnLEDOn(greenLEDStart); //Visual allowing them to answer the question
    ClearTimer(T1);
    while (time1[T1]<6000) //Starts timer, contestants  6 seconds
    {

            if (SensorValue[contestantButton1] == 1 && badContestant1 == false) //If they press their button and badcontestant is false then they can answer the question
            {
                badContestant2 = true; //locks other contestants out
                badContestant3 = true; //locks other contestants out
                {
                    ClearTimer(T2); //resets timer[T2]
                    while(time1[T2] < 10000) //Starts Timer for 10 seconds
                    blinkLED(redLED1);//runs function blinkLED

                    turnLEDOn(redLED2); //Visual showing that their turn is over
                    badContestant1 = true; //Contestant can't answer again
                    badContestant2 = false; //allows other contestants to answer
                    badContestant3 = false;//allows other contestants to answer
                    ClearTimer(T1); //Clears timer so that there is another 6 seconds added to the clock
                }

            }
            else if (SensorValue[contestantButton2] == 1 && badContestant2 == false) //If they press their button and badcontestant is false then they can answer the question
            {
                badContestant1 = true; //locks other contestants out
                badContestant3 = true; //locks other contestants out

                {
                    ClearTimer(T2); //resets timer[T2]
                    while(time1[T2] < 10000) //Starts Timer for 10 seconds  
                    blinkLED(redLED2);//runs function blinkLED

                    turnLEDOn(redLED2); //Visual showing that their turn is over
                    badContestant2 = true;//Contestant can't answer again
                    badContestant1 = false;//allows other contestants to answer
                    badContestant3 = false;//allows other contestants to answer
                    ClearTimer(T1); //Clears timer so that there is another 6 seconds added to the clock
                }
            }
            else if (SensorValue[contestantButton3] == 1 && badContestant3 == false) //If they press their button and badcontestant is false then they can answer the question
            {
                badContestant2 = true; //locks other contestants out
                badContestant1 = true; //locks other contestants out

                ClearTimer(T2); //resets timer[T2]
                while(time1[T2] < 10000) //Starts Timer for 10 seconds
                blinkLED(redLED3);//runs function blinkLED

                turnLEDOn(redLED3); //Visual showing that their turn is over
                badContestant3 = true;//Contestant can't answer again
                badContestant2 = false;//allows other contestants to answer
                badContestant1 = false;//allows other contestants to answer
                ClearTimer(T1); //Clears timer so that there is another 6 seconds added to the clock
            }
        }
    }
}

Lundin · Accepted Answer · 2013-12-10 07:46:01Z

Program design issues:

Do not ever use global variables. In this case, the variables contestantsAllow and so on should be declared locally in main, because there is no reason to have them in the global namespace.

Had you been using a more object-oriented design, where for example everything related to sensors was put in a separate module (sensor.h + sensor.c etc), then variables that are only used by that module could be declared at file scope, but with the keyword static, thereby using the object-oriented concepts of private variables and encapsulation.
y should be given a meaningful name. It is a delay in miliseconds, why don't name it such? led_delay_ms for example.
You lock up the whole CPU at 100% whenever you execute your delays. That may not be an issue in this case, but it will be in a larger program. Consider using polling or interrupts instead.
Even though Jeopardy always come with exactly 3 competitors, consider rewriting the code so that it works with any amount of competitors. The 3 different badContestant variables should for example be stored in an array, to increase readability and make the program more generic: bool badContestant [COMPETITORS];. Same goes for contestantButton and so on. This will not only make your program more powerful, it also prevents you from having the same code at different places all over the program, making it easier to maintain.

Bugs:

y = y *(5/10); performs a division on an integer, not a floating point. 5/10 is always 0. You need to learn the difference between integers and floats. You can probably fix this specific bug by changing the code to y = (y*5) / 10;.
Variables shared between different tasks in a RTOS, or between the main code and an interrupt, or between threads, must always be declared as volatile. This will prevent potential bugs caused by code optimization, where the compiler does not realize that a particular variable is updated. In addition, you may need to protect such variables with semaphores, to achieve thread safety (volatile does not give thread-safe, atomic access).
You don't seem to include stdbool.h. Is the code posted really complete? It shouldn't compile. If it does compile without that header, chances are you are compiling as C++, which is always a bad idea. C and C++ are quite different languages in many ways.
This may or may not be a bug, but are you certain that the function wait() expects a floating point variable? Looks fishy to me.

Coding style:

It is a good habit of always using {} after each if, for or while. This will save you from a lot of accidental bugs.
Your indention isn't consistent, making the code hard to read. Always indent with either 2 or 4 spaces (pick one style and stick to it). If you are using tab key for indention, make sure your IDE replaces tabs with spaces.

Consider placing comments at a specific column whenever possible, like after 40 or 50 symbols. (There are indention programs that can do this for you, like UniversalIndent.) Example:

Instead of

ClearTimer(T2); //resets timer[T2]
while(time1[T2] < 10000) //Starts Timer for 10 seconds
blinkLED(redLED1);//runs function blinkLED

put comments like this:

ClearTimer(T2);                  //resets timer[T2]
while(time1[T2] < 10000)         //Starts Timer for 10 seconds
  blinkLED(redLED1);             //runs function blinkLED

Avoid meaningless comments. Your comments should address what the program is doing, so that another C programmer can tell what it does. You should assume that the reader of your comments know the C language.

An example of a good comment is badContestant2 = true; //locks other contestants out. It tells the reader what this code row will cause.

An example of a bad comment is blinkLED(redLED1); //runs function blinkLED. Every C programmer knows that the code is a function call, so the comment is pointless.

Also, if the code speaks for itself, you need not comment it. For example, ClearTimer(T2); speaks for itself. The comment //resets timer[T2] is superfluous.
Never use "magic numbers" in the code. If you have something like while (time1[T1]<6000), then nobody can tell what 6000 is supposed to mean, unless there is a comment. Instead, put the numeric constant in a #define or a const variable. For example you could write while (time1[T1]< 6*SECOND). And then suddenly you can remove the comment too, because the code speaks for itself.

Thank you so much, and the wait function is correct, and the program I use will compile the code without any issues. But do you think you could figure out an equation that is based off of timer2 for my blinking light function, so as the timer approaches 6 seconds ( I'm changing it from 10 seconds) the light will blink faster? — Nategamer
– Nategamer, Commented Nov 15, 2013 at 12:55
Nice answer, +1. But are private variables and encapsulation really "object-oriented concepts"? — William Morris
– William Morris, Commented Nov 16, 2013 at 13:42
Mmm, I use both in C, but I never though I was being object orientated. — William Morris
– William Morris, Commented Nov 18, 2013 at 16:13
@WilliamMorris Encapsulation + Abstraction + Inheritance + Polymorphism = OOP ;) — Mathieu Guindon
– Mathieu Guindon, Commented Apr 18, 2014 at 22:55

Stack Exchange Network

ROBOTC Jeopardy program

1 Answer 1

You must log in to answer this question.

Hot Network Questions

ROBOTC Jeopardy program

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions