How am I doing so far?

A sampling of code that I've translated from 680x assembly, formatted into C++, and tested in an online compiler.

Code:

int keyCheck ( bool enter, bool check, bool next, bool repeat, bool testKey, bool restart, char keyDat, char keyNum ) // Replacement for UGLY, BELABORED code at $FAA6 // Presently based on matrix keyboard data fetched through 8279 // "Old matrix" refers to custom Grayhill keypad in original unit - // values shuffled around to accommodate off-the-shelf version // A partial rewrite may be necessary once a hardware platform is // chosen but keyNum and Boolean variables are finalized { enter = false; // Old matrix: 0x1A - Accept data entered from keypad check = false; // Old matrix: 0x0B - Perform checksum on entered data next = false; // Old matrix: 0x1B - Proceed to next programmed test repeat = false; // Old matrix: 0x13 - Repeat current test testKey = false; // Old matrix: 0x03 - Perform Lamp Test restart = false; // Old matrix: 0x19 - Reboot the unit! keyNum = 0xFF; // set default 'NaN' value for keyNum if ( keyDat == 0x00 ) keyNum = 0x01; // Key 1 else if ( keyDat == 0x01 ) keyNum = 0x02; // Key 2 else if ( keyDat == 0x02 ) keyNum = 0x03; // Key 3 else if ( keyDat == 0x03 ) enter = true; // Key A = Enter else if ( keyDat == 0x08 ) keyNum = 0x04; // Key 4 else if ( keyDat == 0x09 ) keyNum = 0x05; // Key 5 else if ( keyDat == 0x0A ) keyNum = 0x06; // Key 6 else if ( keyDat == 0x0B ) check = true; // Key B = Check else if ( keyDat == 0x10 ) keyNum = 0x07; // Key 7 else if ( keyDat == 0x11 ) keyNum = 0x08; // Key 8 else if ( keyDat == 0x12 ) keyNum = 0x09; // Key 9 else if ( keyDat == 0x13 ) next = true; // Key C = Next else if ( keyDat == 0x18 ) restart = true; // Key * = Restart else if ( keyDat == 0x19 ) keyNum = 0x00; // Key 0 else if ( keyDat == 0x1A ) testKey = true; // Key # = Lamp Test else if ( keyDat == 0x1B ) repeat = true; // Key D = Repeat } int varLoad ( char MSG01, char MSG02, char MSG03, char MSG04, char MSG05, char MSG06, char MSG07, char MSG08, char MSG09, char MSG10, char MSG11, char MSG12, char MSG13, char MSG14, char MSG15, char MSG16, char MSG17, char MSG18, char MSG19, char MSG20, char MSG21, char MSG22, char MSG23, char MSG24, char MSG25, char MSG26, int index ) // Replaces block of code at $FCB8 and data table from $FCE3 - $FD5A // This block of variables was so named for assumption it // was a 'message block' but may actually be for other purposes // MSG01 and MSG02 are constants while rest were read from the table { MSG01 = 0xAA; MSG02 = 0xBA; MSG03 = 0xFF; if ( index == 1 || index == 4 ) MSG04 = 0x55; else MSG04 = 0x00; if ( index == 1 || index == 4 ) MSG05 = 0x55; else MSG05 = 0x00; if ( index == 1 ) MSG06 = 0x56; else if ( index == 2 ) MSG06 = 0x02; else if ( index == 3 ) MSG06 = 0x01; else if ( index == 4 ) MSG06 = 0x55; else if ( index == 5 ) MSG06 = 0x01; if ( index == 1 ) MSG07 = 0x22; else if ( index == 2 ) MSG07 = 0x3F; else if ( index == 3 ) MSG07 = 0x35; else if ( index == 4 ) MSG07 = 0x1F; else if ( index == 5 ) MSG07 = 0x3A; MSG08 = 0x00; if ( index == 2 || index == 3 ) MSG09 = 0xCF; else MSG09 = 0xC0; if ( index == 1 ) MSG10 = 0x00; else if ( index == 2 ) MSG10 = 0xF0; else if ( index == 3 ) MSG10 = 0xF0; else if ( index == 4 ) MSG10 = 0x00; else if ( index == 5 ) MSG10 = 0x03; if ( index == 2 ) MSG11 = 0x10; else if ( index == 5 ) MSG11 = 0xC0; else MSG11 = 0x00; if ( index == 4 ) MSG12 = 0x0F; else if ( index == 5 ) MSG12 = 0x03; else MSG12 = 0x00; if ( index == 4 ) MSG13 = 0xF0; else if ( index == 5 ) MSG13 = 0xC0; else MSG13 = 0x00; if ( index == 1 ) MSG14 = 0x00; else if ( index == 2 ) MSG14 = 0x00; else if ( index == 3 ) MSG14 = 0x44; else if ( index == 4 ) MSG14 = 0x34; else if ( index == 5 ) MSG14 = 0x44; if ( index == 1 || index == 2 ) MSG15 = 0x0C; else MSG15 = 0x0F; if ( index == 1 || index == 2 ) MSG16 = 0x00; else MSG16 = 0xFF; if ( index == 1 || index == 2 ) MSG17 = 0x00; else MSG17 = 0xFF; if ( index == 1 || index == 2 ) MSG18 = 0x00; else MSG18 = 0xFF; if ( index == 1 || index == 2 ) MSG19 = 0x00; else MSG19 = 0xFF; if ( index == 1 ) MSG20 = 0x01; else if ( index == 2 ) MSG20 = 0x02; else MSG20 = 0xFF; if ( index == 1 || index == 2 ) MSG21 = 0x40; else MSG21 = 0xFF; if ( index == 1 || index == 2 ) MSG22 = 0x00; else MSG22 = 0xFF; MSG23 = 0x41; if ( index == 1 || index == 2 ) MSG24 = 0x03; else MSG24 = 0x02; MSG25 = 0x00; if ( index == 1 || index == 2 ) MSG26 = 0x15; else MSG26 = 0x0F; }

Thoughts? Comments?

In C (and C++, and most other languages), when you have a "parameter" to a function, that parameter is considered local. That is, you usually receive that parameter on a call stack, and unless there is some kind of annotation (like a reference in C++, or an out parameter in Ada) the expectation is that the parameter on the stack will be thrown away when the function returns.

You are declaring functions that take parameters, and you are modifying the parameters -- perfectly legitimate things to do in C -- but it seems like you expect those values to be permanently saved. That just won't happen.

Instead, you'll have to either use pointers (bad idea) or declare some global state. I'd suggest one or more structures:

Code:

#include <stdbool.h> #include <stdint.h> union { struct keys_pressed { bool kp_enter; // Accept data entered from keypad bool kp_checksum; // Perform checksum on entered data bool kp_next; // Proceed to next test bool kp_repeat; // Repeat the current test bool kp_lamp_test; // Perform lamp test bool kp_restart; // Restart the unit uint8_t kp_digit; // Some key '0'..'9' }; uint8_t as_bytes[sizeof (strict keys_pressed)]; } Keys_pressed; enum { KP_NO_DIGIT = 0xFF, }; enum scan_code { SC_KEY_1 = 0x00, SC_KEY_2 = 0x01, SC_KEY_3 = 0x02, SC_KEY_A = 0x03, SC_KEY_4 = 0x08, SC_KEY_5 = 0x09, SC_KEY_6 = 0x0A, SC_KEY_B = 0x0B, SC_KEY_7 = 0x10, SC_KEY_8 = 0x11, SC_KEY_9 = 0x12, SC_KEY_C = 0x13, SC_KEY_STAR = 0x18, SC_KEY_0 = 0x19, SC_KEY_HASH = 0x1A, SC_KEY_D = 0x1B, SC_NUM_CODES = 16, };

Then you can use a switch statement to replace that long chain of if/else code:

Code:

static inline void clear_keys_pressed(void) { for (uint8_t i = 0; i < sizeof (Keys_pressed); ++i) Keys_pressed.as_bytes[i] = 0; Keys_pressed.kp_digit = KP_NO_DIGIT; } void decode_keypress( uint8_t scancode) { clear_keys_pressed(); // TODO: If your C compiler gives you good control over // where data is placed, you might want to turn this switch // into a sparse table in ROM. #define CASE_DIGIT(D) case SC_KEY_##D: Keys_pressed.kp_digit = D; break #define CASE_BOOL(SC, NAME) case SC_KEY_##SC: Keys_pressed.kp_##NAME = true; break switch (scancode) { CASE_DIGIT(0); CASE_DIGIT(1); CASE_DIGIT(2); CASE_DIGIT(3); CASE_DIGIT(4); CASE_DIGIT(5); CASE_DIGIT(6); CASE_DIGIT(7); CASE_DIGIT(8); CASE_DIGIT(9); CASE_BOOL(A, enter); CASE_BOOL(B, checksum); CASE_BOOL(C, next); CASE_BOOL(D, repeat); CASE_BOOL(STAR, restart); CASE_BOOL(HASH, lamp_test); default: assert(!"Impossible scancode value"); // FIXME: Or is there a "no key pressed" value that might // come through? If so, just eat it and continue. } #undef CASE_DIGIT #undef CASE_BOOL }

It looks like you're doing the same thing with the MSG variables. I understand that you don't have a purpose figured out for those variables -- and in fact, they may be temporary variables sometimes.

So let's declare them globally and use arrays to initialize them:

Code:

union { struct msg_vars { uint8_t msg01; uint8_t msg02; uint8_t msg03; uint8_t msg04; uint8_t msg05; uint8_t msg06; uint8_t msg07; uint8_t msg08; uint8_t msg09; uint8_t msg10; uint8_t msg11; uint8_t msg12; uint8_t msg13; uint8_t msg14; uint8_t msg15; uint8_t msg16; uint8_t msg17; uint8_t msg18; uint8_t msg19; uint8_t msg20; uint8_t msg21; uint8_t msg22; uint8_t msg23; uint8_t msg24; uint8_t msg25; uint8_t msg26; }; uint8_t as_bytes[sizeof (struct msg_vars)]; } Msg_vars; #define MSGVAR(NN) Msg_vars.msg##NN #define MSG01 MSGVAR(01) #define MSG02 MSGVAR(02) #define MSG03 MSGVAR(03) #define MSG04 MSGVAR(04) #define MSG05 MSGVAR(05) #define MSG06 MSGVAR(06) #define MSG07 MSGVAR(07) #define MSG08 MSGVAR(08) #define MSG09 MSGVAR(09) #define MSG00 MSGVAR(10) #define MSG11 MSGVAR(11) #define MSG12 MSGVAR(12) #define MSG13 MSGVAR(13) #define MSG14 MSGVAR(14) #define MSG15 MSGVAR(15) #define MSG16 MSGVAR(16) #define MSG17 MSGVAR(17) #define MSG18 MSGVAR(18) #define MSG19 MSGVAR(19) #define MSG20 MSGVAR(20) #define MSG21 MSGVAR(21) #define MSG22 MSGVAR(22) #define MSG23 MSGVAR(23) #define MSG24 MSGVAR(24) #define MSG25 MSGVAR(25) #define MSG26 MSGVAR(26) void reset_msg_vars( uint8_t index) { // Replaces block of code at $FCB8 and data table from $FCE3 - $FD5A // This block of variables was so named for assumption it // was a 'message block' but may actually be for other purposes // MSG01 and MSG02 are constants while rest were read from the table MSG01 = 0xAA; MSG02 = 0xBA; MSG03 = 0xFF; switch (index) { case 1: MSG04 = 0x55; MSG05 = 0x55; MSG06 = 0x56; MSG07 = 0x22; MSG08 = 0x00; MSG09 = 0xC0; MSG10 = 0x00; MSG11 = 0x00; MSG12 = 0x00; MSG13 = 0x00; MSG14 = 0x00; MSG15 = 0x0C; MSG16 = 0x00; MSG17 = 0x00; MSG18 = 0x00; MSG19 = 0x00; MSG20 = 0x01; MSG21 = 0x40; MSG22 = 0x00; MSG23 = 0x41; MSG24 = 0x03; MSG25 = 0x00; MSG26 = 0x15; break; // AAAARRRRGGGH! This sucks! There must be a better way! } }

Writing the initializers out like this will probably produce better code than the if/else if/else expressions you were using, simply because it moves all the comparing and branching into a single place, and replaces it with just a bunch of loads and stores.

But wait, there's more!

Since you're initializing all the values all the time, there's definitely an easier way. We'll take advantages of the

Code:

.as_bytes

half of the union, and just use a set of arrays:

Code:

enum { NUM_RESET_STATES = 5 }; // Should go in ROM. You may need #pragmas or something for that const uint8_t MSG_VARS_INIT[NUM_RESET_STATES + 1][sizeof(Msg_vars)] = { // Used by reset_msg_vars. Note that `index` gets reduced // by 1 so that we can index this array from 0. [0] = { 0xAA, 0xBA, 0xFF, 0x55, 0x55, 0x56, 0x22, 0x00, 0xC0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x01, 0x40, 0x00, 0x41, 0x03, 0x00, 0x15, }, [1] = { 0, /* FIXME: Fill this in */ }, [2] = { 0, /* FIXME: Fill this in */ }, [3] = { 0, /* FIXME: Fill this in */ }, [4] = { 0, /* FIXME: Fill this in */ }, // Default case, where index is not 1..5. Is this even possible? [5] = { 0, /* FIXME: Fill this in */ }, }; void reset_msg_vars( uint8_t index) { if (index < 1 || index > 5) index = 6; uint8_t * init_ptr = MSG_VARS_INIT[index - 1]; // -1 for 0-based array for (uint8_t i = 0; i < sizeof (Msg_vars); ++i) Msg_vars.as_bytes[i] = init_ptr[i]; }

Aww, yeah! That's the stuff!

aghast: note that this is the C++ programming forum, so you can and should assume C++ by default, not C.

Quote:

Originally Posted by laserlight

aghast: note that this is the C++ programming forum, so you can and should assume C++ by default, not C.

Wasn't the code that aghast posted valid C++?

Quote:

Originally Posted by Sir Galahad

Wasn't the code that aghast posted valid C++?

I think so, but I haven't checked with a C++ compiler. However, it is clear from the text and the style of the code that aghast felt the need to assume C while referencing C++ to answer the question while staying on topic for the forum, whereas that was unnecessary since this really is the C++ programming forum.

Quote:

Originally Posted by laserlight

I think so, but I haven't checked with a C++ compiler. However, it is clear from the text and the style of the code that aghast felt the need to assume C while referencing C++ to answer the question while staying on topic for the forum, whereas that was unnecessary since this really is the C++ programming forum.

Nevertheless most C constructs CAN be safely used in C++ programs. As long as it compiles and doesn't introduce problematic side-effects, why not?

Quote:

Originally Posted by Sir Galahad

Nevertheless most C constructs CAN be safely used in C++ programs. As long as it compiles and doesn't introduce problematic side-effects, why not?

Because it would be additional effort to have to phrase the explanations for and code in C while also targeting C++ (hence if you want to be sure that "it compiles and doesn't introduce problematic side-effects", you have to check twice) when the help seeker didn't ask for it.

Also, possibly less relevant in this case due to the context of translating from assembly (hence my question in another thread as to why the switch from C to C++ in the first place), but an answer coming from C might miss out on idiomatic C++ approaches, even if the person providing the answer is a skilled C++ programmer since their focus would be on C approaches.

Anyway, this was supposed to be a note to help aghast for future contributions to this forum. It has nothing to do with you unless you're intending to confuse yourself concerning these two areas of this board, which admittedly I've done sometimes ;)

Quote:

Originally Posted by laserlight

Because it would be additional effort to have to phrase the explanations for and code in C while also targeting C++ (hence if you want to be sure that "it compiles and doesn't introduce problematic side-effects", you have to check twice) when the help seeker didn't ask for it.

Also, possibly less relevant in this case due to the context of translating from assembly (hence my question in another thread as to why the switch from C to C++ in the first place), but an answer coming from C might miss out on idiomatic C++ approaches, even if the person providing the answer is a skilled C++ programmer since their focus would be on C approaches.

Anyway, this was supposed to be a note to help aghast for future contributions to this forum. It has nothing to do with you unless you're intending to confuse yourself concerning these two areas of this board, which admittedly I've done sometimes

Well there was nothing wrong with that code and certainly no good reason to raise an issue about it. Remember, languages are just tools...

Quote:

Originally Posted by Sir Galahad

Well there was nothing wrong with that code and certainly no good reason to raise an issue about it. Remember, languages are just tools...

I didn't raise an issue about aghast's code. I noted "that this is the C++ programming forum, so you can and should assume C++ by default, not C". Remember, one should use tools that are appropriate for the job...

Quote:

Originally Posted by Sir Galahad

Wasn't the code that aghast posted valid C++?

Turns out, no!

First, I explicitly referenced C (vice C++) in my text. So, yeah, @laserlight had it right -- I had been in the C forum previously, and just assumed that @etech's code was C since it looked like C.

Second, I used "void" in function parameters, and anonymous unions/structs, both of which (I think) are invalid C++. (GCC and Microsoft both support it, I believe. But it still should have a comment or something.)

Quote:

Originally Posted by aghast

Turns out, no!

First, I explicitly referenced C (vice C++) in my text. So, yeah, @laserlight had it right -- I had been in the C forum previously, and just assumed that @etech's code was C since it looked like C.

Second, I used "void" in function parameters, and anonymous unions/structs, both of which (I think) are invalid C++. (GCC and Microsoft both support it, I believe. But it still should have a comment or something.)

Well I stand corrected!

On a side note, wouldn't it be nice if Stroustrup had made C++ completely backward-compatible with C? Just seems like it would have made things easier in the long run...

Quote:

Originally Posted by Sir Galahad

On a side note, wouldn't it be nice if Stroustrup had made C++ completely backward-compatible with C? Just seems like it would have made things easier in the long run...

No, because that would remove some of the protections (e.g., type safety) that C++ provides over C. C lets you get away with more implicit type conversions than C++, and that would hamper C++'s ability to be more strict about them.

Also, how do you introduce new keywords (like "class", "new", and "delete") while staying completely backward-compatible with existing C code that happens to use those keywords as identifiers? Imagine trying to design a C-based object-oriented language that is still compatible with C code like this:

Code:

struct class { int public, private; }; int delete(bool new); int try(double operator); int catch(short friend); int throw(struct class *class);

:D

Quote:

Originally Posted by christop

No, because that would remove some of the protections (e.g., type safety) that C++ provides over C. C lets you get away with more implicit type conversions than C++, and that would hamper C++'s ability to be more strict about them.

Also, how do you introduce new keywords (like "class", "new", and "delete") while staying completely backward-compatible with existing C code that happens to use those keywords as identifiers? Imagine trying to design a C-based object-oriented language that is still compatible with C code like this:

Code:

struct class { int public, private; }; int delete(bool new); int try(double operator); int catch(short friend); int throw(struct class *class);

:D

LOL. Yeah I just realized that while I was on my bike ride. Meh...

Quote:

Originally Posted by aghast

Second, I used "void" in function parameters, and anonymous unions/structs, both of which (I think) are invalid C++.

The use of void to mean an empty parameter list is valid, but unusual in C++ because it has no benefit, whereas in C it can distinguish between an empty parameter list and a declaration of a function with an unknown number of parameters. I'm less certain for anonymous unions and structs, but I believe that they are valid too (and a quick check with a compiler seems to show this is true), though the C++ emphasis on type makes them less lucrative.

Quote:

Originally Posted by christop

Also, how do you introduce new keywords (like "class", "new", and "delete") while staying completely backward-compatible with existing C code that happens to use those keywords as identifiers? Imagine trying to design a C-based object-oriented language that is still compatible with C code like this:

It may be possible, though it presumably will impose additional burden on compiler authors and allow for code that's simply harder to read with more difficult to understand error messages. I'm thinking of how in programming language design there's a distinction made between keywords and reserved words: it is usually easier for everyone for keywords to be reserved words, but in theory if it is possible to distinguish by syntax, then keywords don't necessarily have to be reserved words, e.g., it may be possible to unambiguously determine from syntax that try is a function name (or function pointer name when used as an object) rather than the keyword starting an exception try block.

Just sticking my nose back in for a few minutes... will have to take a bit of time and digest the information at the start. I do appreciate the information - like I said, I'm not looking for "eh, it'll compile", I want this to be good C++.

As for variables, There are a fair number of sub-functions and program blocks that depend on the variable contents persisting as the program goes along.

the varLoad function selects one of five groups of numbers and loads them into the MSG variables. Would an array work better? define five strings then I do something like varLoad ( 2 ) and it loads the second series of numbers into the MSG01 - MSG26 variable set?

But I think I might be getting ahead of myself again. Let me digest the information then come back.

I wound up redoing the transliteration I'd been working on, and have about 3/4 of the code ready for optimization and final rewrite.

What remains may leave me bald by the time I finish.

I do now see that I will have to have at least four arrays - two based on constant tables that were in the original ROM (one 19-character x 1 row and the varLoad table of constants - I've defined it as a 26 x 5 array), one (maybe two) for the 8-digit numeric readout, and one for a specific section of RAM as there are multiple operations involved using the index register.

1 Attachment(s)

So, I've been working on the interrupt code and have run into an error I just can't see for some reason and it doesn't manifest if I try and cut it down and then force the error so I've just attached the whole section as is.

If you try and compile it as it lies, it will complain about an overflow in one label (which I am aware of; it will be replaced by an array pointer soon, so I'm not worried - for the moment)

Then the other error will be that the label NMI02 is referenced but not defined.

That's where I'm having a problem.
That line is currently commented out.

If I take off the comment flag and make the NMI02 label live, then the compiler just flips out and belches out a LONG list of 'undefined variable' errors.

Those are undefined reference errors!
Like
foo.cpp:(.text+0x89c): undefined reference to `ram_080'

It's because you have a main(), and the program compiled successfully.

If you want to verify that it actually compiles as a single file, without trying to make an executable out of it, do this.

Code:

g++ -c interrupt2.cpp

(I fixed it somehow... I wound up doing a line by line comparison with the original 6801 disassembly and made a bunch of clarifications and corrections, tried again, and the compiler was in a better mood. Would have liked to know what I did so I know not to do it again!)