Hi Hannes,

On 02/02/2015 09:32 AM, Hannes Petermaier wrote:
"U-Boot" <u-boot-boun...@lists.denx.de> schrieb am 01.02.2015 15:48:51:

From: Nikita Kiryanov <nik...@compulab.co.il>
To: Hannes Petermaier <oe5...@oevsv.at>, u-boot@lists.denx.de
Date: 01.02.2015 15:49
Subject: Re: [U-Boot] [PATCH 02/21] common/lcd: Add command for setting
cursor
within lcd-framework
Sent by: "U-Boot" <u-boot-boun...@lists.denx.de>

Hi Hannes,
Hi Nikita,


On 01/30/2015 03:25 PM, Hannes Petermaier wrote:
We need this function if we want to make some outputs i.e position the
writing
cursor out of u-boot scripts.

This commit message is inaccurate. Positioning the writing cursor is not
in
itself output.
Also, what is the use case for such a command?

I want to set the "cursor" on the screen to a specific position and write
there something
with "puts".

For example:

setcurs 1 9;            sets the cursor to first column and 9th row
puts "Hello World!"     writes the text to the cursor.

Alright... So setcurs does not output anything, just prepares us to do output.
I think that's how the commit message should be phrased.



Signed-off-by: Hannes Petermaier <oe5...@oevsv.at>
---
   common/lcd.c |   21 +++++++++++++++++++++
   1 file changed, 21 insertions(+)

diff --git a/common/lcd.c b/common/lcd.c
index cc34b8a..f418da9 100644
--- a/common/lcd.c
+++ b/common/lcd.c
@@ -279,12 +279,33 @@ static int do_lcd_clear(cmd_tbl_t *cmdtp, int
flag, int argc,
      return 0;
   }

+static int do_lcd_setcursor(cmd_tbl_t *cmdtp, int flag, int argc,
+             char *const argv[])
+{
+   unsigned int col, row;
+
+   if (argc != 3)
+      return CMD_RET_USAGE;
+
+   col = simple_strtoul(argv[1], NULL, 10);
+   row = simple_strtoul(argv[2], NULL, 10);
+   lcd_position_cursor(col, row);
+
+   return 0;
+}
+
   U_BOOT_CMD(
      cls,   1,   1,   do_lcd_clear,
      "clear screen",
      ""
   );

+U_BOOT_CMD(
+   setcurs, 3,   1,   do_lcd_setcursor,
+   "sets cursor for 'puts'",

Another thing: this function is not really limited to working with puts. It
can do preparation for any other command that outputs to console. I recommend
rewriting the description string to not mention it, maybe something along the
lines of "set cursor on the screen".

+   "    <col> <row> in character"
+);
+

I think it would be better if the U_BOOT_CMD macros were adjacent to the
functions they
use. Also, I think this command is better suited for the lcd_console.c
file.

You're maybe right. Only thing for decission was, that existing commands
are allready defined within
lcd.c.

That's ok; we should place code where it's most relevant, and your command
is related to lcd console functionality.

But i've no problem to move it. What do you say? let's move?

Yes, please move it to lcd_console.c

--
Regards,
Nikita Kiryanov
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to