Re: hd44780-serial bug?

Hi,

On 06.02.2014 09:56, Graham Smith wrote:
> In the end I have tracked it down to what I believe is a bug in the
> hd44780-serial.c code when it works out if it has to transmit a data
> escape code before sending a byte of data.  I have implemented a small
> patch to correct this but I haven't been able to work out how I should
> go about submitting the patch to you.

Sending the patch to the mailing list is fine. Please send the patch as
an attachment, not as inline text. You don't need to resend it though.

> I believe that there are effectively 2 bugs in the code but these only
> show up when using a connection type which requires data escape codes
> or supports multiple displays - so currently is only seen with the
> picanlcd and vdr_wakeup connection types.  (Note that this is based on
> the 0.5.5 release which is current in Ubuntu 13.10 but I've checked
> the 0.5.6 release code and it hasn't changed in this area).

I have checked with latest code and it only shows up when data escape is
used.

> Problem 1: I believe that the code is attempting to look up the data
> escape code for the connection type in use and then adding the
> displayID to it, which it does regardless of whether the connection
> type supports multiple displays.  This is itself a problem as the
> function serial_HD44780_senddata is sometimes called with a displayID
> of 0 (all displays) or 1 (1st display) even if the connection type
> only supports a single display and this means that for the picanlcd
> connection type the data escape character is sometimes sent as 0x12
> (if displayID is 0) and sometimes as 0x13 (if displayID is 1) -
> however the picanlcd code only expects a data escape of 0x12 as the
> 0x13 code is used for other things.  So I believe the code should be
> modified to only add the displayID if the connection type supports
> multiple displays.
>
> Problem 2: The code does not currently add the displayID to the data
> escape code for the connection type, but rather it adds the displayID
> to the address of the data escape character (as it is the address of
> the data to be written that is required in the call to write()).  This
> leads, for the picanlcd connection type, to the code 0x12 being sent
> for displayID == 0 and 0x00 for displayID == 1 (0x00 being the
> data_escape_min code in the hd44780_SerialInterface structure which is
> at the next address).
>
> My fix is simply to use a temporary variable to hold the data escape
> character and only add the displayID if the connection type supports
> multiple displays.  I then pass the address of the temporary variable
> to write.
>

First of all, I will add your patch, as the address calculation in
current code is wrong. However, I will not be able to test this now.

On the other hand, I believe we need to re-think how multiple displays
are supposed to work in the hd44780-serial. I looked at various parts of
hd44780.c and hd44780-serial.c but have not figured it completely yet.
 
Regards,
Markus