Help required

Hello everybody,

I am a newbie in the field of microcontrollers and embedded programming. I am trying to implement a basic version of the hangman game as a project for my course. (I have to use 8051)

I am using RIDE software for coding in C. I only have a rough idea as to how i should go about writing the code. By referring to the examples, I tried to just declare my functions and my main program. I am yet to define the functions and I am not sure that what I have done so far is correct.

Could you look into it and tell me whether I am doing alright as of now or am I hopelessly lost?

Thanks!

Reply to
akshaychander
Loading thread data ...

Here is the code:

#include

char code word1[] = {'A','B','S','O','L','U','T','E'}; //WORD TO BE GUESSED char code table1[] = {'A','B','C','D','E','F','G','H','I'}; //LOOKUP TABLE1 FOR KEYPAD char code table2[] = {'J','K','L','M','N','O','P','Q','R'}; //LOOKUP TABLE2 FOR KEYPAD char code table3[] = {'S','T','U','V','W','X','Y','Z'}; //LOOKUP TABLE3 FOR KEYPAD void LCD_INIT(void){} //ROUTINE TO INITIALIZE LCD WITH 8 UNDERSCORES

char GET_CHAR(void){ //KEYPAD ROUTINE TO GET A CHARACTER /*THIS ROUTINE WILL GET TWO CHARACTERS FROM THE KEYPAD. THE FIRST CHARACTER WILL BE 1,2 OR 3 AND WILL DECIDE THE LOOKUP TABLE. THE SECOND CHARACTER RANGING FROM 1-9 WILL DETERMINE THE CHARACTER FROM THE LOOKUP TABLE

*/ }

void LCD_DISPLAY(char c,int i){} //LCD ROUTINE TO DISPLAY A CHAR IN POSITION i

void DISPLAY_MESG(char* str){} //LCD ROUTINE TO DISPLAY A MESSAGE IN LINE 2

void LEDDISP(int i){} //TO SWITCH ON THE i'th LED

void main(void){ data char keypressed; //A BYTE IN THE RAM FROM 30H TO 7FH int i,ccnt=0,wcnt=0; char found='N'; P2=255; //P2 IS FOR INPUT FOR KEYPAD COLUMNS LCD_INIT(); while(1){ keypressed=GET_CHAR(); i=0; while(i

Reply to
Akshay

So sorry about the horrible formatting! Is there any way for me to edit the message?

Reply to
Akshay

#include

char code word1[] = {'A','B','S','O','L','U','T','E'}; //WORD TO BE GUESSED char code table1[] = {'A','B','C','D','E','F','G','H','I'}; //LOOKUP TABLE1 FOR KEYPAD char code table2[] = {'J','K','L','M','N','O','P','Q','R'}; //LOOKUP TABLE2 FOR KEYPAD char code table3[] = {'S','T','U','V','W','X','Y','Z'}; //LOOKUP TABLE3 FOR KEYPAD

void LCD_INIT(void){}//ROUTINE TO INITIALIZE LCD WITH 8 UNDERSCORES

char GET_CHAR(void){ //KEYPAD ROUTINE TO GET A CHARACTER /*This routine will get two characters from the keypad and determine which alphabet corresponds to the combination

*/ }

void LCD_DISPLAY(char c,int i){} //LCD ROUTINE TO DISPLAY A CHAR IN POSITION i

void DISPLAY_MESG(char* str){} //LCD ROUTINE TO DISPLAY A MESSAGE IN LINE 2

void LEDDISP(int i){} //TO SWITCH ON THE i'th LED

void main(void){ data char keypressed; //A BYTE IN THE RAM FROM 30H TO 7FH int i,ccnt=0,wcnt=0; char found='N'; P2=255; //P2 IS FOR INPUT FOR KEYPAD COLUMNS LCD_INIT(); while(1){ keypressed=GET_CHAR(); i=0; while(i

Reply to
Akshay

#include

char code word1[] = {'A','B','S','O','L','U','T','E'}; //WORD TO BE GUESSED char code table1[] = {'A','B','C','D','E','F','G','H','I'}; //LOOKUP TABLE1 FOR KEYPAD char code table2[] = {'J','K','L','M','N','O','P','Q','R'}; //LOOKUP TABLE2 FOR KEYPAD char code table3[] = {'S','T','U','V','W','X','Y','Z'}; //LOOKUP TABLE3 FOR KEYPAD

void LCD_INIT(void){}//ROUTINE TO INITIALIZE LCD WITH 8 UNDERSCORES

char GET_CHAR(void){ //KEYPAD ROUTINE TO GET A CHARACTER /*This routine will get two characters from the keypad and determine which alphabet corresponds to the combination

*/ }

void LCD_DISPLAY(char c,int i){} //LCD ROUTINE TO DISPLAY A CHAR IN POSITION i

void DISPLAY_MESG(char* str){} //LCD ROUTINE TO DISPLAY A MESSAGE IN LINE 2

void LEDDISP(int i){} //TO SWITCH ON THE i'th LED

void main(void){ data char keypressed; //A BYTE IN THE RAM FROM 30H TO 7FH int i,ccnt=0,wcnt=0; char found='N'; P2=255; //P2 IS FOR INPUT FOR KEYPAD COLUMNS LCD_INIT(); while(1){ keypressed=GET_CHAR(); i=0; while(i

Reply to
Akshay

Great. Now you've guaranteed that the OP will not actually LEARN anything.

--Gene

Reply to
Gene S. Berkowitz

That was the OP following up (with the program he's copied from another student, probably).

pete

--
pete@fenelon.com [Support no2id.net: working to destroy Blair's ID card fraud]
Reply to
Pete Fenelon

Gene S. Berkowitz scrobe on the papyrus:

If he writes C code with "goto" in it he's not capable of learning anything anyway.

--
John B
Reply to
John B

You haven't lived until you've debugged code like this (there are NO errors on my part in this code, it's pseudo representation of ACTUAL code I've been called upon to fix in the past):

int func(int param) { if (param) do_something((int) do_something_else); else if (param) do_something_else(); }

int do_something (int param) { if (param == 1) do_something_else(); label: asm { mov a,#1 call do_something_else jmp do_something } }

int do_something_else(void) { if (global_variable) goto label; else asm { ret; } }

Just what the hell this code was supposed to do, your guess is as good as mine. I lost track trying to trace through just how it ever exited given that you have C calling assembly and stack frames appearing and disappearing all over the place.

Reply to
larwe

IMHO, the use of goto is quite appropriate in this case, since C only supports exiting the innermost loop with break. A selective use of goto can often improve code readability and reduces the level of indent required.

Some seems to fear the "horrible" word goto so much that they rather use complex flags to control loop execution etc. making programs very hard to read.

Paul

Reply to
Paul Keinanen

What the hell is that code supposed to do? That is just downright nasty way of coding. I don't know if the person who wrote that was still working there when you were called upon to fixed the code, I would suggest that he or she go to some seminar at least to avoid a problem like that again!!

-Isaac

Reply to
Isaac Bosompem

Hmmm, I am a newbie in this field and I am trying to find my way. So I am sorry if my code is terrible.Which is why I am asking for help.

I am trying to interafce LCD along with keypad along with 8051 to implement the hangman game. I am using RIDE. And this is as far as I have got.(after going through the examples).

word1[] is the word to be used in the game.

tables1,2,3 are lookup tables for the hex keypad.

This code WILL not work as I have only declared most of my subroutines.

I am getting a character from the user, checking whether it is there in the word, if so displaying it in the coorect position in the LCD. I also check whether the number of correct guesses equals 8(length of word). If so I display "Congrats"

Else, I increment the number of wrong guesses and check whether it equals 4. If so I display "Game Over"

If any of you have suggestions or advice, I am ready to listen

I posted here only to get help, not to get flamed.

Reply to
Akshay

I'm still not entirely sure. I mean, what do you do - besides maybe resign - when you see a piece of code with comments and structure like this:

if (flags && FLAG_1) // is FLAG_1 clear? { do_something();

// select thing to do based on FLAG_1 if (flags & FLAG_1) do_something_else(); else if (flags & FLAG_1) do_something_different();

special_setting = 0x80; // uint8_t type special_setting = special_setting

Reply to
larwe

Well Akshay, I was not referring to your code. I was referring to the code Iarwe posted.

Your code looks decent and is readable. The genral consensus of C pro's though is to avoid using goto. To tell you the truth, easier said than done in some specific cases.

-Isaac

Reply to
Isaac Bosompem

If they have the QA department, they are supposed to have code reviews, aren't they? If they don't have code reviews, what 'Q' stands for than? I know, the questions are rhetorical :)

I saw the code review, where the reviewee had left a section of code surrounded with "#if 0 / #endif" pair. The coding standard (yes, there was one) explicitly prohibited such (ab)use of preprocessor. The reviewer had to perform a lecture that "#if 0" could not be used. The next bunch of diffs contained, among others, the following wonderful lines:

137c137 < #if 0
--
> #if 1
Reply to
Vadim Barshaw

Reminds me of some C code on the internet that I saw in which someone decided to use a base 2 log to determine if a bit in a variable was set!! I mean, why do these complicated things if there is a much easier, simpler (and MUCH faster) way to accomplish it?

yikes, I never knew anyone that would not use a device simply because it served its purpose.

Again, you'd think people would not do things like this to make debugging easy. I just cannot imagine why something like you described above would be a good idea.

Reminds me of some Amiga based code I was reading about in which a demo programmer (using I think a loader) overwrote some critical OS data structure and still managed to run without a hitch!! Perhaps he should take a shot at the slot machines ;).

You'd think that the developers working on this stuff would go out of their way to make debugging the firmware of these products much simpler and easier, rather than obfuscating simple logic testing structures like that! How do they debug code like that?!

hahaha, It might do everyone else some good too.

Also Akshay, you can basically take the examples Iarwe posted as pitfalls to avoid when coding for an embedded system. Not only will you save yourself headaches and stress, you will have a lot more fun doing your project :).

-Isaac

Reply to
Isaac Bosompem

they?

questions are rhetorical :)

Actually there are "good" answers to these questions. This code predates our current ISO procedures. At the time it went through, I believe the official procedure was "test it in engineering, throw it over the wall, and hope". I believe it is currently at its fortieth or so QA release...

Reply to
larwe

Most likely the person had previously only been working with languages that did not support bitwise operations.

When people with wildly different backgrounds work on a project, the project manager should follow the new persons quite closely for a few months to get a way with their previously learned "bad habits".

I have been working with realtime control systems for quite a while, I noticed that it is very hard to learn people with batch processing background to understand the concepts of multitasking. Persons that have only worked with 8 bitters are very eager to sneak some busy loops into the code when they think there is nothing useful to do for a while and Windows programmers tend use a huge amount of resources indiscriminately :-).

When getting new people with very different background into a new project, quite a lot of resources should be allocated for familiarisation to the new environment in the beginning of the project.

The worst thing to do when the deadlines are approaching very fast, is to add new persons with wildly different experience into the project ("he/she can program and thus help the project finish in time"), since the current project staff would have to supervise the newcomers that will not be very productive in a month or two.

Paul

Reply to
Paul Keinanen

Hello ,

I also worked on interfacing a LCD and a keyboard to 8051 before 6 years! It was an interesting experience. You should be very careful while initialising the LCD, I mean the timing signal generated by your

8051 using for loop. Btw, which LCD you are using. Do you have any idea about the hardware level interfacing - how LCD is connected to 8051, through multiplexers, buffers, etc...

Best Regards, Vivekanandan M

Reply to
Vivekanandan M

Strike 1: Casting a function pointer to an integer.

Strike 2: comparing a parameter that may well be a casted function pointer to an integer literal. That's completely out of line.

Strike 3: goto from one function to another. In an anywhere nearly sane toolchain, this shouldn't even compile.

That's three counts of totally inadmissable C code, before we even start looking at violent abuse of inline assembly. Let's just say that if that's not immediate grounds for firing whoever wrote this, and everybody else who let it slip past after having actually seen it, and closely re-evaluating everybody ever hired by any of them, I find it hard to imagine what might be.

The only way to understand what this nightmare might do probably is to take a full day looking at the assembly code, after having taken a very good look into the platforms's C ABI. Then rewrite it from scratch either completely in C, or completely in ASM.

I strongly suggest you convince management to set a prize of a couple thousand dollars' worth of stocks in the company for fixing this horror.

--
Hans-Bernhard Broeker (broeker@physik.rwth-aachen.de)
Even if all the snow were burnt, ashes would remain.
Reply to
Hans-Bernhard Broeker

ElectronDepot website is not affiliated with any of the manufacturers or service providers discussed here. All logos and trade names are the property of their respective owners.