[Lcdproc] [Lcdproc][patch] Cwlnx driver bug fix

[Gideon]

This is a multi-part message in MIME format.
--------------060506030309020403020700
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi Peter,

Thanks for the suggestion.  I worked on it a little bit more.  The loop 
will quit after 30 retries(Would be nice if there are some error 
handling if all fail).   There will be no delay if it writes 
successfully at the first time, but after that the delay helps to keep 
the number of retries smaller. I've tested on both 12232 USB and 1602 
serial, both look fine.

I set the "Model" parameter correctly in LCDd.conf, but vertical bar 
didn't draw correctly on 12232 display because there's no mask for the 
graphical mode.  Now the program would still limit 12232 display to draw 
a vertical bar with width 5, but that's the width for all icons.  Any 
other way to do it?

regards,

Gideon

Peter Marschall wrote:
> Hi Gideon,
> 
> I don't think the patch is correct.
> 
> On Monday, 15. January 2007 05:05, Gideon Tsang wrote:
>> --- lcdproc.orig/server/drivers/CwLnx.c 2006-10-07 01:23:21.000000000 +0800
>> +++ lcdproc/server/drivers/CwLnx.c      2007-01-12 13:06:48.000000000 +0800
>> @@ -153,9 +153,15 @@
>>  
>>  static int Write_LCD(int fd, char *c, int size)
>>  {
>> -    int rc;
>> +    int i, rc;
>>  
>> -    rc = write(fd, c, size);
>> +    for (i = 0;; i++) {
>> +       rc = write(fd, c, size);
>> +       if (rc == size) {
>> +           break;
>> +       }
>> +       usleep(2000);
>> +    }
>>  /* Debuging code to be cleaned when very stable */
>>  /*
>>      if (size == 1) {
> 
> In the chunk above the loop will run forever if errors occur permanently.
> This leads to a hang in LCDd.
> 
> What about simply uncommenting the 
> 	/* usleep(DELAY); */ 
> just before the end of the function.
> Will this help ?
> Plase note: the delay is much shorter than the one you suggested.
> Maybe you can tweak it a bit.
> (I'd like the delay small, so that LCDd can react as quickly as possible)
> 
> 
>> @@ -965,7 +971,8 @@
>>  
>>         for (i = 1; i < p->cellheight; i++) {
>>             // add pixel line per pixel line ...
>> -           vBar[p->cellheight - i] = 0xFF;
>> +
>> +            vBar[p->cellheight - i] = 0x1F;            // match the vbar
>>             CwLnx_set_char(drvthis, i+1, vBar);
>>         }
>>      }
> 
> This limits the width of the vertical bars to 5 pixels.
> But the CwLnx driver supprots two types of LCDs:
> the 1602 where a character is 5 pixels wide and the 12232
> with characters that are 6 pixels wide.
> CwLnx_set_char() contains a masking function that should adjust to the width 
> of each display.
> 
> Did you set the "Model" parameter in LCDd.cnf properly ?
> 
> Regards
> Peter
> 


--------------060506030309020403020700
Content-Type: text/x-patch;
 name="cwlnx.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="cwlnx.patch"

--- lcdproc.orig/server/drivers/CwLnx.c	2006-10-07 01:23:21.000000000 +0800
+++ lcdproc/server/drivers/CwLnx.c	2007-01-16 16:55:15.000000000 +0800
@@ -153,9 +153,19 @@
 
 static int Write_LCD(int fd, char *c, int size)
 {
-    int rc;
+    int rc, retries = 0;
+
+    do {
+	rc = write(fd, c, size);
+        if (rc != size) {
+	    usleep(2000);
+	    retries++;
+	    printf("retries = %d\n", retries);
+	} else {
+	    break;
+        }
+    } while (retries < 30);
 
-    rc = write(fd, c, size);
 /* Debuging code to be cleaned when very stable */
 /* 
     if (size == 1) {
@@ -1079,8 +1089,7 @@
 	    c = dat[row] & mask;
 	    Write_LCD(p->fd, &c, 1);
 	}
-    }
-    else {			// the graphical model
+    } else if (p->model == 12232) {	// the graphical model
 	int col;
 
 	for (col = p->cellwidth - 1; col >= 0; col--) {
@@ -1092,7 +1101,10 @@
 		letter |= ((dat[row] >> col) & 1);
 	    }
 
-	    c = letter;
+	    if (col >= 5)
+	    	c = 0;
+	    else
+	    	c = letter;
 	    Write_LCD(p->fd, &c, 1);
 	}
     }

--------------060506030309020403020700--