C51: Getting to work volatile variable

problem is that variable 'key' is not rechecked inside a function

This is a predictable outcome since the argument is passed to the function by value.
You'd have to pass a pointer to the port register, then the function would access the register when needed. Unfortunately, there is no such thing as a pointer to an SFR in C51. This is a limitation of the 8051 CPU. Follow the link for more information:

http://www.keil.com/support/docs/185.htm

problem is that variable 'key' is not rechecked inside a function

This is a predictable outcome since the argument is passed to the function by value.
You'd have to pass a pointer to the port register, then the function would access the register when needed. Unfortunately, there is no such thing as a pointer to an SFR in C51. This is a limitation of the 8051 CPU. Follow the link for more information:

http://www.keil.com/support/docs/185.htm

Thanks Mike.
This is a great disadvantage of C51/8051 system (I think).

Anyway, some workaround still exists.

Well, remember that other architectures can't normally even manage bit variables - much less pointers to bit variables.

The 8051 is good with bits, and have an extreme code efficiency when accessing SFR, because it has dedicated instructions. But the compromise is that different address ranges have different limitations in how the objects can be addressed. An 8-bit processor can't have so many instructions, unless the decoder is made more complex, and requiring extra code fetches to make the instructions longer.

Ok, I will consider that.

As a beginner which look to problem from C side it is odd to not be able to access certain declared bit address at any level of program no matter on through reference or value. I think, if 'sbit key1=P3^7;' is declared then compiler would have to know what and where is 'key1' and how to handle (get/set) its value.

But I'm totally new in microcontrollers so sorry if my opinion is too offensive :)

The compiler do know ho to handle that bit.

But the compiler is 100% unable to create a pointer to that variable that can be sent as parameter to a function.

Because in C, all parameters are passed by value. So the only way a parameter can access the original content (which is a requirement if that volatile should mean what you intend it to mean), is if the parameter passed by value is a pointer. Then that pointer can be indirected and access the global variable you want.

But then we have already noted that you can't have pointers to bit variables with the 8051 processor, because there are zero processor instructions to support indirect addressing. If the program would run in RAM, it would be possible to create a small piece of self-modifying code that could patch the direct-addressing instruction to specify the correct address of the specific bit. But the compiler can obviously not generate code that is self-modifying and requires the use of RAM - especially since many 8051 chips can't run any code at all from RAM.

So the best you can get is to have something like:

enum {
    MY_BIT_1,
    MY_BIT_2,
    ...
};
void my_function(uchar signal) {
    switch (signal) {
        case MY_BIT_1:
            // code hard-coded for one specific sbit variable/port-pin
            break;
        case MY_BIT_2:
            // code hard-coded for another specific sbit variable/port-pin
            break;
        ...
    }
}

The function could also take an enumerated value representing ports, and a second variable representing bits, and then have one case section for each port and within the case section perform normal logic operations on the 8-bit SFR based on which bit that should be operated on.

I am grateful for wide explanations but still unable to get this working in many tries.
So please help on concrete case...
Regarding Mike's link (under section ANSWER) this should be possible to do.
To reference a hardware bit inside a function regarding on argument's value.

Now is such situation:

sbit key1=P3^7;
sbit key2=P3^6;
volatile bit key; // key now probably don't have to be 'volatile'?
bit key_up(unsigned char mykey);

        if (!key_up('2'))
        {
            green=~green;
        }

        if (!key_up('1'))
        { ... etc...

bit key_up(unsigned char mykey)
{
        switch (mykey)
    {
        case '1':
                //key=key1;
                key=0xB7;
                break;

        case '2':
                //key=key2;
                key=0xB6;
                break;
    }

    if (key==0)                 // check if key is pressed
    {
        delay1ms(20);           // debounce
        if (key==0)             // really pressed
        {
            while(key==0)       // hold until pressed
            {;}
            // key is released
            delay1ms(20);       // debounce
            if (key!=0)         // check if still released
            {
                key=1;          // reset key
                return 0;       // return released
            }
        }
    }
    return 1;   // key was not pressed at all
}

This still don't work as expected.

This still don't work as expected.

That's because you're expecting volatile to work some kind of magic that it never did, and hopefully never will.

A volatile variable is still a variable, it's not a magic tunnel to the actually volatile object (a port pin) whose value you assigned into it at some point. For what you're trying to do here, volatile is of no use at all. Forget about volatile for now.

You need to actually read the port bits explicitly, every time you want to know their status. Yes, that means you have to do your switch()/case block every simple time you do that, to decide which pin bit to read.

So: break the reading of a single button port pin out of the function that handles debouncing, into a function of its own, that does nothing else. Call that function every time you need a pin state.

I find it counter-productive trying to get the code to work without trying to understand how every bit of code works. Obviously, you lack understanding of basic concepts of the programming language. Yet you want us to provide you with a working example. Sorry, I'm not interested in that.

@Mike, really no need for that.
I can leave here well tested and fully workable example of multipurpose, multikey checking key_release routine with advanced double debouncing written in the best manner which C51 with 8051 can do!
For users which may have similar situation in the future.

declaration:

sbit key1=P3^7;
sbit key2=P3^6;
bit debounce(unsigned char AdditionalSemaphore);

usage:

        if (!debounce('2'))
        {
            green=~green;
        }

        if (!debounce('1'))
        {
        ... on_key1_up do something...

function:

bit debounce(unsigned char AdditionalSemaphore)
{
bit MagicTunnelToAlreadyDefinedVariable;

   if (AdditionalSemaphore=='1') MagicTunnelToAlreadyDefinedVariable=key1;
   if (AdditionalSemaphore=='2') MagicTunnelToAlreadyDefinedVariable=key2;

    if (MagicTunnelToAlreadyDefinedVariable==0)
    {
        delay1ms(20);
        if (MagicTunnelToAlreadyDefinedVariable==0)
        {
            while(MagicTunnelToAlreadyDefinedVariable==0)
            {
                if (AdditionalSemaphore=='1') MagicTunnelToAlreadyDefinedVariable=key1;
                if (AdditionalSemaphore=='2') MagicTunnelToAlreadyDefinedVariable=key2;
            }
            delay1ms(20);
            if (MagicTunnelToAlreadyDefinedVariable!=0) return 0;
        }
    }
    return 1;
}

Not nice, really, but works.

Works?

How would your "magic tunnel" change value between the two times you use it?

if (MagicTunnelToAlreadyDefinedVariable==0) {
    delay1ms(20);
    if (MagicTunnelToAlreadyDefinedVariable==0) <= still same variable with zero interaction with hardware
    ...
}

Next thing - your loop locks up the main code while one single button is pressed. That while loop will stay until released. Let's say one button gums up - then your program will permanently lock up.

Next thing - your code only manages one single button at a time. But if you have dedicated processor pins for each button, it's likely that you want to be able to enter input by pressing more than one button. Let's say A+B should force a reset of something. Quite common for example that you press multiple buttons on a watch to enter clock-setting mode. Or resets a stop watch.

But your code there will not leave processing of button A until you release it - how can it then detect that buttton B is pressed at the same time?

In the end, keyboard scan code normally always look at full set of signal lines. Doesn't matter if you have 8 buttons connected to 8 processor pins, or you have 16 buttons in a matrix of 4 rows+4 columns. For individual lines, the code would normally allow random combinations of keys pressed at the same time. For multiplexed keyboards, the code would normally reject as invalid all combinations where two row or two column signals gets activated at the same time.

With your code, you have multiple sections that looks at an input parameter to figure out which hardware signal to look at - this code will quickly grow if you have more buttons. Why do you think you got a suggestion to move this code to a separate helper function?

In the end, it would be much better if you read in all your button bits into a unsigned char and check if any key is pressed. If none is pressed, you are done. If one or more are pressed, you can then make a later check to see if you get the same pattern. If you get a stable pattern, you can report it. If you need multi-key patterns, then you need to have long enough delays in reporting that the user have time to press all buttons before you start to return the first key as a single-press. And you should normally consider always requiring all buttons released before accepting a new press of a single key or a pattern - else you need individual state info up/down for every button so that A+B reported as a pattern isn't later seen as a B press just because the user released A a bit earlier.

There is nothing magical about the 8051. When you take a copy of a hardware state, that copy is frozen. Volatile can't help. And you can't use pointers. So if you want to operate on a state that can change inside your function, then you need to go back to the actual hardware state every time.

Always program using a layered approach.
So one function handling actions based on what was pressed.
One function looking at time, to decide the debounce.
One function looking at current hardware state to find current press state.

It's nicer to have a main loop looking like:

buttons_acceptable = TRUE;
for (;;) {
    buttons = button_info();
    switch (buttons) {
        case 0:
            // No button pressed, so rearm acceptance of button presses.
            buttons_acceptable = TRUE;
        case BUTTON_A:
            handle_button_a();
            break;
        case BUTTON_B:
            handle_button_b();
            break;
        case BUTTON_C:
            handle_button_c();
            break;
        case BUTTON_A+BUTTON_B:
            handle_button_ab();
            break;
        ...
        case STILL_DEBOUNCING:
            // seeing button presses, but haven't decided yet.
            break;
        default:
            handle_invalid_button_combination();
    }
}

And when you write the layer that does the debouncing - let it count time. But let it return instantly. Don't lock up the main loop waiting for the debounce. Or waiting for the release of a button. Your main may have lots of other fun things to do, while waiting.

Seem's you are right.
But Ok, here is enchanced version with shortened variable names :) of my tunnel which becomes uglier with every new version :)
Thanks for a revision.

bit debounce(unsigned char pKey)
{
bit localKey;

   if (pKey=='1') localKey=key1;
   if (pKey=='2') localKey=key2;

   if (localKey==0)
    {
        delay1ms(20);
        if (pKey=='1') localKey=key1;
        if (pKey=='2') localKey=key2;

        while(localKey==0)
        {
            if (pKey=='1') localKey=key1;
            if (pKey=='2') localKey=key2;
        }
        delay1ms(20);
        if (localKey!=0) return 0;
    }
    return 1;
}

As a beginner in first steps with microcontroller I have never need to check multiple buttons at once but I mostly understand those bunch of useful tips on how to set a problem in such situation.

Thanks for that!