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

Mike Frysinger
2009-06-15T06:04:54Z

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