preprations for more flexible key handling in menus

docs/lcdproc-user/configuration.docbook

@@ -615,7 +615,7 @@ while the others work in shared mode.
     The key that switches into menu mode (=open the main menu).
     In menu mode it cancels any operation. Cancelling the main menu
     means returning to the regular display mode.
-    It defaults to <literal>Menu</literal>.
+    It has no default, but a natural candidate is <literal>Menu</literal>.
   </para></listitem>
 </varlistentry>
 
@@ -626,7 +626,7 @@ while the others work in shared mode.
   </term>
   <listitem><para>
     The key to enter a sub menu and/or, to select an entry.
-    It defaults to <literal>Enter</literal>.
+    It is not set by default, but a natural candidate is <literal>Enter</literal>.
   </para></listitem>
 </varlistentry>
 
@@ -639,7 +639,7 @@ while the others work in shared mode.
     The key to move to the previous menu item.
     If the <replaceable>LeftKey</replaceable> is not set,
     it is also used to move left in input fields.
-    It default to <literal>Up</literal>.
+    It is not set by default, but a natural candidate is <literal>Up</literal>.
   </para></listitem>
 </varlistentry>
 
@@ -652,7 +652,7 @@ while the others work in shared mode.
     The key to move to the next menu item.
     If the <replaceable>RightKey</replaceable> is not set,
     it is also used to move right in input fields.
-    It default to <literal>Down</literal>.
+    It has no default, but a natural candidate is <literal>Down</literal>.
   </para></listitem>
 </varlistentry>
 
@@ -682,6 +682,17 @@ while the others work in shared mode.
 </varlistentry>
 </variablelist>
 
+<para>
+The keys required for the menu work correctly are the <command>MenuKey</command>, 
+the <command>EnterKey</command> and one of <command>UpKey</command> or
+<command>DownKey</command>.
+With these 3 keys the menus can be operated.
+Of course with only 3 keys the navigation gets a bit awkward.
+So if you have 4 or more keys, you better use them.
+Especially the <command>LeftKey</command> and <command>RightKey</command>
+make a big difference in user experience.
+</para>
+
 </sect2>
 
 

server/input.c

@@ -152,7 +152,7 @@ input_internal_key(const char *key)
 	}
 	else {
 		/* Keys are for scrolling or rotating */
-		if (strcmp(key,toggle_rotate_key) == 0) {
+		if (strcmp(key, toggle_rotate_key) == 0) {
 			autorotate = !autorotate;
 			if (autorotate) {
 				server_msg("Rotate", 4);
@@ -160,17 +160,17 @@ input_internal_key(const char *key)
 				server_msg("Hold", 4);
 			}
 		}
-		else if (strcmp(key,prev_screen_key) == 0) {
+		else if (strcmp(key, prev_screen_key) == 0) {
 			screenlist_goto_prev();
 			server_msg("Prev", 4);
 		}
-		else if (strcmp(key,next_screen_key) == 0) {
+		else if (strcmp(key, next_screen_key) == 0) {
 			screenlist_goto_next();
 			server_msg("Next", 4);
 		}
-		else if (strcmp(key,scroll_up_key) == 0) {
+		else if (strcmp(key, scroll_up_key) == 0) {
 		}
-		else if (strcmp(key,scroll_down_key) == 0) {
+		else if (strcmp(key, scroll_down_key) == 0) {
 		}
 	}
 }
@@ -179,7 +179,8 @@ int input_reserve_key(const char *key, bool exclusive, Client *client)
 {
 	KeyReservation *kr;
 
-	debug(RPT_DEBUG, "%s(key=\"%.40s\", exclusive=%d, client=[%d])", __FUNCTION__, key, exclusive, (client?client->sock:-1));
+	debug(RPT_DEBUG, "%s(key=\"%.40s\", exclusive=%d, client=[%d])",
+		__FUNCTION__, key, exclusive, (client?client->sock:-1));
 
 	/* Find out if this key is already reserved in a way that interferes
 	 * with the new reservation.
@@ -200,7 +201,8 @@ int input_reserve_key(const char *key, bool exclusive, Client *client)
 	kr->client = client;
 	LL_Push(keylist, kr);
 
-	report(RPT_INFO, "Key \"%.40s\" is now reserved in %s mode by client [%d]", key, (exclusive?"exclusive":"shared"), (client?client->sock:-1));
+	report(RPT_INFO, "Key \"%.40s\" is now reserved %s by client [%d]",
+		key, (exclusive ? "exclusively" : "shared"), (client ? client->sock : -1));
 
 	return 0;
 }
@@ -209,15 +211,16 @@ void input_release_key(const char *key, Client *client)
 {
 	KeyReservation *kr;
 
-	debug(RPT_DEBUG, "%s(key=\"%.40s\", client=[%d])", __FUNCTION__, key, (client?client->sock:-1));
+	debug(RPT_DEBUG, "%s(key=\"%.40s\", client=[%d])", __FUNCTION__, key, (client ? client->sock : -1));
 
-	for (kr = LL_GetFirst(keylist); kr; kr = LL_GetNext(keylist)) {
+	for (kr = LL_GetFirst(keylist); kr != NULL; kr = LL_GetNext(keylist)) {
 		if (kr->client == client
 		&& strcmp(kr->key, key) == 0) {
 			free(kr->key);
 			free(kr);
 			LL_DeleteNode(keylist);
-			report(RPT_INFO, "Key \"%.40s\" was reserved in %s mode by client [%d] and is now released", key, (kr->exclusive?"exclusive":"shared"), (client?client->sock:-1));
+			report(RPT_INFO, "Key \"%.40s\" reserved %s by client [%d] and is now released",
+				key, (kr->exclusive ? "exclusively" : "shared"), (client ? client->sock : -1));
 			return;
 		}
 	}
@@ -227,12 +230,13 @@ void input_release_client_keys(Client *client)
 {
 	KeyReservation *kr;
 
-	debug(RPT_DEBUG, "%s(client=[%d])", __FUNCTION__, (client?client->sock:-1));
+	debug(RPT_DEBUG, "%s(client=[%d])", __FUNCTION__, (client ? client->sock : -1));
 
-	kr=LL_GetFirst(keylist);
+	kr = LL_GetFirst(keylist);
-	while (kr) {
+	while (kr != NULL) {
 		if (kr->client == client) {
-			report(RPT_INFO, "Key \"%.40s\" was reserved in %s mode by client [%d] and is now released", kr->key, (kr->exclusive?"exclusive":"shared"), (client?client->sock:-1));
+			report(RPT_INFO, "Key \"%.40s\" reserved %s by client [%d] and is now released",
+				kr->key, (kr->exclusive ? "exclusive" : "shared"), (client ? client->sock : -1));
 			free(kr->key);
 			free(kr);
 			LL_DeleteNode(keylist);
@@ -249,9 +253,9 @@ KeyReservation *input_find_key(const char *key, Client *client)
 
 	debug(RPT_DEBUG, "%s(key=\"%.40s\", client=[%d])", __FUNCTION__, key, (client?client->sock:-1));
 
-	for (kr = LL_GetFirst(keylist); kr; kr = LL_GetNext(keylist)) {
+	for (kr = LL_GetFirst(keylist); kr != NULL; kr = LL_GetNext(keylist)) {
 		if (strcmp(kr->key, key) == 0) {
-			if (kr->exclusive || client==kr->client) {
+			if (kr->exclusive || client == kr->client) {
 				return kr;
 			}
 		}

server/menuitem.c

@@ -1068,8 +1068,8 @@ MenuResult menuitem_process_input_numeric(MenuItem *item, MenuToken token, const
 					return MENURESULT_ERROR;
 				}
 				/* Test the value */
-				if (value < item->data.numeric.minvalue
+				if ((value < item->data.numeric.minvalue) ||
-				|| value > item->data.numeric.maxvalue) {
+				    (value > item->data.numeric.maxvalue)) {
 					/* Out of range !
 					 * We can't exit this screen now
 					 */
@@ -1355,7 +1355,7 @@ MenuResult menuitem_process_input_ip(MenuItem *item, MenuToken token, const char
 			}
 			return MENURESULT_NONE;
 		case MENUTOKEN_ENTER:
-			if (extended || (pos >= item->data.ip.maxlength - 1)) {
+			if ((extended) || (pos >= item->data.ip.maxlength - 1)) {
 				// remove the leading spaces/zeros in each octet-representing string
 				char tmp[40];	// 40 = max. length of IPv4 & IPv6 addresses incl. '\0'
 				char *start = tmp;

server/menuitem.h

@@ -59,15 +59,16 @@ typedef enum CheckboxValue {
 	CHECKBOX_OFF = 0, CHECKBOX_ON, CHECKBOX_GRAY
 } CheckboxValue;
 
-/** Recognized input token codes */
+/** Recognized input token codes: they need to be bit values */
 typedef enum MenuToken {
-	MENUTOKEN_MENU,
+	MENUTOKEN_NONE  = 0x0000,	/**< no key */
-	MENUTOKEN_ENTER,
+	MENUTOKEN_MENU  = 0x0001,	/**< MenuKey */
-	MENUTOKEN_UP,
+	MENUTOKEN_ENTER = 0x0002,	/**< EnterKey */
-	MENUTOKEN_DOWN,
+	MENUTOKEN_UP    = 0x0004,	/**< UpKey */
-	MENUTOKEN_LEFT,
+	MENUTOKEN_DOWN  = 0x0008,	/**< DownKey */
-	MENUTOKEN_RIGHT,
+	MENUTOKEN_LEFT  = 0x0010,	/**< LeftKey */
-	MENUTOKEN_OTHER
+	MENUTOKEN_RIGHT = 0x0020,	/**< RightKey */
+	MENUTOKEN_OTHER = 0x1000	/**< any other key */
 } MenuToken;
 
 /** Return codes from an input handler */
@@ -88,13 +89,13 @@ typedef enum MenuResult {
 /** Events caused by a menuitem */
 typedef enum MenuEventType {
 	MENUEVENT_SELECT = 0,	/**< Item has been selected
-				(action chosen) */
+				 * (action chosen) */
 	MENUEVENT_UPDATE = 1,	/**< Item has been modified
-				(checkbox, numeric, alphanumeric) */
+				 * (checkbox, numeric, alphanumeric) */
 	MENUEVENT_PLUS   = 2,	/**< Item has been modified in positive direction
-				 (slider moved) */
+				 * (slider moved) */
 	MENUEVENT_MINUS  = 3,	/**< Item has been modified in negative direction
-				(slider moved) */
+				 * (slider moved) */
 	MENUEVENT_ENTER  = 4,	/**< Menu has been entered */
 	MENUEVENT_LEAVE  = 5,	/**< Menu has been left */
 	NUM_EVENTTYPES   = 6

server/menuscreens.c

@@ -48,6 +48,7 @@ char *up_key;
 char *down_key;
 char *left_key;
 char *right_key;
+static int keymask;	/* mask of defined menu keys */
 
 Screen *menuscreen = NULL;
 MenuItem *active_menuitem = NULL;
@@ -78,26 +79,54 @@ int menuscreens_init(void)
 
 	debug(RPT_DEBUG, "%s()", __FUNCTION__);
 
-	/* Get keys from config file */
+	/* Get keys from config file: MenuKey, EnterKey, UpKey, DownKey, LeftKey, RightKey.
-	menu_key = strdup(config_get_string("menu", "MenuKey", 0, "Menu"));
+	 * For a working menu at least 3 are necessary: MenuKey, EnterKey, UpKey/DownKey.
-	enter_key = strdup(config_get_string("menu", "EnterKey", 0, "Enter"));
+	 */
-	up_key = strdup(config_get_string("menu", "UpKey", 0, "Up"));
+	keymask = 0;
-	down_key = strdup(config_get_string("menu", "DownKey", 0, "Down"));
+	menu_key = enter_key = NULL;
+	tmp = config_get_string("menu", "MenuKey", 0, NULL);
+	if (tmp != NULL) {
+		menu_key = strdup(tmp);
+		keymask |= MENUTOKEN_MENU;
+	}
+	tmp = config_get_string("menu", "EnterKey", 0, NULL);
+	if (tmp != NULL) {
+		enter_key = strdup(tmp);
+		keymask |= MENUTOKEN_ENTER;
+	}
+
+	up_key = down_key = NULL;
+	tmp = config_get_string("menu", "UpKey", 0, NULL);
+	if (tmp != NULL) {
+		up_key = strdup(tmp);
+		keymask |= MENUTOKEN_UP;
+	}
+	tmp = config_get_string("menu", "DownKey", 0, NULL);
+	if (tmp != NULL) {
+		down_key = strdup(tmp);
+		keymask |= MENUTOKEN_DOWN;
+	}
 
-	/* if the user has specified in the conf file a left and right key */
 	left_key = right_key = NULL;
 	tmp = config_get_string("menu", "LeftKey", 0, NULL);
-	if (tmp != NULL)
+	if (tmp != NULL) {
 		left_key = strdup(tmp);
+		keymask |= MENUTOKEN_LEFT;
+	}
 	tmp = config_get_string("menu", "RightKey", 0, NULL);
-	if (tmp != NULL)
+	if (tmp != NULL) {
 		right_key = strdup(tmp);
+		keymask |= MENUTOKEN_RIGHT;
+	}
 
-
+	/* Now reserve the keys that were defined */
-	/* Now reserve keys */
+	if (menu_key != NULL)
 		input_reserve_key(menu_key, true, NULL);
+	if (enter_key != NULL)
 		input_reserve_key(enter_key, false, NULL);
+	if (up_key != NULL)
 		input_reserve_key(up_key, false, NULL);
+	if (down_key != NULL)
 		input_reserve_key(down_key, false, NULL);
 	if (left_key != NULL)
 		input_reserve_key(left_key, false, NULL);
@@ -144,14 +173,19 @@ int menuscreens_shutdown(void)
 	/* Forget menu's key reservations */
 	input_release_client_keys(NULL);
 
+	if (menu_key != NULL)	
 		free(menu_key);
+	if (enter_key != NULL)	
 		free(enter_key);
+	if (up_key != NULL)	
 		free(up_key);
+	if (down_key != NULL)	
 		free(down_key);
 	if (left_key != NULL)
         	free(left_key);
 	if (right_key != NULL)
         	free(right_key);
+	keymask = 0;	
 
 	return 0;
 }
@@ -188,7 +222,7 @@ void menuscreen_inform_item_modified(MenuItem *item)
 
 bool is_menu_key(const char *key)
 {
-	if (menu_key && key && strcmp(key, menu_key) == 0)
+	if ((menu_key != NULL) && (key != NULL) && (strcmp(key, menu_key) == 0))
 		return true;
 	else
 		return false;
@@ -284,10 +318,9 @@ static void handle_predecessor(void)
 	assert(item != NULL);
 	debug(RPT_DEBUG, "%s: Switching to registered predecessor '%s' of '%s'.",
 	       __FUNCTION__, item->predecessor_id, item->id);
-	predecessor = menuitem_search(
+	predecessor = menuitem_search(item->predecessor_id,
-		item->predecessor_id, (Client*)active_menuitem->client);
+				      (Client *) active_menuitem->client);
-	if (predecessor == NULL)
+	if (predecessor == NULL) {
-	{
 		// note: if _quit_, _close_, _none_ get here this
 		// would be an implementation error - they should
 		// have been handled via different MENURESULT codes.
@@ -306,9 +339,8 @@ static void handle_predecessor(void)
 		menuitem_update_screen(active_menuitem, menuscreen);
 		break;
 	default:
-		if (predecessor->parent != NULL
+		if ((predecessor->parent != NULL) &&
-		    && predecessor->parent->type == MENUITEM_MENU)
+		    (predecessor->parent->type == MENUITEM_MENU)) {
-		{
 			// update parent menu too
 			menu_select_subitem(predecessor->parent, predecessor->id);
 		}
@@ -326,10 +358,9 @@ static void handle_successor(void)
 	assert(item != NULL);
 	debug(RPT_DEBUG, "%s: Switching to registered successor '%s' of '%s'.",
 	       __FUNCTION__, item->successor_id, item->id);
-	successor = menuitem_search(
+	successor = menuitem_search(item->successor_id,
-		item->successor_id, (Client*)active_menuitem->client);
+				    (Client *) active_menuitem->client);
-	if (successor == NULL)
+	if (successor == NULL) {
-	{
 		// note: if _quit_, _close_, _none_ get here this
 		// would be an implementation error - they should
 		// have been handled via different MENURESULT codes.
@@ -348,9 +379,8 @@ static void handle_successor(void)
 		menuitem_update_screen(active_menuitem, menuscreen);
 		break;
 	default:
-		if (successor->parent != NULL
+		if ((successor->parent != NULL) &&
-		    && successor->parent->type == MENUITEM_MENU)
+		    (successor->parent->type == MENUITEM_MENU)) {
-		{
 			// update parent menu too
 			menu_select_subitem(successor->parent, successor->id);
 		}
@@ -361,27 +391,27 @@ static void handle_successor(void)
 
 void menuscreen_key_handler(const char *key)
 {
-	char token = 0;
+	MenuToken token = MENUTOKEN_NONE;
 	MenuResult res;
 
 	debug(RPT_DEBUG, "%s(\"%s\")", __FUNCTION__, key);
 
-	if (strcmp(key, menu_key) == 0) {
+	if ((menu_key != NULL) && (strcmp(key, menu_key) == 0)) {
 		token = MENUTOKEN_MENU;
 	}
-	else if (strcmp(key, enter_key) == 0) {
+	else if ((enter_key != NULL) && (strcmp(key, enter_key) == 0)) {
 		token = MENUTOKEN_ENTER;
 	}
-	else if (strcmp(key, up_key) == 0) {
+	else if ((up_key != NULL) && (strcmp(key, up_key) == 0)) {
 		token = MENUTOKEN_UP;
 	}
-	else if (strcmp(key, down_key) == 0) {
+	else if ((down_key != NULL) && (strcmp(key, down_key) == 0)) {
 		token = MENUTOKEN_DOWN;
 	}
-	else if (left_key && strcmp(key, left_key) == 0) {
+	else if ((left_key != NULL) && (strcmp(key, left_key) == 0)) {
 		token = MENUTOKEN_LEFT;
 	}
-	else if (right_key && strcmp(key, right_key) == 0) {
+	else if ((right_key != NULL) && (strcmp(key, right_key) == 0)) {
 		token = MENUTOKEN_RIGHT;
 	}
 	else {
@@ -396,7 +426,7 @@ void menuscreen_key_handler(const char *key)
 	}
 
 	res = menuitem_process_input(active_menuitem, token, key,
-			((left_key || right_key) ? 1 : 0));
+			(keymask & (MENUTOKEN_LEFT | MENUTOKEN_RIGHT)) ? 1 : 0);
 
 	switch (res) {
 	  case MENURESULT_ERROR: