Re: MtxOrb driver... include file comment

On 08.08.2011 11:11, Liselore Vermeulen wrote:
>> L.S.
>>
>> Hi,
>> first of all, congratulations and thanks for the effort in lcd proc.
>> I'm using this system in a blackbox solution for my customer, since about 2000.  The version we are using there, was pre 0.5....
>>
>> Currently, I'm revisiting your program...
>>
>>
>> Some remarks 
>>
>> in cvs-r05dev, I discover removal of
>> #include "lcd.h" 
>>
>> from the MtxOrb.h file, and probably from other driver header files as wel...
>> although I didn't check it there.
>>
>> 1. the removal of #include "lcd.h" from e.g. MtxOrb.h, 
>> adds a requirement to the includer of MtxOrb.h 
>> to also include "lcd.h", otherwise Driver would not be defined.
>>
>>
>> I personally appreciate the rule where include files are to be 
>> "self sufficient"
>> i.e. 
>> - inclusion of one file should not require inclusion of another
>> - order of inclusion files is free.
>> This is one of the reasons for the almost "common" habit of 
>> "protecting an include file against multiple inclusion" through 
>> "ifdef/define/endif constructions".
>>
>>
>> 2. I would argument for moving the defines and typedefs from 
>> MtxOrb.h into MtxOrb.c,
>> because:
>> - they are (currently) not needed, nor used, outside of the MtxOrb.c.
>> - IF another module *would* include MtxOrb.h, (erroneously or while 
>>  needing some functions from it),
>> One can  easily come up with a construction in which these defines 
>> (like "DEFAULT_SPEED",... ) could potentially break up (quietly or not) 
>> that other code.
>>
>> Attached you'll find a patch for what I'm suggesting.
>>

Hi,

I understand your thoughts.

Actually these driver headers file are now superfluous. AFAIK they are
remiscents from ealier versions of LCDproc (0.3 or so) where all drivers
were compiled into one program. With the rise of the module loader they
were no longer needed.

However, we still continue to use them. They provide the necessary
prototypes and show quickly which part of the API a driver implements.

Unless there is very good reason to to so, I will not allow code that
include other driver's header files.

Looking at the patch, I won't commit it now as it contains no functional
change and I don't like that comment style. If any comment fot the
typedef struct is used it should be Doxygen anyway.

Regards,
Markus