Code review comment for lp:~adeuring/launchpad/hwdb-class-udev-device-5

Revision history for this message
Abel Deuring (adeuring) wrote :

This branch is another step to prepare the BWDB submission processing script for submissions from Karmic's HWDB client.

- a fix of two properties for class UdevDevice:

BaseDevice.vendor_id_for_db and BaseDevice.product_id_for_db look up the dictionaries DB_FORMAT_FOR_VENDOR_ID and DB_FORMAT_FOR_PRODUCT_ID, respectively, using BaseDevice,raw_bus as the key. The udev data contains slightly different bus/subsystem data, so we get the raw_value "scsi_device" for submissions with udev data, while we have raw_bus == "scsi" for HAL-based submissions. Hence I added a "scsi_device" entry to the dicitionaries. Since the raw_bus value "scsi_device" is not used by HAL, there are no side effects for HAL-based submissions.

- a new property UdevDevice.driver_name

- The method BaseDevice.translateScsiBus() figures out, if a given device where rw_bus is "scsi" (submission with HAL data) or "scsi_device" (submissions with udev data) is a real SCSI device or if it is in fact a USB storage device or a IDE/ATA/SATA device. (Since most od today's storage devices understand SCSI commands, the Linux kernel accesses them via the SCSI layer.) We detect the real device type of given device with raw_bus in ("scsi", "scsi_device") by looking at the device that controls the given (fake) SCSI device: If the controller is a USB device, we have a memory stick, a card reader or a "real" IDE/SATA disk connected to USB adapter; if the controller is a PCI device, we have a real SCSI device, or an IDE/ATA/SATA device. What exactly ww have in the latter case, can be derived from the PCI class/subclass of the controller.

For submissions with HAL, the controller is the grandparent of the SCSI device, but for submissions with udev data, the controller is the grand-grand-grand-grand-grandparent (6th ancestor).

Since the core logic of translateScsiBus() is identical for HAL and udev data, I did not move the method to the two classes HALDevice, UdevDevice derived from BaseDevice. Instead, I defined a new property BaseDevice.scsi_controller that must be implemented by both HALDevice and UdevDevice.

A implementation of UdevDevice.scsi_controller is still missing -- when I started to write tests for that property, I noticed that the diff of the branch would probably exceed our 800 lines limit, so I left that for another sequel.

The change in test_hwdb_submission_processing.py: replace scsi_device_data by scsi_scanner_device_data may look a bit odd. There are two reasons for this change:

(1) I wanted a real-world SCSI vendor name that is shorter than 8 characters in order to have proper test data for UdevDevice.vendor_id_for_db, which right-pads vendor names with spaces to length 8.

(2) The old test device "MATSHITA DVD-RAM UJ-841S" is in fact an IDE device; it might show up as an IDE device later again in a test of BaseDevice.translateScsiBus().

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)

« Back to merge proposal