I've already posted a few threads in this forum for help with some basics, but this is the project that I am working towards. The code below works but I think it'll make most people here cringe!

I have 4 input switches (RA0 to RA3) and 4 output LEDs (RB0 to RB3). The game starts with an LED countdown. Once the countdown is over, the first player to hit the switch lights their corresponding LED. I added a basic check to prevent cheating (by holding the switch during the countdown).

I would really appreciate any feedback as to how I can improve this program. At the moment, the winning LED remains lit as it is stuck in a never-ending loop. I tried to come up with a better solution but I'm not sure how. I'd also like to add a buzzer and perhaps some chasing LED effects.

Thanks in advance.

#include "htc.h"
#define _XTAL_FREQ 4000000
#define delay_s(T) {unsigned char i; for (i=0; i<T*10; i++) __delay_ms(100);}

__CONFIG(FOSC_INTRC_NOCLKOUT & WDTE_OFF & PWRTE_ON & MCLRE_ON & CP_OFF & CPD_OFF & BOREN_OFF & IESO_OFF & FCMEN_OFF & LVP_OFF & DEBUG_OFF & BOR4V_BOR40V & WRT_OFF);

char team0_ok, team1_ok, team2_ok, team3_ok;

int main(void) {

 	TRISA = 0b00001111; // RA0 to RA3 are inputs, RA4 to RA7 are outputs
	TRISB = 0b11110000; // RB0 to RB3 are outputs, RB4 to RB7 are inputs
	ANSEL = 0; // configure PORTA analog channels as a digital inputs

	// initial LED "countdown"
	PORTB = 0b00000001;
	delay_s(1);
	PORTB = 0b00000011;
	delay_s(1);
	PORTB = 0b00000111;
	delay_s(1);
	PORTB = 0b00001111;
	delay_s(3);
	PORTB = 0; // no output LEDs

	// reset team_ok values
	team0_ok = 1;
	team1_ok = 1;
	team2_ok = 1;
	team3_ok = 1;
	
	// check whether an input is already high (cheating!)
	if(RA0 == 1){
		team0_ok = 0;
	}
	if(RA1 == 1){
		team1_ok = 0;
	}
	if(RA2 == 1){
		team2_ok = 0;
	}
	if(RA3 == 1){
		team3_ok = 0;
	}

	// loop to detect an input and light the corresponding LED
 	for(;;) {
		if((RA0 == 1)&&(team0_ok == 1)){
			for(;;){RB0 = 1;}
		}
		else if((RA1 == 1)&&(team1_ok == 1)){
			for(;;){RB1 = 1;}
		}
		else if((RA2 == 1)&&(team2_ok == 1)){
			for(;;){RB2 = 1;}
		}
		else if((RA3 == 1)&&(team3_ok == 1)){
			for(;;){RB3 = 1;}
		}
		else {
			// nothing -- is this ok?
		}
	}
}

 

How do you design your switch setup. Do you use pull-up or pull-down resistors

 

How do you design your switch setup. Do you use pull-up or pull-down resistors

I use pull-up resistors with push-to-break switches at the moment.

 

I use pull-up resistors with push-to-break switches at the moment.

Just to be sure. Can you make a sketch of your switch setup. Nothing fancy is needed. just use paint, or a hand drawing will do fine. You are aware of that then digital inputs are not connect they will see a high level. As a rule of thumb. But in worst case the digital input left open or floating. May flicker between high and low level. So digital inputs should never be left floating but connected to either a high proper high or low level.

 

I have since extended the program to support 8 switches and added a buzzer on RC7. The program below works perfectly, but I'm still interested in hearing feedback on how I could improve it. I've attached the schematic.

PORTA = switches
PORTB = LEDs
PORTC = 7 segment display (+ buzzer on RC7)

#include "htc.h"
#define _XTAL_FREQ 4000000
#define delay_s(T) {unsigned char i; for (i=0; i<T; i++) __delay_ms(100);}

__CONFIG(FOSC_INTRC_NOCLKOUT & WDTE_OFF & PWRTE_ON & MCLRE_ON & CP_OFF & CPD_OFF & BOREN_OFF & IESO_OFF & FCMEN_OFF & LVP_OFF & DEBUG_OFF & BOR4V_BOR40V & WRT_OFF);

void chase(void) {
	unsigned char i;
	
	PORTB = 0b00011111;
	for(;;){
		for(i=0; i<7; i++){
			PORTB = (PORTB >> 1)|(PORTB << (sizeof(PORTB)*8 - 1)); //bitwise rotate
			__delay_ms(75);
		}
	}
}

void beep(unsigned char beeps) {
	unsigned char i;
	
	for(i=0; i<beeps; i++){
		RC7 = 0;
		__delay_ms(100);
		RC7 = 1;
		__delay_ms(100);
	}
}		

int main(void) {
	unsigned char team0_ok, team1_ok, team2_ok, team3_ok, team4_ok, team5_ok, team6_ok, team7_ok;
	
	TRISA = 1; // PORTA inputs (switches)
	TRISB = 0; // PORTB outputs (LEDs)
	TRISC = 0; // PORTC outputs (7 seg)
	ANSEL = 0; // configure PORTA analog channels as digital inputs
	ANSELH = 0;
	
	//reset
	PORTA = 0b11111111;
	PORTB = 0b11111111;
	PORTC = 0b11111111;
	
	// initial LED "countdown"
	beep(4);
	PORTB = 0b01111111;
	delay_s(5);
	PORTB = 0b00111111;
	delay_s(5);
	PORTB = 0b00011111;
	delay_s(5);
	PORTB = 0b00001111;
	delay_s(5);
	PORTB = 0b00000111;
	delay_s(5);
	PORTB = 0b00000011;
	delay_s(5);
	PORTB = 0b00000001;
	delay_s(5);
	PORTB = 0b00000001;
	delay_s(30);
	PORTB = 0b11111111; // no output LEDs
	
	// reset team_ok values
	team0_ok = 1;
	team1_ok = 1;
	team2_ok = 1;
	team3_ok = 1;
	team4_ok = 1;
	team5_ok = 1;
	team6_ok = 1;
	team7_ok = 1;
	
	// check whether a switch is being pressed (cheating!)
	if(RA0 == 0){
		team0_ok = 0;
	}
	if(RA1 == 0){
		team1_ok = 0;
	}
	if(RA2 == 0){
		team2_ok = 0;
	}
	if(RA3 == 0){
		team3_ok = 0;
	}
	if(RA4 == 0){
		team4_ok = 0;
	}
	if(RA5 == 0){
		team5_ok = 0;
	}
	if(RA6 == 0){
		team6_ok = 0;
	}
	if(RA7 == 0){
		team7_ok = 0;
	}

	// loop to detect an input
 	for(;;) {
		if((RA0 == 0)&&(team0_ok == 1)){
			for(;;){
				PORTC = 0b10000000; //8
				beep(2);
				chase();
			}
		}
		else if((RA1 == 0)&&(team1_ok == 1)){
			for(;;){
				PORTC = 0b11111000; //7
				beep(2);
				chase();
			}
		}
		else if((RA2 == 0)&&(team2_ok == 1)){
			for(;;){
				PORTC = 0b10000010; //6
				beep(2);
				chase();
			}
		}
		else if((RA3 == 0)&&(team3_ok == 1)){
			for(;;){
				PORTC = 0b10010010; //5
				beep(2);
				chase();
			}
		}
		else if((RA4 == 0)&&(team4_ok == 1)){
			for(;;){
				PORTC = 0b10011001; //4
				beep(2);
				chase();
			}
		}
		else if((RA5 == 0)&&(team5_ok == 1)){
			for(;;){
				PORTC = 0b10110000; //3
				beep(2);
				chase();
			}
		}
		else if((RA6 == 0)&&(team6_ok == 1)){
			for(;;){
				PORTC = 0b10100100; //2
				beep(2);
				chase();
			}
		}
		else if((RA7 == 0)&&(team7_ok == 1)){
			for(;;){
				PORTC = 0b11111001; //1
				beep(2);
				chase();
			}					
		}
		else {
			// nothing
		}
	}
}

 

Well if it works then... SHIP IT !!! <grin>

One thing I noticed in your first post is in your "loop to detect an input" you read an individual pin then move to the next one. Thus there is a (admittedly very small) chance that either two buttons are pressed at the same time (within one loop) or one is pressed just after it was sensed but still slightly before another button is pressed as it is being sensed.

Ugh, I mean it is possible to not detect ties, and possible to miss a true winner.

To solve that, inside the "loop to detect an input" I would read the pins once and store the value, then do the button test against the stored value. That eliminated the race to give a fair result.

That does nothing to solve the issue of ties.

Also, you need to press the reset button to get out of your loop. That makes it hard to make a total of many tries.

 

To solve that, inside the "loop to detect an input" I would read the pins once and store the value, then do the button test against the stored value. That eliminated the race to give a fair result.

That makes sense. I'll give it a try.

Just out of interest, approximately how quickly will the PIC cycle through the entire "loop to detect an input"? Are we talking tens/hundreds/thousands of times per second?

 

That makes sense. I'll give it a try.

Just out of interest, approximately how quickly will the PIC cycle through the entire "loop to detect an input"? Are we talking tens/hundreds/thousands of times per second?

That is a very good point for you to determine using the MPLAB simulator running your code, with the instruction clock set to match your hardware.

Drop in a breakpoint at the start of the loop, run to it, reset the stopwatch, and run till it again stops.

The stopwatch will tell you the whole story.

 

That is a very good point for you to determine using the MPLAB simulator running your code, with the instruction clock set to match your hardware.

Thanks. According to MPLAB SIM, it cycles through in just 9.6 uSecs.