On Fri, Jan 28, 2011 at 10:49:04AM -0800, Ping Cheng wrote: > On Thu, Jan 27, 2011 at 8:57 PM, Peter Hutterer > <peter.hutte...@who-t.net> wrote: > > On Wed, Jan 26, 2011 at 10:20:39AM -0800, Ping Cheng wrote: > >> This change makes some of the lines in WacomModelDesc a bit too > >> long. Since we plan to move the table to the kernel driver soon, > >> I updated the table without updating the table format. It is only > >> to keep the code consistent between wcmUSB and wcmISDV4 for now. > >> > >> Once we move USB resolutions to the kernel, the defines can be > >> moved to wcmISDV4.c. > >> > >> Signed-off-by: Ping Cheng <pingli...@gmail.com> > >> --- > >> src/wcmISDV4.c | 6 +- > >> src/wcmUSB.c | 242 > >> +++++++++++++++++++++++++------------------------- > >> src/xf86WacomDefs.h | 12 +++ > >> 3 files changed, 136 insertions(+), 124 deletions(-) > >> > > > > [...] > > > >> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h > >> index 91adf72..7aa049d 100644 > >> --- a/src/xf86WacomDefs.h > >> +++ b/src/xf86WacomDefs.h > >> @@ -45,6 +45,18 @@ > >> #define MIN_PAD_RING 0 /* I4 absolute scroll ring min value > >> */ > >> #define MAX_PAD_RING 71 /* I4 absolute scroll ring max value > >> */ > >> > >> +/* Supported default resolutions in points/m */ > >> +#define RES_100000 100000 > >> +#define RES_200000 200000 > >> +#define RES_80000 80000 > >> +#define RES_50000 50000 > >> +#define RES_44173 44173 > >> +#define RES_39842 39842 > >> +#define RES_39370 39370 > >> +#define RES_36772 36772 > >> +#define RES_20000 20000 > >> +#define RES_10000 10000 > >> + > > > > I don't think this is a good idea. defines are used for mainly two reasons: > > - improve readability of the code > > x = MAX_X_VALUE is more expressive than x = 10000 > > - ease refacturing, providing a single place to change the value without > > changing the meaning. > > > > with the defines above, neither is realy met. RES_80000 is about as > > explanatory as 8000 but even worse: if we change this define we can never > > change the value without also changing every occurance of the macro. > > think of a how confusing the line below would be > > #define RES_80000 40000 > > > > so either we just leave the code as-is, or we find meaningful names for the > > defines. RES_INTUOS4 for example, or something like this if applicable. > > I thought about model or feature related names as well. But those > numbers do not have clean links to either the model or feature. Once > we made sure the actual numbers are right, I am sure someone will come > up with meaningful names for them.
if you with access do the docs can't seem to find a decent grouping, I doubt we will anytime soon. The model table currently exists for that reason, we have a huge number of models that are slightly different and thus need different treatment. the defines above don't really help with that. besides, having a look at the patch again I noticed that this doesn't just change all 100000 into RES_100000, it also changes the values of a few tablets around (0x9F for example). this is in itself a showstopper for the patch, these changes must be in a separate patch. either way, I'm happy to merge a patch that changes those that are currently wrong, but I won't merge the define RES_100000 part. > Anyway, I'd like to leave the code as is so we can move on. Since I > have access to the HW protocol and most HWs, my intention was to > provide as much HW infomation as I can to the code in an acceptable > way so other developers can start making the code more readable. > Hopefully, this would fully utilizes the limited resources we have in > the project and speed up the release cycle a bit. what's the chance of Wacom publishing the HW protocol? this would be the best utilization of the limited resources. Cheers, Peter ------------------------------------------------------------------------------ Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! Finally, a world-class log management solution at an even better price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer expires February 28th, so secure your free ArcSight Logger TODAY! http://p.sf.net/sfu/arcsight-sfd2d _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
