MtxOrb driver... include file comment
Liselore Vermeulen <[email protected]>
| Newsgroups | gmane.comp.sysutils.lcdproc |
|---|---|
| Message-ID | <[email protected]> |
> 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. > _______________________________________________ LCDproc mailing list [email protected] http://lists.omnipotent.net/mailman/listinfo/lcdproc
lcdproc.MtxOrb_includefile.patch
(application/octet-stream, 2.2 KB)
Index: MtxOrb.c
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/drivers/MtxOrb.c,v
retrieving revision 1.120
diff -u -r1.120 MtxOrb.c
--- MtxOrb.c 28 Mar 2011 18:17:58 -0000 1.120
+++ MtxOrb.c 8 Aug 2011 05:54:09 -0000
@@ -62,6 +62,22 @@
#include "report.h"
+/*----------------------------------------------------------------------
+ * Local Macro's (defines)
+ *----------------------------------------------------------------------*/
+#define DEFAULT_CONTRAST 480
+#define DEFAULT_ADJ_BACKLIGHT 1
+#define DEFAULT_BRIGHTNESS 1000
+#define DEFAULT_OFFBRIGHTNESS 0
+#define DEFAULT_DEVICE "/dev/lcd"
+#define DEFAULT_SPEED 19200
+#define DEFAULT_LINEWRAP 1
+#define DEFAULT_AUTOSCROLL 0
+#define DEFAULT_CURSORBLINK 0
+#define DEFAULT_SIZE "20x4"
+#define DEFAULT_BACKLIGHT 1
+#define DEFAULT_TYPE "lcd"
+
/* MO displays allow 25 keys that map by default to 'A' - 'Y' */
#define MAX_KEY_MAP 25
@@ -92,6 +108,14 @@
* custom characters.
*/
+/*----------------------------------------------------------------------
+ * Local Type definitions
+ *----------------------------------------------------------------------*/
+typedef struct MtxOrbModule {
+ unsigned int model;
+ char *name;
+ unsigned int flags;
+} MtxOrbModuleEntry;
typedef enum {
MTXORB_LCD,
Index: MtxOrb.h
===================================================================
RCS file: /cvsroot/lcdproc/lcdproc/server/drivers/MtxOrb.h,v
retrieving revision 1.29
diff -u -r1.29 MtxOrb.h
--- MtxOrb.h 28 Mar 2011 18:17:58 -0000 1.29
+++ MtxOrb.h 8 Aug 2011 05:54:09 -0000
@@ -1,24 +1,7 @@
#ifndef MTXORB_H
#define MTXORB_H
-#define DEFAULT_CONTRAST 480
-#define DEFAULT_ADJ_BACKLIGHT 1
-#define DEFAULT_BRIGHTNESS 1000
-#define DEFAULT_OFFBRIGHTNESS 0
-#define DEFAULT_DEVICE "/dev/lcd"
-#define DEFAULT_SPEED 19200
-#define DEFAULT_LINEWRAP 1
-#define DEFAULT_AUTOSCROLL 0
-#define DEFAULT_CURSORBLINK 0
-#define DEFAULT_SIZE "20x4"
-#define DEFAULT_BACKLIGHT 1
-#define DEFAULT_TYPE "lcd"
-
-typedef struct MtxOrbModule {
- int model;
- char *name;
- int flags;
-} MtxOrbModuleEntry;
+#include "lcd.h"
MODULE_EXPORT int MtxOrb_init (Driver *drvthis);
MODULE_EXPORT void MtxOrb_close (Driver *drvthis);