Re: MtxOrb older models not supported (anymore ! ?) - ERROR in previous PATCH

Markus Dolze <[email protected]> Tue, 27 Nov 2012 07:14:51 +0100
Newsgroups gmane.comp.sysutils.lcdproc
Message-ID <[email protected]>
On 26.10.2012 17:37, Liselore Vermeulen wrote:
> 
> *ERROR*
> Sorry,
> Previous Patch file sent, contained a proof of dyslexia:
> 
> s/DebugWaitAfterWrite=on/DebugWaitAfterWrite=no/
> 
> LV

Hi,

I had some time to look into your patch. Here are my first impressions:

1. Yes, using flags to compensate for different display behavior is a
good idea. I did similar for the CFontzPacket driver some time ago.

2. Yes, querying the device for its type and firmware version and
detecting the correct model (with fallback settings) is a good idea.
Reading the serial number is nice, but just informational.

3. Yes, 500 us timeout for reading is not enough for transmitting a
single byte at 19200, but close and OK for larger speeds, but definitely
not for slower speed (at 9600 one will need ~1 ms).

4. Yes, using proper size write buffers is cleaner style.

5. No, I will not commit new changes to make LCDproc compile with GCC
2.96. We much rely on new C features, especially those run-time sized
arrays.

6. No, for the coding style. Especially:

6a. Please refrain from using "value comparator variable" style (1 <
var). While it is correct and I acknowledge some people liking it, this
is not used elsewhere (at least it should not) within LCDproc.

6b. For the same reason, please do not use "!0" in assignments or return
values. If you want to mimic Boolean behavior, use the system's
TRUE/FALSE macro or include "shared/defines.h".

6c) There is no punctuation at the end of any report/debug messages.

7. Now for the biggest part: The timed read/write functions.

I am still vary about that, but I feel that ~700 lines (including blank
and comment lines) are just a little exaggerated as a wrapper around
select(), read(), and write() calls.

Especially the two read_with_timeout functions (each 100 lines) which
are identical except for two lines!

I am still not sure on the improvements of the times write operation and
want to have a more closer look at that.

Thank you so far!
Regards,
Markus