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. > 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). > 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
