Hi all,
I thought I'd poke my head here and say that proposal looks like a good
compromise for the data structures. I'll add that the "get_*" methods of
dev_data aren't needed if you're simply returning the values (just use
direct attribute access; you can update to properties later if
getter/setter functionality is needed without disrupting the interface).
- Keith
Jack Schwartz wrote:
> Hi Zhongyuan.
>
> Thanks for your feedback. Here's my assessment of requirements, based
> on both of our emails, and a new proposal which I believe addresses both
> of our needs and concerns. Hopefully it is not too long...
>
> A) You need a way to easily restrict the output to a given device type.
> I assume this why you proposed a dictionary keyed to the device type,
> at the top level. I didn't account for that in my original proposal.
>
> B) My proposal placed more of an emphasis on readability and clarity.
> This is why I originally proposed to store a device's attributes in a
> dictionary: attributes could be extracted by name.
>
> Additional requirements, as I see things:
>
> C) We don't want to make the data structures too complicated from a data
> access perspective. For example a dictionary of lists of dictionaries
> makes the code hard to follow.
>
> D) We don't want to introduce unnecessary overhead. If we can use
> something more lightweight than a dictionary and still have the
> functionality we need, then I would favor not using the dictionary.
>
>
> ddu_devscan() will be returning a group of items corresponding to
> devices, so it is appropriate to use a collection of some kind for the
> top level data structure it returns. At first I agreed with you that a
> dictionary would be best at the top level:
> - its items are named (via a key),
> - it can filter out unwanted items,
> - it doesn't have to keep a place for items it doesn't need.
> However, after thinking more, this could prove restrictive in the
> future. We return lots of information for each device, any of which
> could potentially be used as a filter for returning data. If the
> dictionary is keyed on device type and later we decide we need to
> filter on something else (or perhaps multiple items), say vendor ID
> and/or device ID, we're not set up for that.
>
> Instead, I propose a new class, call it devscan_data, which stores the
> full list of items returned by ddu_devscan() and has a method for
> searching and returning only items of a particular device type. Then
> in the future we could add other methods to support returning items
> based on some other filter criteria. It is also lightweight and
> provides only the functionality needed.
>
> What are the devscan_data list items? They would store the different
> fields of data per device. The list could point to dictionaries as I
> had originally proposed, to tuples as you have proposed, or to something
> else. I have changed my mind about dictionaries, and think they are the
> wrong tool; the number and type of fields will not change from one
> device to the next. I also still don't like tuples as the fields are
> hard to keep track of. Named tuples are better, but more heavyweight
> than if we defined a new class to hold the data. So I propose a new
> class, which would have attributes and get() methods for each field we
> want to return. Let's call it dev_data for now.
>
> To summarize:
>
> ddu_devscan() would return a devscan_data object defined like this:
>
> class devscan_data():
>
> def __init__()
> # Holds objects of dev_data.
> self.dev_data_list = []
>
> def add_dev_data_object(dev_data):
> self.dev_data_list.append(dev_data)
>
> def get_list_by_devtype(device_type):
> # loop through instance list data to find and return matches on
> # device_type
>
> Lists returned by get_list_by_devtype() point to objects of type dev_data.
> There is one of these objects per device returned.
>
> class dev_data:
>
> def __init__(...):
> self.device_type = ...
> self.description = ...
> self.driver_name
> self.instance_ID
> self.item_ID
> self.parent_item_ID
> self.attach_status
> self.vendor_id
> self.device_id
> self.class_code
> self.pci_path
>
> def get_device_type():
> return self.device_type
>
> def get_description():
> return self.description
>
> ...
> ...
>
> def get_pci_path:
> return self.pci_path
>
> Does this work for you?
>
> Thanks,
> Jack
>
> On 12/15/09 21:56, zhongyuan sun - Sun Microsystems - Beijing China
> wrote:
>> Hi Jack,
>>
>> It's OK ddu_devscan return dictionary type instead of a list of tuples.
>> I suggest use device type as this dictionary's key and the dictionary's
>> value would be a list of tuples.
>>
>> ddu_devscan description would be as following:
>>
>> Arguments:
>> - return_missing_only: Boolean; default value is True
>> - When True, ddu_devscan returns only the devices which are missing
>> drivers.
>> - When False, all devices found are returned.
>> - device_type: Type of devices to scan for. "All" to check all.
>> - Possible types
>> are:"audio"/"battery"/"cpu"/"cd_dvd"/"memory"/"network"/"storage"/"usb"/"video"/"others"
>>
>>
>> In each tuple of the list:
>> - ItemID:
>> - Parent Item ID:
>> /*these two fields(ItemID and Parent Item ID) used for device tree to
>> identify a device and the controller it attached to. Of course
>> for a
>> controller we do not need to set its parent Item ID.
>> */
>> - Device description:Device name
>> - Device ID:
>> - Classcode:
>> - PCI physical path:This field used to get its detail information.
>> - Driver name:
>> - Instance ID:
>> - Attachment status:
>> /*These two fields(Instance ID and Attachment status) used for
>> determine whether a driver
>> has been attached abnormally.
>> */
>> - Vendor ID
>>
>>
>> Here is a short example of ddu_devscan return value:
>>
>>>>> {'Audio': [(200,"","nVidia Corporation MCP55 High Definition
>>>>>
>> Audio","0x0371","00040300","[0,6,1]","unknown",-1,"Detached","0x10de")],'Network':[(0,"","nVidia
>> Corporation MCP55
>> Ethernet","0x0373","00068000","[80,8,0]","nge",0,"Attached","0x10de"),(1,"","nVidia
>> Corporation MCP55
>> Ethernet","0x0373","00068000","[80,9,0]","nge",1,"Attached","0x10de")],'DVD':[(400,"","nVidia
>> Corporation MCP55
>> IDE","0x036e","0001018a","[0,4,0]","pci-ide",0,"Attached","0x10de"),(401,400,"MATSHITA
>> DVD-RAM UJ-85JS","","","/pci at 0,0/pci-ide at 4/ide at 0/sd at
>> 0,0","sd",0,"Attached","")]}
>>
>>
>>
