On Fri, Nov 23, 2012 at 09:28:26AM +0100, Olivier Fourdan wrote: > > Hey Peter, > > Peter Hutterer said the following on 11/23/2012 12:21 AM: > > > >[...] > >two things: > >I'd like to see a test that screams bloody murder when the flag is in fact > >unset. presumably if we add new devices we known whether they're built-in > >and the test should complain if we forget to add this. > > No, it's what is meant here. > > A database entry can miss the "IntegratedIn" filed, that's OK, many > do actually. > > By Not specifying "IntegratedIn" in the device database, you let > libwacom decide by using the kernel flags (see previous patch). > > If you specify IntegratedIn empty, then it means you actually force > the value and ignore the kernel flags. > > That's how it was before with "BuiltIn" actually, if "BuiltIn" is > set it takes precedence over the kernel flags.
OK, that makes sense. though given the limited set of integrated tablets in our comparatively small database it would still make sense to add the field to all descriptions in any case. > >the other thing: having UNSET as -1 for a bitmask is not a good choice, it > >requires any user to check for UNSET _and_ check for the mask set, since -1 > >will always set all bits. > > Oh I completely agree to this. > > My first submitted patch was using WACOM_DEVICE_INTEGRATED_UNSET = > (1 << 31) so that all lower (meaningful) bits where guaranteed to be > zero, while we could still tell if the flags were set in the > database definition or not (becuase we also use that to determine if > we use the kernel flags if not set in the database definition, just > like it was with "BuiltIn"). > > The reason I changed that to -1 as a flag is because I've been told > to in the previous review, see [1], [2] and [3]. > > So I'd rather not revert the change even though I agree. Unless we > all reach some sort of a consensus (contradictory reviews somehow > make it harder to adjust the patches). I'd like to get this API right before we make it public and have clients depend on it, with all the lag that entails (I know you already have something rely on it, but... :) Is UNSET actually necessary outside of libwacom itself? I can see why you need it for the auto-detection internally, but why does a client care if something is UNSET or NONE? assume that for the client UNSET == NONE in any case (how else could you treat it?). if that not correct it's a bug that needs to be fixed with a database entry. So how about making UNSET internal-only (in which case you can leave it as -1) but filter it to NONE before returning it to the client? Cheers, Peter > >so either we bump all up by one so UNSET is 0 and NONE is 1, or we leave > >NONE at 0 and guarantee that we always define this and drop UNSET. > > I don't think we want to drop unset (at least if we want to keep > consistent with current behavior). > > As to why 0 is not a good idea, well, that's because we want to tell > if the flag is set in the database, see [2] > > Cheers, > Olivier. > > [1] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5302 > [2] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5305 > [3] http://permalink.gmane.org/gmane.linux.drivers.wacom.devel/5315 > ------------------------------------------------------------------------------ Monitor your physical, virtual and cloud infrastructure from a single web console. Get in-depth insight into apps, servers, databases, vmware, SAP, cloud infrastructure, etc. Download 30-day Free Trial. Pricing starts from $795 for 25 servers or applications! http://p.sf.net/sfu/zoho_dev2dev_nov _______________________________________________ Linuxwacom-devel mailing list Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
