[U-Boot] [RFC/PATCH 1/2] Add menu Framework

[Mike Frysinger <vapier@gentoo.org>]

On Saturday 13 June 2009 15:13:01 Jean-Christophe PLAGNIOL-VILLARD wrote:
> Introduce a menu framework that allow us to create list menu to simplify
> u-boot and make it more convivial for the end-user.
>
> This kind of menu is very usefull when you do not have a keyboard or a
> serial console attached to your board to allow you to interract with
> u-boot
>
> For the develloper part,
> The framework introduce two API
>
> 1) C
> that allow you to create menu, submenu, entry and complex menu action
>
> 2) Command
> that allow you as the C API to create menu, submenu, entry and complex
> menu action but this time the actions will be store in a env var and
> then be evaluated and excecuted.

so you could create a multiple choice menu without writing a single line of C 
code ?  that would certainly be preferred as writing C code seems error prone 
and silly for a static menu setup.

could you give an example of using the menu command ?  e.g. openmoko presents 
a menu with a few options:
 - boot
 - set console to usb
 - set console to serial
 - reset

so using only the menu command, how could you achieve the same thing ?

> +	INIT_LIST_HEAD(&(menus.list));

you use &(...) in a lot of places where the parenthesis are unnecessary

> +	if (m->name)
> +		free(m->name);
> +	if (m->display)
> +		free(m->display);

free(NULL) works fine, so the if() is unnecessary

> +int menu_add(struct menu *m)
> +{
> +	if (!m || !m->name)
> +		return -1;

would all of these sanity checks make more sense as debug-oly checks ?  or 
does the code rely on these in the normal running of things ?

> +		if(strcmp(m->name, name) == 0)

should do a search to make sure you're using "if ()" and not "if()" and 
similar

> +	do {
> +		ch = getc();
> +		switch(ch) {
> +		case 0x1b:
> +			escape = 1;
> +			break;
> +		case '[':
> +			if (escape)
> +				break;
> +		case 'A': /* up */
> +			escape = 0;
> ...
> +		case 'B': /* down */
> +			escape = 0;
> ...

i'm guessing you're parsing arrow keys here (comment should say "up key" 
rather than just "up").  but if you get just a '[' or 'A' or 'B', then this 
doesnt work right.  you probably want something like:
switch (ch) {
	case 0x1b:
		escape = 1;
		break;
	case '[':
		if (escape == 1)
			escape = 2;
		break;
	case 'A':
		if (escape != 2)
			break;
	...

then again, this kind of key parsing is duplicated in quite a few places in u-
boot.  we really should have this centralized so people can say getkey() and 
have it return cooked values.

> +int menu_action_exit(struct menu *m, struct menu_entry *me)
> +{
> +	return 0;
> +}

what's the point ?

> --- a/include/console.h
> +++ b/include/console.h
> +#define printf_reverse(fmt,args...)	printf("\e[7m" fmt "\e[m",##args)
> +#define puts_reverse(fmt)		puts("\e[7m" fmt "\e[m")
> +#define gotoXY(row, col)		printf("\e[%d;%dH", row, col)
> +#define clear()				puts("\e[2J")

i'm guessing this works with serial consoles and linux terminals.  how does 
this work with framebuffer consoles ?  (the answer may be obvious as i'm not 
familiar with the framebuffer console layers that may exist in u-boot)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090615/4a80b5fc/attachment.pgp