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