C Coding Problem(MPLab-PIC18f45k22)

Whew! Really hard to follow the logic without comments!

Also, you have TRISB set to 0x00, yet your comment says it's for "push buttons". Setting a TRIS bit to '0' makes it an output. I would think you would want to set the PButton bits to '1' for 'input'.

hope now will be little bit easy to read the code....thx if can help.

I notice that you have all of Port D and all of Port E set as inputs, yet you seem to only be using bit 7 on Port D. Are all of the other inputs floating? And, for the one or more (looks like three) push buttons, are you ensuring that the voltage on the inputs always represents a high or a low [i.e. with some sort of pull-up or pull-down (typically a resistor)--since I only see weak pull-ups enabled on PortB--though not sure if a pic18f45k22 has weak pull-ups on the other ports)?

Also, it's difficult to determine if your code will never "run away", or even follow the logic, because of the gotos -- your code is what is known as "spaghetti code".

I suggest you read up on "structured programming techniques".

Plus, your bock diagram doesn't tell me anything about which port terminals are connected to what.

Hi First of all couple of things I notice about your coding



1: No Flow charts ( Very Poor one attached not really a flow chart its more an idea of what you are trying to do , semic type schematic come process )

I understand what you are trying to do from this however the process and what , when, why is missing

2: No Schematics
3: Goto A "Very bad coding when it comes to C or C++ We don’t use Goto at all in C " Its not military standards

As far as Universities are concerned etc They don’t allow goto instructions

You enclose instructions like this using a function and you use a function call or you use a nested for next loop with if statements and sets flags to true or false conditions to achieve this and exit when conditions are reached

Or Use recursion although even this is not truly acceptable but it is a little more acceptable than goto instructions Recusive methods are last resort when no other way of achieving this can be sort Its is permissable but its not truly liked

For Proton basic Yes this is acceptable but that’s the way the language is structured For C however , this is regarded as very poor coding




4: Keypad debounce routines ( If Switch press , while switch pressed , wait for release ) Hence flow charts see the following below

http://www.ganssle.com/debouncing.htm

Also see this site for debouncing

http://drmarty.blogspot.co.uk/2009/05/best-switch-debounce-routine-ever.html

And this site too is very informative
http://www.micahcarrick.com/avr-tutorial-switch-debounce.html

If you need a worked simple example then also see here

http://www.eeweb.com/project/mark_harrington/single-pic-micro-single-shot-bi-stable-relay

5: Line 250 to 254 Use a for next loop to do this or create function written in asm if you need to achieve delays or use the timer function with interrupts "Terrible coding "

I inclined to agree with Steve re this You must have flow charts and there is no point in showing people coding without schematics

No one knows what your doing or more importantly why you are doing it this way

Advice here uploads schematics
2: need a decent flow chart then we might be able to assist you

I see you have attached some type of flow chart however this too is very poor

A flow chart should consist of an entry point It should show your for next loops , your while statements , your if else conditions and of course any other loops within the main method

Treat this as constructive criticism not negative critisism we all had to start somewhere with this and I too make mistakes "Im not perfect either " but with programming you have to be as near dammit 99% perfect or at least how you get to your answers has to be consise

As a further point of interest for you your coding may be incorrect but if your flow charts and other explanations are correct then you will find that examiners will more than likely pass you on this alone because your method structure and how you got to your answers was correct

That’s why I always ask for flow charts reasons being that if you were called in to debug software like this for example having a flow chart ready means someone else who has no experience of your design has a far better chance of deciphering your code and making alterations as necessary

This is what they now insist on flow charts as well otherwise you score Zero in other words they fail you

Problem: Unable to stop the color then choose. I have to press and hold the switch button and let go to choose the color.

What I want is change the color everytime I press the button once.

Here are the codes:

[code lang="c"]/*
/*
* File: main.c
* PIC18f45K22
* Author: YP INTI
*
* Remark :
* mastermind games
*
*/

#include
#include
#include

#define _XTAL_FREQ 4000000 //This is the speed your controller is running at
#define delay_ms(x) __delay_ms(x)

/*Configuration fuses, start*/
#pragma config IESO=OFF, FOSC=XT, PRICLKEN=OFF,FCMEN=OFF,PLLCFG=OFF
#pragma config BOREN=OFF, BORV=285, PWRTEN=ON
#pragma config WDTEN=OFF,MCLRE=INTMCLR
#pragma config P2BMX=PORTC0, CCP2MX=PORTC1,PBADEN=OFF,
#pragma config CCP3MX=PORTE0,MCLRE=INTMCLR,HFOFST=OFF,T3CMX=PORTB5
#pragma config DEBUG=OFF,STVREN=ON,XINST=OFF,LVP=OFF

//define derivative
#define DS PORTBbits.RB7
#define ST_CP PORTBbits.RB6
#define SH_CP PORTBbits.RB5
#define LED PORTBbits.RB4

//Global Scope
unsigned char col_player[ 8 ] = {0, 0, 0, 0, 0, 0, 0, 0};
unsigned char col[ 8 ] = {0, 0, 0, 0, 0, 0, 0, 1};
int counter = 0;
int a=1;

//function prototype
void pic_init();

void main() {

pic_init();

delay_ms(100);
A:if( PORTDbits.RD7==1)//push button
{
switch(a){
//001,1st color
case 1://if a =1
{for(int i = 0 ; i < 2; i++ )//produce color 1
{
col[i]=0;
}
for(int i = 2 ; i < 3; i++ )//produce color 1
{
col[i]=1;
}
for(int i = 3 ; i < 8; i++ )//produce color 1
{
col[i]=0;
}
for(int i = 0 ; i < 8 ; i++)//produce color 1
{
DS = col[ i ];
SH_CP = 1;
SH_CP = 0;
}

ST_CP = 1;
ST_CP = 0;//produce color 1

delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);

B: if(PORTDbits.RD7==0)//if release push button
{
a++;//change color
goto A;
}
}
//010
case 2://color 2
{ for(int i = 0 ; i < 1; i++ )//produce color 2
{
col[i]=0;
}
for(int i = 1 ; i < 2; i++ )//produce color 2
{
col[i]=1;
}
for(int i = 2 ; i < 8; i++ )//produce color 2
{
col[i]=0;
}
for(int i = 0 ; i < 8 ; i++)//produce color 2
{
DS = col[ i ];
SH_CP = 1;
SH_CP = 0;
}

ST_CP = 1;
ST_CP = 0;//produce color 2


delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);

if( PORTDbits.RD7==0)
{
a++;
goto A;
}
}


//011
case 3:
{ for(int i = 0 ; i < 2; i++ )//produce color 3
{
col[i]=1;
}
for(int i = 2 ; i < 8; i++ )//produce color 3
{
col[i]=0;
}
for(int i = 0 ; i < 8 ; i++)//produce color 3
{
DS = col[ i ];
SH_CP = 1;
SH_CP = 0;
}

ST_CP = 1;
ST_CP = 0;//produce color 3

delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);

if( PORTDbits.RD7==0)
{
a++;
goto A;
}
}
//100

case 4:
{for(int i = 0 ; i < 1; i++ )//produce color 4
{
col[i]=1;
}
for(int i = 1 ; i < 8; i++ )//produce color 4
{
col[i]=0;
}
for(int i = 0 ; i < 8 ; i++)//produce color 4
{
DS = col[ i ];
SH_CP = 1;
SH_CP = 0;
}

ST_CP = 1;
ST_CP = 0;//produce color 4

delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);

if( PORTDbits.RD7==0)
{
a++;
goto A;
}
}

//110
case 5:
{for(int i = 0 ; i < 1; i++ )//produce color 5
{
col[i]=0;
}
for(int i = 1 ; i < 3; i++ )//produce color 5
{
col[i]=1;
}
for(int i = 3 ; i < 8; i++ )//produce color 5
{
col[i]=0;
}
for(int i = 0 ; i < 8 ; i++)//produce color 5
{
DS = col[ i ];
SH_CP = 1;
SH_CP = 0;//produce color 5
}

ST_CP = 1;
ST_CP = 0;//produce color 5

delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);

if( PORTDbits.RD7==0)
{
a++;
goto A;
}
}
//111

case 6:
{for(int i = 0 ; i < 3; i++ )//produce color 6
{
col[i]=1;
}
for(int i = 3 ; i < 8; i++ )
{
col[i]=0;
}
for(int i = 0 ; i < 8 ; i++)
{
DS = col[ i ];
SH_CP = 1;
SH_CP = 0;
}

ST_CP = 1;
ST_CP = 0;

delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);
delay_ms(100);

if( PORTDbits.RD7==0)
{
a++;
goto A;
}
}


if(a == 7 )//if a=7,loop back to a=1,means it can produce only 6 colors
{
a = 1;
goto A;
}
}
}


}

void pic_init() {
//OSCCON=0B11110000; //sets the IRCF selecting the 16MHz INTOSC and SCS=primary clock
//OSCCON2=0B00000000; //sets MFIOSEL=0 and PRISD=0 turn off external osc drive circuit
//OSCTUNE=0B01000000; //enable PLL TODO
PLLEN = 0; //TODO

SLRCON = 0; //select standard slew rate

TRISA = 0; //for digital output
ANSELA = 0x00; //disable analog input

TRISB = 0x00; //for digital output
ANSELB = 0x00; //disable analog input
RBPU = 0; //weak pull up enable on all PORTB pins

TRISC = 0x00; //this is needed because we don't know if the LCD will set the data line is output or input.
//the TRISC will only be initialized in the LCDinit() function.
ANSELC = 0X00; //disable analog input

TRISD = 0xFF; //for push button
ANSELD = 0; //turn off analog input
TRISE = 0xFF;
ANSELE = 0;
GIEH = 0; //DISABLE ALL INTERRUPTS

}


}[/code]

Here is a good example of how you use proton basic for debounce routines it also provides you with a basic algorithem which is why I like this compiler although you still have to do a flow chart

[code]

' Debounce a switch input (Pulled-Up)
' The LED will toggle On and Off whenever the switch is pressed
'
Include "Amicus18.inc" ' Configure the compiler to use the Amicus18 board

Dim Switch_Count As Byte ' Holds the switch counter amounts

Symbol DetectsInARow = 5 ' The amount of counts to perform
Symbol Switch_Pin = PORTB.4 ' Pin where the switch is connected
Symbol LED = PORTB.0 ' Pin where the LED is connected

Main:
Low LED ' Extinguish the LED
Input Switch_Pin ' Make the switch pin a input

While 1 = 1 ' Create an infinite loop
While Switch_Pin 1 : Wend ' Wait for switch to be released (Pulled-Up)
Switch_Count = 5

Repeat ' Monitor switch input for 5 lows in a row to debounce
If Switch_Pin = 0 Then ' Pressed state detected ?
Inc Switch_Count ' Yes. So increment the counter
Else ' Otherwise...
Switch_Count = 0 ' Reset the counter
EndIf

DelayMS 1 ' Wait for 1ms

Until Switch_Count >= DetectsInARow ' Exit when 5 iterations have been performed
Toggle LED ' Toggle the LED On/Off
Wend

[/code]

Nice little development bourd as well

i just upload a flow chart,and is this flow chart ok?

Now, take a good look at your flowchart. Is there anything in there that would help someone who relies ENTIRELY on what YOU tell them about your project? How does this relate to your original question about a button not functioning as expected? And, what, if anything, is this thing doing? What is the intent of this design?

Put yourself in the position of someone trying to help you -- then supply the information YOU would need, were you that person!

[Hint: your flow chart is too abstract. Also, it doesn't even seem to relate to the code you posted--for example: code: "colors", flow chart: "sounds" or does "feedback" have something to do with "colors"!?! Also, where in the flowchart is there a "button press"?!? Actually, my friend, now that I've looked closer at this, until you unravel the spaghetti in your code, and write a structured program, no Flow Chart is going to help!]