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 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!