Direction Pad - movement

[samuelmoneill]

I am writing a menu system for an LCD screen. The screen has 6 buttons, Up, Down, Left, Right, Select, Cancel.

I have written the following code to scroll up and down through the menu. Here is my code

void position_cursor(void)
{
    cursor[0] = 0x10; // 0x10 : cursor select character
    clear_cursor[0] = 0x20; //clear cursor character
   // Up Arrow Pressed
   if((data4 == 0x07) && (cursorY != 0)){ //0x07 : Up arrow release cursorY: int psoition of //cursor
             write_cursor(cursorY, clear_cursor);
             write_cursor(--cursorY, cursor);
   }
   
   // Down Arrow Pressed
   else if((data4 == 0x08) && (cursorY != 3)) { // 0x08: Down arrow release
             write_cursor(cursorY, clear_cursor);
             write_cursor(++cursorY, cursor);
   }

}

cursorY is initially set to 0. Cursor is in position 0.

Scenario.
Press Down: Moves cursor to position #1. Clears cursor from position #0 . Perfect
Press Down: Nothing happens
Press Up: Clears cursor. Does not re-write cursor
Press Down: Re-writes cursor to position #1
Press Down: Nothing happens
Press Up: Clears cursor. Does not re-write cursor.
Press Down: Re-writes cursor to position #1

The last 2 button presses can only be executed from then on and continuously.
Nothing happens if a button is not pressed, as should be. The previous only happens when the buttons are pressed.

Does anyone know of an effecient algorithm to implement this?

Any advice or suggestions would be greatly appreciated.

Thanks

[matsp]

Sorry but I can't see anything directly, immediately, wrong with your code as such.

Some "style" comments: This is not WRONG as such, but...
change code like this

             write_cursor(++cursorY, cursor);

to something like this:

             ++cursorY;
             write_cursor(cursorY, cursor);

(Obviously likewise for the cursor up code)

That way, the increment/decrement is much more obvious, and if you ever change write_cursor to be a macro or some such, it won't affect the update of the actual position. It also helps if you ever try to debug the code - you can check the value before you call the function, rather than having the function call and increment as one line which you can't (triivally) separate into different steps.

Following that a step further, you can do:

    oldCursorY = cursorY;
    ... check if cursor key pressed; update cursorY....
    if (oldCursorY != cursorY)
    {
      write_cursor(oldCursorY, clear_cursor);
      write_cursor(cursorY, cursor);
    }

This would make smaller code, and easier to follow/debug/understand.


--
Mats