Merge lp:~adeuring/launchpad/hwdb-class-udev-device-4 into lp:launchpad/db-devel

[Abel Deuring]

[Abel Deuring (adeuring)]

This branch adds the properties pci_ids, raw_bus, vendor, vendor_id, product, product_id to class UdevDevice.

The HWDB client in Karmic will no longer send data gathered from HAL, but gathered from udev, supplemented by sysfs and DMI data.

The HWDB submission processing script represents devices found in the submission data in a class derived from BaseDevice; this branch is another step to complete the implementation of class UdevDevice.

I forgot in earlier branches to specify the DMI data as a parameter of the UdevDevice constructor, so this branch contains also a number of purely formal changes in the tests, where class UdevDevice is instantiated.

I also noticed that I forgot a "raise NotImplemented" for a property of class BaseDevice.

test:
./bin/test --test=test_hwdb_submission_processing

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/scripts/hwdbsubmissions.py
  lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py

== Pyflakes notices ==

lib/canonical/launchpad/scripts/hwdbsubmissions.py
    22: redefinition of unused 'etree' from line 20

== Pylint notices ==

lib/canonical/launchpad/scripts/hwdbsubmissions.py
    20: [F0401] Unable to import 'xml.etree.cElementTree' (No module named etree)

The complaints about etree are not related to my changes.

[Brad Crittenden (bac)]

Hi Abel,

Thanks for the changes. We discussed some of my suggestions on IRC already. Please post an incremental diff when you've made these changes.

--Brad

> === modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
> --- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-12 15:36:04 +0000
> +++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-13 17:50:28 +0000
@@ -59,6 +59,7 @@
> re.VERBOSE)
>
> ROOT_UDI = '/org/freedesktop/Hal/devices/computer'
> +UDEV_ROOT_PATH = '/devices/LNXSYSTM:00'
>
> # These UDIs appears in some submissions more than once.
> KNOWN_DUPLICATE_UDIS = set((
> @@ -1606,6 +1607,7 @@
> @property
> def driver_name(self):
> """The name of the driver contolling this device. May be None."""
> + raise NotImplementedError

Nice clean up.

> def translateScsiBus(self):
> """Return the real bus of a device where raw_bus=='scsi'.
> @@ -2358,7 +2360,7 @@
> class UdevDevice(BaseDevice):
> """The representation of a udev device node."""
>
> - def __init__(self, udev_data, sysfs_data, parser):
> + def __init__(self, udev_data, sysfs_data, dmi_data, parser):
> """HALDevice constructor.

Seeing all of your test changes that required adding another parameter
make me wonder if you don't want to have default values on some of
these, especially the new one. Also I'm surprised to see you didn't
add the new on at the end, which helps with backward compatability.

> :param udevdata: The udev data for this device
> @@ -2369,6 +2371,7 @@
> super(UdevDevice, self).__init__(parser)
> self.udev = udev_data
> self.sysfs = sysfs_data
> + self.dmi = dmi_data
>
> @property
> def device_id(self):
> @@ -2410,6 +2413,24 @@
> return self.pci_class_info[1]

As we discussed on IRC, a lot of your code would be cleaner if methods
such as this one returned a dictionary with 'vendor' and 'product' as
the keys. Take a stab at doing that throughout and see what happens.

> @property
> + def pci_ids(self):
> + """The PCI vendor and product IDs.
> +
> + :return: (vendor_id, product_id) for PCI devices
> + or (None, None) for other devices.
> + """
> + if self.is_pci:
> + # SubmissionParser.checkUdevPciProperties() ensures that
> + # each PCI device has the property PCI_ID and that is
> + # consists of two 4-digit hexadecimal numbers, separated
> + # by a ':'.
> + id_string = self.udev['E']['PCI_ID']
> + ids = id_string.split(':')
> + return [int(part, 16) for part in ids]
> + else:
> + return [None, None]
> +
> + @property
> def is_usb(self):
> """True, if this is a USB device, else False."""
> return self.udev['E'].get('SUBSYSTEM') == 'usb'
> @@ -2476,6 +2497,116 @@
> else:
> return None
>
> + @property
> + def raw_bus(self):
> + """See `BaseDevice`."""
> + # udev specifies the property SUBSYSTEM for most devices;
> + # some devices have additionally the more specific property
...

review: Needs Fixing (code)

[Abel Deuring (adeuring)]

Hi Brad,

thanks for your review.

On 13.10.2009 21:28, Brad Crittenden wrote:
> Review: Needs Fixing code
> Hi Abel,
>
> Thanks for the changes. We discussed some of my suggestions on IRC already. Please post an incremental diff when you've made these changes.
>
> --Brad
>
>
>> === modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
>> --- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-12 15:36:04 +0000
>> +++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-13 17:50:28 +0000
> @@ -59,6 +59,7 @@
>> re.VERBOSE)
>>
>> ROOT_UDI = '/org/freedesktop/Hal/devices/computer'
>> +UDEV_ROOT_PATH = '/devices/LNXSYSTM:00'
>>
>> # These UDIs appears in some submissions more than once.
>> KNOWN_DUPLICATE_UDIS = set((
>> @@ -1606,6 +1607,7 @@
>> @property
>> def driver_name(self):
>> """The name of the driver contolling this device. May be None."""
>> + raise NotImplementedError
>
> Nice clean up.
>
>> def translateScsiBus(self):
>> """Return the real bus of a device where raw_bus=='scsi'.
>> @@ -2358,7 +2360,7 @@
>> class UdevDevice(BaseDevice):
>> """The representation of a udev device node."""
>>
>> - def __init__(self, udev_data, sysfs_data, parser):
>> + def __init__(self, udev_data, sysfs_data, dmi_data, parser):
>> """HALDevice constructor.
>
> Seeing all of your test changes that required adding another parameter
> make me wonder if you don't want to have default values on some of
> these, especially the new one. Also I'm surprised to see you didn't
> add the new on at the end, which helps with backward compatability.

Well, class UdevDevice is yet not used anywhere, so backward
compatibility is not an issue. But you are right, dmi_data and
sysfs_data in many cases not needed, while parser _is_ needed (in class
BaseDevice), so I shuffled the parameters again...

>
>> :param udevdata: The udev data for this device
>> @@ -2369,6 +2371,7 @@
>> super(UdevDevice, self).__init__(parser)
>> self.udev = udev_data
>> self.sysfs = sysfs_data
>> + self.dmi = dmi_data
>>
>> @property
>> def device_id(self):
>> @@ -2410,6 +2413,24 @@
>> return self.pci_class_info[1]
>
> As we discussed on IRC, a lot of your code would be cleaner if methods
> such as this one returned a dictionary with 'vendor' and 'product' as
> the keys. Take a stab at doing that throughout and see what happens.

Changed. Though this made the diff a bit larger, since it affected (via
the method getScsiVendorAnModel()) also class HALDevice...

>
>> @property
>> + def pci_ids(self):
>> + """The PCI vendor and product IDs.
>> +
>> + :return: (vendor_id, product_id) for PCI devices
>> + or (None, None) for other devices.
>> + """
>> + if self.is_pci:
>> + # SubmissionParser.checkUdevPciProperties() ensures that
>> + # each PCI device has the property PCI_ID and that is
>> + # consists of two 4-digit hexadecimal numbers, separated
>> + # by a ':'.
>> + id_string = self.udev['E']['PCI_ID']
>> + ids = id...

[Brad Crittenden (bac)]

Thanks for the changes Abel!

review: Approve (code)

=== modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
--- lib/canonical/launchpad/scripts/hwdbsubmissions.py	2009-10-12 15:36:04 +0000
+++ lib/canonical/launchpad/scripts/hwdbsubmissions.py	2009-10-13 21:06:13 +0000
@@ -59,6 +59,7 @@
     re.VERBOSE)
 
 ROOT_UDI = '/org/freedesktop/Hal/devices/computer'
+UDEV_ROOT_PATH = '/devices/LNXSYSTM:00'
 
 # These UDIs appears in some submissions more than once.
 KNOWN_DUPLICATE_UDIS = set((
@@ -1606,6 +1607,7 @@
     @property
     def driver_name(self):
         """The name of the driver contolling this device. May be None."""
+        raise NotImplementedError
 
     def translateScsiBus(self):
         """Return the real bus of a device where raw_bus=='scsi'.
@@ -2067,10 +2069,15 @@
             # it is hard to find a better heuristic to separate
             # the vendor name from the product name.
             splitted_name = self.scsi_model.split(' ', 1)
-            if len(splitted_name) < 2:
-                return 'ATA', splitted_name[0]
-            return splitted_name
-        return (vendor, self.scsi_model)
+            if len(splitted_name) == 2:
+                return {
+                    'vendor': splitted_name[0],
+                    'product': splitted_name[1],
+                    }
+        return {
+            'vendor': vendor,
+            'product': self.scsi_model,
+            }
 
     def getDriver(self):
         """Return the HWDriver instance associated with this device.
@@ -2289,11 +2296,7 @@
             # below does not work properly.
             return self.getProperty('system.hardware.' + type_)
         elif bus == 'scsi':
-            vendor, product = self.getScsiVendorAndModelName()
-            if type_ == 'vendor':
-                return vendor
-            else:
-                return product
+            return self.getScsiVendorAndModelName()[type_]
         else:
             result = self.getProperty('info.' + type_)
             if result is None:
@@ -2358,7 +2361,7 @@
 class UdevDevice(BaseDevice):
     """The representation of a udev device node."""
 
-    def __init__(self, udev_data, sysfs_data, parser):
+    def __init__(self, parser, udev_data, sysfs_data=None, dmi_data=None):
         """HALDevice constructor.
 
         :param udevdata: The udev data for this device
@@ -2369,6 +2372,7 @@
         super(UdevDevice, self).__init__(parser)
         self.udev = udev_data
         self.sysfs = sysfs_data
+        self.dmi = dmi_data
 
     @property
     def device_id(self):
@@ -2376,6 +2380,14 @@
         return self.udev['P']
 
     @property
+    def root_device_ids(self):
+        """The vendor and product IDs of the root device."""
+        return {
+            'vendor': self.dmi.get('/sys/class/dmi/id/sys_vendor'),
+            'product': self.dmi.get('/sys/class/dmi/id/product_name')
+            }
+
+    @property
     def is_pci(self):
         """True, if this is a PCI device, else False."""
         return self.udev['E'].get('SUBSYSTEM') == 'pci'
@@ -2410,6 +2422,30 @@
         return self.pci_class_info[1]
 
     @property
+    def pci_ids(self):
+        """The PCI vendor and product IDs.
+
+        :return: A dictionary containing the vendor and product IDs.
+            The IDs are set to None for Non-PCI devices.
+        """
+        if self.is_pci:
+            # SubmissionParser.checkUdevPciProperties() ensures that
+            # each PCI device has the property PCI_ID and that is
+            # consists of two 4-digit hexadecimal numbers, separated
+            # by a ':'.
+            id_string = self.udev['E']['PCI_ID']
+            ids = id_string.split(':')
+            return {
+                'vendor': int(ids[0], 16),
+                'product': int(ids[1], 16),
+                }
+        else:
+            return  {
+                'vendor': None,
+                'product': None,
+                }
+
+    @property
     def is_usb(self):
         """True, if this is a USB device, else False."""
         return self.udev['E'].get('SUBSYSTEM') == 'usb'
@@ -2418,8 +2454,9 @@
     def usb_ids(self):
         """The vendor ID, product ID, product version for USB devices.
 
-        :return: [vendor_id, product_id, version] for USB devices
-            or [None, None, None] for other devices.
+        :return: A dictionary containing the vendor and product IDs and
+            the product version for USB devices.
+            The IDs are set to None for Non-USB devices.
         """
         if self.is_usb:
             # udev represents USB device IDs as strings
@@ -2428,19 +2465,27 @@
             # SubmissionParser.checkUdevUsbProperties() ensures that
             # the string PRODUCT is in the format required below.
             product_info = self.udev['E']['PRODUCT'].split('/')
-            return [int(part, 16) for part in product_info]
+            return {
+                'vendor': int(product_info[0], 16),
+                'product': int(product_info[1], 16),
+                'version': int(product_info[2], 16),
+                }
         else:
-            return [None, None, None]
+            return {
+                'vendor': None,
+                'product': None,
+                'version': None,
+                }
 
     @property
     def usb_vendor_id(self):
         """See `BaseDevice`."""
-        return self.usb_ids[0]
+        return self.usb_ids['vendor']
 
     @property
     def usb_product_id(self):
         """See `BaseDevice`."""
-        return self.usb_ids[1]
+        return self.usb_ids['product']
 
     @property
     def is_scsi_device(self):
@@ -2476,6 +2521,102 @@
         else:
             return None
 
+    @property
+    def raw_bus(self):
+        """See `BaseDevice`."""
+        # udev specifies the property SUBSYSTEM for most devices;
+        # some devices have additionally the more specific property
+        # DEVTYPE. DEVTYPE is preferable.
+        # The root device has the subsystem/bus value "acpi", which
+        # is a bit nonsensical.
+        if self.device_id == UDEV_ROOT_PATH:
+            return None
+        properties = self.udev['E']
+        devtype = properties.get('DEVTYPE')
+        if devtype is not None:
+            return devtype
+        return properties.get('SUBSYSTEM')
+
+    def getVendorOrProduct(self, type_):
+        """Return the vendor or product of this device.
+
+        :return: The vendor or product data for this device.
+        :param type_: 'vendor' or 'product'
+        """
+        assert type_ in ('vendor', 'product'), (
+            'Unexpected value of type_: %r' % type_)
+
+        bus = self.raw_bus
+        if self.device_id == UDEV_ROOT_PATH:
+            # udev does not known about any product information for
+            # the root device. We use DMI data instead.
+            return self.root_device_ids[type_]
+        elif bus == 'scsi_device':
+            return self.getScsiVendorAndModelName()[type_]
+        elif bus in ('pci', 'usb_device'):
+            # XXX Abel Deuring 2009-10-13, bug 450480: udev does not
+            # provide human-readable vendor and product names for
+            # USB and PCI devices. We should retrieve these from
+            # http://www.linux-usb.org/usb.ids and
+            # http://pciids.sourceforge.net/v2.2/pci.ids
+            return 'Unknown'
+        else:
+            # We don't process yet other devices than complete systems,
+            # PCI, USB devices and those devices that are represented
+            # in udev as SCSI devices: real SCSI devices, and
+            # IDE/ATA/SATA devices.
+            return None
+
+    @property
+    def vendor(self):
+        """See `BaseDevice`."""
+        return self.getVendorOrProduct('vendor')
+
+    @property
+    def product(self):
+        """See `BaseDevice`."""
+        return self.getVendorOrProduct('product')
+
+    def getVendorOrProductID(self, type_):
+        """Return the vendor or product ID of this device.
+
+        :return: The vendor or product ID for this device.
+        :param type_: 'vendor' or 'product'
+        """
+        assert type_ in ('vendor', 'product'), (
+            'Unexpected value of type_: %r' % type_)
+
+        bus = self.raw_bus
+        if self.device_id == UDEV_ROOT_PATH:
+            # udev does not known about any product information for
+            # the root device. We use DMI data instead.
+            if type_ == 'vendor':
+                return self.dmi.get('/sys/class/dmi/id/sys_vendor')
+            else:
+                return self.dmi.get('/sys/class/dmi/id/product_name')
+        elif bus == 'scsi_device':
+            return self.getScsiVendorAndModelName()[type_]
+        elif bus == 'pci':
+            return self.pci_ids[type_]
+        elif bus == 'usb_device':
+            return self.usb_ids[type_]
+        else:
+            # We don't process yet other devices than complete systems,
+            # PCI, USB devices and those devices that are represented
+            # in udev as SCSI devices: real SCSI devices, and
+            # IDE/ATA/SATA devices.
+            return None
+
+    @property
+    def vendor_id(self):
+        """See `BaseDevice`."""
+        return self.getVendorOrProductID('vendor')
+
+    @property
+    def product_id(self):
+        """See `BaseDevice`."""
+        return self.getVendorOrProductID('product')
+
 
 class ProcessingLoop(object):
     """An `ITunableLoop` for processing HWDB submissions."""

=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py	2009-10-12 14:50:22 +0000
+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py	2009-10-13 21:06:13 +0000
@@ -1598,18 +1598,16 @@
         parser = SubmissionParser(self.log)
         parser.buildDeviceList(parsed_data)
         device = parser.hal_devices[self.UDI_SCSI_DISK]
-        vendor, model = device.getScsiVendorAndModelName()
+        vendor_model = device.getScsiVendorAndModelName()
         self.assertEqual(
-            vendor, 'SHARP',
+            {
+                'vendor': 'SHARP',
+                'product': 'JX250 SCSI',
+                },
+            vendor_model,
             'Unexpected result of HWDevice.getScsiVendorAndModelName '
             'for a regular SCSI device. Expected vendor name SHARP, got %r.'
-            % vendor)
-        self.assertEqual(
-            model, 'JX250 SCSI',
-            'Unexpected result of HWDevice.getScsiVendorAndModelName '
-            'for a regular SCSI device. Expected model name JX250 SCSI , '
-            'got %r.'
-            % model)
+            % vendor_model)
 
     def testHALDeviceSCSIVendorModelNameATADiskShortModelName(self):
         """Test of HALDevice.getScsiVendorAndModelName, ATA disk (1).
@@ -1639,18 +1637,16 @@
         parser = SubmissionParser(self.log)
         parser.buildDeviceList(parsed_data)
         device = parser.hal_devices[self.UDI_SCSI_DISK]
-        vendor, model = device.getScsiVendorAndModelName()
-        self.assertEqual(
-            vendor, 'Hitachi',
-            'Unexpected result of HWDevice.getScsiVendorAndModelName '
-            'for an ATA SCSI device. Expected vendor name Hitachi, got %r.'
-            % vendor)
-        self.assertEqual(
-            model, 'HTS54161',
-            'Unexpected result of HWDevice.getScsiVendorAndModelName '
-            'for a reguale SCSI device. Expected vendor name HTS54161, '
-            'got %r.'
-            % model)
+        vendor_model = device.getScsiVendorAndModelName()
+        self.assertEqual(
+            {
+                'vendor': 'Hitachi',
+                'product': 'HTS54161',
+                },
+            vendor_model,
+            'Unexpected result of HWDevice.getScsiVendorAndModelName '
+            'for an ATA SCSI device: %r.'
+            % vendor_model)
 
     def testHALDeviceSCSIVendorModelNameATADiskLongModelName(self):
         """Test of HALDevice.getScsiVendorAndModelName, ATA disk (2).
@@ -1679,18 +1675,16 @@
         parser = SubmissionParser(self.log)
         parser.buildDeviceList(parsed_data)
         device = parser.hal_devices[self.UDI_SCSI_DISK]
-        vendor, model = device.getScsiVendorAndModelName()
-        self.assertEqual(
-            vendor, 'ATA',
-            'Unexpected result of HWDevice.getScsiVendorAndModelName '
-            'for a reguale SCSI device. Expected vendor name ATA, got %r.'
-            % vendor)
-        self.assertEqual(
-            model, 'HTC426060G9AT00',
-            'Unexpected result of HWDevice.getScsiVendorAndModelName '
-            'for a reguale SCSI device. Expected vendor name '
-            'HTC426060G9AT00 , got %r.'
-            % model)
+        vendor_product = device.getScsiVendorAndModelName()
+        self.assertEqual(
+            {
+                'vendor': 'ATA',
+                'product': 'HTC426060G9AT00',
+                },
+            vendor_product,
+            'Unexpected result of HWDevice.getScsiVendorAndModelName '
+            'for a reguale SCSI device: %r.'
+            % vendor_product)
 
     def testHALDeviceVendorFromInfoVendor(self):
         """Test of HALDevice.vendor, regular case.
@@ -2606,6 +2600,11 @@
             }
         }
 
+    root_device_dmi_data = {
+        '/sys/class/dmi/id/sys_vendor': 'FUJITSU SIEMENS',
+        '/sys/class/dmi/id/product_name': 'LIFEBOOK E8210',
+        }
+
     pci_device_data = {
         'P': '/devices/pci0000:00/0000:00:1f.2',
         'E': {
@@ -2641,97 +2640,149 @@
         'type': '5',
         }
 
+    no_subsystem_device_data = {
+        'P': '/devices/pnp0/00:00',
+        'E': {}
+        }
+
     def test_device_id(self):
         """Test of UdevDevice.device_id."""
-        device = UdevDevice(self.pci_device_data, None, None)
+        device = UdevDevice(None, self.pci_device_data)
         self.assertEqual(
             '/devices/pci0000:00/0000:00:1f.2', device.device_id,
             'Unexpected value of UdevDevice.device_id.')
 
+    def test_root_device_ids(self):
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual(
+            {
+                'vendor': 'FUJITSU SIEMENS',
+                'product': 'LIFEBOOK E8210',
+                },
+            device.root_device_ids)
+
+        device = UdevDevice(
+            None, self.root_device, None, {})
+        self.assertEqual(
+            {
+                'vendor': None,
+                'product': None,
+                },
+            device.root_device_ids)
+
     def test_is_pci(self):
         """Test of UdevDevice.is_pci."""
-        device = UdevDevice(self.pci_device_data, None, None)
+        device = UdevDevice(None, self.pci_device_data)
         self.assertTrue(device.is_pci)
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertFalse(device.is_pci)
 
     def test_pci_class_info(self):
         """Test of UdevDevice.pci_class_info"""
-        device = UdevDevice(self.pci_device_data, None, None)
+        device = UdevDevice(None, self.pci_device_data)
         self.assertEqual(
             (1, 6, 2), device.pci_class_info,
             'Invalid value of UdevDevice.pci_class_info for PCI device.')
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(
             (None, None, None), device.pci_class_info,
             'Invalid value of UdevDevice.pci_class_info for Non-PCI device.')
 
     def test_pci_class(self):
         """Test of UdevDevice.pci_class"""
-        device = UdevDevice(self.pci_device_data, None, None)
+        device = UdevDevice(None, self.pci_device_data)
         self.assertEqual(
             1, device.pci_class,
             'Invalid value of UdevDevice.pci_class for PCI device.')
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(
             None, device.pci_class,
             'Invalid value of UdevDevice.pci_class for Non-PCI device.')
 
     def test_pci_subclass(self):
         """Test of UdevDevice.pci_subclass"""
-        device = UdevDevice(self.pci_device_data, None, None)
+        device = UdevDevice(None, self.pci_device_data)
         self.assertEqual(
             6, device.pci_subclass,
             'Invalid value of UdevDevice.pci_class for PCI device.')
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(
             None, device.pci_class,
             'Invalid value of UdevDevice.pci_class for Non-PCI device.')
 
+    def test_pci_ids(self):
+        """Test of UdevDevice.pci_ids"""
+        device = UdevDevice(None, self.pci_device_data)
+        self.assertEqual(
+            {'vendor': 0x8086,
+             'product': 0x27C5,
+             },
+            device.pci_ids,
+            'Invalid value of UdevDevice.pci_ids for PCI device.')
+
+        device = UdevDevice(None, self.usb_device_data)
+        self.assertEqual(
+            {'vendor': None,
+             'product': None,
+             },
+            device.pci_ids,
+            'Invalid value of UdevDevice.pci_ids for Non-PCI device.')
+
     def test_is_usb(self):
         """Test of UdevDevice.is_usb"""
-        device = UdevDevice(self.usb_device_data, None, None)
+        device = UdevDevice(None, self.usb_device_data)
         self.assertTrue(device.is_usb)
 
-        device = UdevDevice(self.pci_device_data, None, None)
+        device = UdevDevice(None, self.pci_device_data)
         self.assertFalse(device.is_usb)
 
     def test_usb_ids(self):
         """Test of UdevDevice.usb_ids"""
-        device = UdevDevice(self.usb_device_data, None, None)
+        device = UdevDevice(None, self.usb_device_data)
         self.assertEqual(
-            [0x46d, 0xa01, 0x1013], device.usb_ids,
+            {
+                'vendor': 0x46d,
+                'product': 0xa01,
+                'version': 0x1013,
+                },
+            device.usb_ids,
             'Invalid value of UdevDevice.usb_ids for USB device.')
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(
-            [None, None, None], device.usb_ids,
+            {
+                'vendor': None,
+                'product': None,
+                'version': None,
+                },
+            device.usb_ids,
             'Invalid value of UdevDevice.usb_ids for Non-USB device.')
 
     def test_usb_vendor_id(self):
         """Test of UdevDevice.usb_vendor_id"""
-        device = UdevDevice(self.usb_device_data, None, None)
+        device = UdevDevice(None, self.usb_device_data)
         self.assertEqual(
             0x46d, device.usb_vendor_id,
             'Invalid value of UdevDevice.usb_vendor_id for USB device.')
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(
             None, device.usb_vendor_id,
             'Invalid value of UdevDevice.usb_vendor_id for Non-USB device.')
 
     def test_usb_product_id(self):
         """Test of UdevDevice.usb_product_id"""
-        device = UdevDevice(self.usb_device_data, None, None)
+        device = UdevDevice(None, self.usb_device_data)
         self.assertEqual(
             0xa01, device.usb_product_id,
             'Invalid value of UdevDevice.usb_product_id for USB device.')
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(
             None, device.usb_product_id,
             'Invalid value of UdevDevice.usb_product_id for Non-USB device.')
@@ -2739,29 +2790,127 @@
     def test_is_scsi_device(self):
         """Test of UdevDevice.is_scsi_device."""
         device = UdevDevice(
-            self.scsi_device_data, self.scsi_device_sysfs_data, None)
+            None, self.scsi_device_data, self.scsi_device_sysfs_data)
         self.assertTrue(device.is_scsi_device)
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertFalse(device.is_scsi_device)
 
     def test_scsi_vendor(self):
         """Test of UdevDevice.scsi_vendor."""
         device = UdevDevice(
-            self.scsi_device_data, self.scsi_device_sysfs_data, None)
+            None, self.scsi_device_data, self.scsi_device_sysfs_data, None)
         self.assertEqual('MATSHITA', device.scsi_vendor)
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(None, device.scsi_vendor)
 
     def test_scsi_model(self):
         """Test of UdevDevice.scsi_model."""
         device = UdevDevice(
-            self.scsi_device_data, self.scsi_device_sysfs_data, None)
+            None, self.scsi_device_data, self.scsi_device_sysfs_data)
         self.assertEqual('DVD-RAM UJ-841S', device.scsi_model)
 
-        device = UdevDevice(self.root_device, None, None)
+        device = UdevDevice(None, self.root_device)
         self.assertEqual(None, device.scsi_model)
 
+    def test_raw_bus(self):
+        """Test of UdevDevice.raw_bus."""
+        device = UdevDevice(None, self.root_device)
+        self.assertEqual(None, device.raw_bus)
+
+        device = UdevDevice(None, self.pci_device_data)
+        self.assertEqual('pci', device.raw_bus)
+
+        device = UdevDevice(None, self.usb_device_data)
+        self.assertEqual('usb_device', device.raw_bus)
+
+        device = UdevDevice(None, self.no_subsystem_device_data)
+        self.assertEqual(None, device.raw_bus)
+
+    def test_getVendorOrProduct(self):
+        """Test of UdevDevice.getVendorOrProduct()."""
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual(
+            'FUJITSU SIEMENS', device.getVendorOrProduct('vendor'))
+        self.assertEqual(
+            'LIFEBOOK E8210', device.getVendorOrProduct('product'))
+        self.assertRaises(
+            AssertionError, device.getVendorOrProduct, 'nonsense')
+
+        device = UdevDevice(None, self.pci_device_data)
+        self.assertEqual('Unknown', device.getVendorOrProduct('vendor'))
+        self.assertEqual('Unknown', device.getVendorOrProduct('product'))
+
+        device = UdevDevice(None, self.usb_device_data)
+        self.assertEqual('Unknown', device.getVendorOrProduct('vendor'))
+        self.assertEqual('Unknown', device.getVendorOrProduct('product'))
+
+        device = UdevDevice(
+            None, self.scsi_device_data, self.scsi_device_sysfs_data)
+        self.assertEqual('MATSHITA', device.getVendorOrProduct('vendor'))
+        self.assertEqual(
+            'DVD-RAM UJ-841S', device.getVendorOrProduct('product'))
+
+        device = UdevDevice(None, self.no_subsystem_device_data)
+        self.assertEqual(None, device.getVendorOrProduct('vendor'))
+        self.assertEqual(None, device.getVendorOrProduct('product'))
+
+    def test_vendor(self):
+        """Test of UdevDevice.vendor."""
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual('FUJITSU SIEMENS', device.vendor)
+
+    def test_product(self):
+        """Test of UdevDevice.product."""
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual('LIFEBOOK E8210', device.product)
+
+    def test_getVendorOrProductID(self):
+        """Test of UdevDevice.getVendorOrProduct()."""
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual(
+            'FUJITSU SIEMENS', device.getVendorOrProductID('vendor'))
+        self.assertEqual(
+            'LIFEBOOK E8210', device.getVendorOrProductID('product'))
+        self.assertRaises(
+            AssertionError, device.getVendorOrProductID, 'nonsense')
+
+        device = UdevDevice(None, self.pci_device_data)
+        self.assertEqual(0x8086, device.getVendorOrProductID('vendor'))
+        self.assertEqual(0x27C5, device.getVendorOrProductID('product'))
+
+        device = UdevDevice(None, self.usb_device_data)
+        self.assertEqual(0x46d, device.getVendorOrProductID('vendor'))
+        self.assertEqual(0xa01, device.getVendorOrProductID('product'))
+
+        device = UdevDevice(
+            None, self.scsi_device_data, self.scsi_device_sysfs_data)
+        self.assertEqual('MATSHITA', device.getVendorOrProductID('vendor'))
+        self.assertEqual(
+            'DVD-RAM UJ-841S', device.getVendorOrProductID('product'))
+
+        device = UdevDevice(
+            None, self.no_subsystem_device_data)
+        self.assertEqual(None, device.getVendorOrProductID('vendor'))
+        self.assertEqual(None, device.getVendorOrProductID('product'))
+
+    def test_vendor_id(self):
+        """Test of UdevDevice.vendor_id."""
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual('FUJITSU SIEMENS', device.vendor_id)
+
+    def test_product_id(self):
+        """Test of UdevDevice.product_id."""
+        device = UdevDevice(
+            None, self.root_device, None, self.root_device_dmi_data)
+        self.assertEqual('LIFEBOOK E8210', device.product_id)
+
+
 class TestHWDBSubmissionTablePopulation(TestCaseHWDB):
     """Tests of the HWDB popoluation with submitted data."""