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_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 >> + # DEVTYPE. DEVTYPE is preferable. >> + # The root device has the subsystem/bus value "acpi", which >> + # is a bit nonsensical. >> + if self.device_id == '/devices/LNXSYSTM:00': > > Please use UDEV_ROOT_PATH for the comparison Done. > >> + return None >> + properties = self.udev['E'] >> + return properties.get('DEVTYPE') or properties.get('SUBSYSTEM') > > I find this construct unclear and suspect others may as well. Can you > rewrite it to be more explicit? Done. > >> + 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. >> + if type_ == 'vendor': >> + return self.dmi.get('/sys/class/dmi/id/sys_vendor') >> + else: >> + return self.dmi.get('/sys/class/dmi/id/product_name') > > A dictionary could be used here that returned one of those two strings > based on a vendor/product key. Done. > >> + elif bus == 'scsi_device': >> + vendor, product = self.getScsiVendorAndModelName() >> + if type_ == 'vendor': >> + return vendor >> + else: >> + return product >> + 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': >> + vendor, product = self.getScsiVendorAndModelName() >> + if type_ == 'vendor': >> + return vendor >> + else: >> + return product >> + elif bus == 'pci': >> + if type_ == 'vendor': >> + return self.pci_ids[0] >> + else: >> + return self.pci_ids[1] >> + elif bus == 'usb_device': >> + if type_ == 'vendor': >> + return self.usb_ids[0] >> + else: >> + return self.usb_ids[1] >> + 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 17:50:28 +0000 >> @@ -2606,6 +2606,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 +2646,114 @@ >> '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(self.pci_device_data, None, None, None) > > This change smells a bit, per my comment on your constructor. changed... Abel === modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py' --- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-13 17:36:22 +0000 +++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-13 20:08:04 +0000 @@ -2069,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. @@ -2291,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: @@ -2360,7 +2361,7 @@ class UdevDevice(BaseDevice): """The representation of a udev device node.""" - def __init__(self, udev_data, sysfs_data, dmi_data, parser): + def __init__(self, parser, udev_data, sysfs_data=None, dmi_data=None): """HALDevice constructor. :param udevdata: The udev data for this device @@ -2379,6 +2380,14 @@ return self.udev['P'] @property + def root_device_ids(self): + """The vendor and product IDs of the rott 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' @@ -2416,8 +2425,8 @@ def pci_ids(self): """The PCI vendor and product IDs. - :return: (vendor_id, product_id) for PCI devices - or (None, None) for other devices. + :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 @@ -2426,9 +2435,15 @@ # by a ':'. id_string = self.udev['E']['PCI_ID'] ids = id_string.split(':') - return [int(part, 16) for part in ids] + return { + 'vendor': int(ids[0], 16), + 'product': int(ids[1], 16), + } else: - return [None, None] + return { + 'vendor': None, + 'product': None, + } @property def is_usb(self): @@ -2439,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 @@ -2449,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): @@ -2505,10 +2529,13 @@ # DEVTYPE. DEVTYPE is preferable. # The root device has the subsystem/bus value "acpi", which # is a bit nonsensical. - if self.device_id == '/devices/LNXSYSTM:00': + if self.device_id == UDEV_ROOT_PATH: return None properties = self.udev['E'] - return properties.get('DEVTYPE') or properties.get('SUBSYSTEM') + 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. @@ -2523,16 +2550,9 @@ 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') + return self.root_device_ids[type_] elif bus == 'scsi_device': - vendor, product = self.getScsiVendorAndModelName() - if type_ == 'vendor': - return vendor - else: - return product + 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 @@ -2575,21 +2595,11 @@ else: return self.dmi.get('/sys/class/dmi/id/product_name') elif bus == 'scsi_device': - vendor, product = self.getScsiVendorAndModelName() - if type_ == 'vendor': - return vendor - else: - return product + return self.getScsiVendorAndModelName()[type_] elif bus == 'pci': - if type_ == 'vendor': - return self.pci_ids[0] - else: - return self.pci_ids[1] + return self.pci_ids[type_] elif bus == 'usb_device': - if type_ == 'vendor': - return self.usb_ids[0] - else: - return self.usb_ids[1] + 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 === modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py' --- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-13 17:36:22 +0000 +++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-13 20:06:04 +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. @@ -2653,107 +2647,142 @@ def test_device_id(self): """Test of UdevDevice.device_id.""" - device = UdevDevice(self.pci_device_data, None, 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, None) + device = UdevDevice(None, self.pci_device_data) self.assertTrue(device.is_pci) - device = UdevDevice(self.root_device, None, 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, 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, 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, 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, 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, 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, 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(self.pci_device_data, None, None, None) + device = UdevDevice(None, self.pci_device_data) self.assertEqual( - [0x8086, 0x27C5], device.pci_ids, + {'vendor': 0x8086, + 'product': 0x27C5, + }, + device.pci_ids, 'Invalid value of UdevDevice.pci_ids for PCI device.') - device = UdevDevice(self.usb_device_data, None, None, None) + device = UdevDevice(None, self.usb_device_data) self.assertEqual( - [None, None], device.pci_ids, + {'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, None) + device = UdevDevice(None, self.usb_device_data) self.assertTrue(device.is_usb) - device = UdevDevice(self.pci_device_data, None, 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, 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, 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, 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, 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, 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, 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.') @@ -2761,47 +2790,47 @@ 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) + None, self.scsi_device_data, self.scsi_device_sysfs_data) self.assertTrue(device.is_scsi_device) - device = UdevDevice(self.root_device, None, 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) + None, self.scsi_device_data, self.scsi_device_sysfs_data, None) self.assertEqual('MATSHITA', device.scsi_vendor) - device = UdevDevice(self.root_device, None, 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) + 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, 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(self.root_device, None, None, None) + device = UdevDevice(None, self.root_device) self.assertEqual(None, device.raw_bus) - device = UdevDevice(self.pci_device_data, None, None, None) + device = UdevDevice(None, self.pci_device_data) self.assertEqual('pci', device.raw_bus) - device = UdevDevice(self.usb_device_data, None, None, None) + device = UdevDevice(None, self.usb_device_data) self.assertEqual('usb_device', device.raw_bus) - device = UdevDevice(self.no_subsystem_device_data, None, None, None) + device = UdevDevice(None, self.no_subsystem_device_data) self.assertEqual(None, device.raw_bus) def test_getVendorOrProduct(self): """Test of UdevDevice.getVendorOrProduct().""" device = UdevDevice( - self.root_device, None, self.root_device_dmi_data, None) + None, self.root_device, None, self.root_device_dmi_data) self.assertEqual( 'FUJITSU SIEMENS', device.getVendorOrProduct('vendor')) self.assertEqual( @@ -2809,41 +2838,40 @@ self.assertRaises( AssertionError, device.getVendorOrProduct, 'nonsense') - device = UdevDevice(self.pci_device_data, None, None, None) + device = UdevDevice(None, self.pci_device_data) self.assertEqual('Unknown', device.getVendorOrProduct('vendor')) self.assertEqual('Unknown', device.getVendorOrProduct('product')) - device = UdevDevice(self.usb_device_data, None, None, None) + device = UdevDevice(None, self.usb_device_data) self.assertEqual('Unknown', device.getVendorOrProduct('vendor')) self.assertEqual('Unknown', device.getVendorOrProduct('product')) device = UdevDevice( - self.scsi_device_data, self.scsi_device_sysfs_data, None, None) + 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( - self.no_subsystem_device_data, None, None, None) + 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( - self.root_device, None, self.root_device_dmi_data, None) + 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( - self.root_device, None, self.root_device_dmi_data, None) + 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( - self.root_device, None, self.root_device_dmi_data, None) + None, self.root_device, None, self.root_device_dmi_data) self.assertEqual( 'FUJITSU SIEMENS', device.getVendorOrProductID('vendor')) self.assertEqual( @@ -2851,35 +2879,35 @@ self.assertRaises( AssertionError, device.getVendorOrProductID, 'nonsense') - device = UdevDevice(self.pci_device_data, None, None, None) + device = UdevDevice(None, self.pci_device_data) self.assertEqual(0x8086, device.getVendorOrProductID('vendor')) self.assertEqual(0x27C5, device.getVendorOrProductID('product')) - device = UdevDevice(self.usb_device_data, None, None, None) + device = UdevDevice(None, self.usb_device_data) self.assertEqual(0x46d, device.getVendorOrProductID('vendor')) self.assertEqual(0xa01, device.getVendorOrProductID('product')) device = UdevDevice( - self.scsi_device_data, self.scsi_device_sysfs_data, None, None) + 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( - self.no_subsystem_device_data, None, None, None) + 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( - self.root_device, None, self.root_device_dmi_data, None) + 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( - self.root_device, None, self.root_device_dmi_data, None) + None, self.root_device, None, self.root_device_dmi_data) self.assertEqual('LIFEBOOK E8210', device.product_id)