sundar_ima
sundar_ima

Reputation: 3890

Python - Returning list/ arry from function

I am writting a small program to list the available usb devices. I have created a seperated file with function so that it can be accessed any time from other scripts. The main objective of this function is to return list/ array as output. If I use print command it successfully prints list of available devices. However, when i use return command it only retun the first detected device. I have gone through other similar question from SO but couldnot find any valid solution for this issue. Here is the code I have tried using dbus. Any help is appriciated.

#!/usr/bin/python2.7
import dbus

def find_usb(self):
    bus = dbus.SystemBus()
    ud_manager_obj = bus.get_object("org.freedesktop.UDisks", "/org/freedesktop/UDisks")
    ud_manager = dbus.Interface(ud_manager_obj, 'org.freedesktop.UDisks')

    for dev in ud_manager.EnumerateDevices():
        device_obj = bus.get_object("org.freedesktop.UDisks", dev)
        device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
        if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") == "usb" and device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsPartition"):
            if device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsMounted"):
                device_file = device_props.Get('org.freedesktop.UDisks.Device', "DeviceFile")
                #print device_file
                return device_file

            else:
                print "Device not mounted"
find_usb("")

Upvotes: 0

Views: 203

Answers (3)

msw
msw

Reputation: 43487

A big problem with even making sense of your code was that the logical structure was hidden in a lot of indentation. Here is a rewrite that preserves the operation of the original while allowing the structure to be seen.

#!/usr/bin/python2.7
import dbus

def find_usb(self):
    ofud = 'org.freedesktop.UDisks' # convenience string for brevity
    bus = dbus.SystemBus()

    ud_manager_obj = bus.get_object(ofud, "/org/freedesktop/UDisks")
    ud_manager = dbus.Interface(ud_manager_obj, ofud)

    for dev in ud_manager.EnumerateDevices():
        device_obj = bus.get_object(ofud, dev)
        device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
        dp_get = device.props.Get   # convenience method for brevity
        if (dp_get(ofud, "DriveConnectionInterface") == "usb" and
            dp_get(ofud, "DeviceIsPartition")):
            if dp_get(ofud, "DeviceIsMounted"):
                device_file = dp_get(ofud + '.Device', "DeviceFile")
                print device_file
                return device_file
            else:
                print "Device not mounted"
                return None         # implied
         else:
             return None            # implied
    return None

find_usb("")

Others have amply answered what the problem was, but it was not helped by the difficulty in reading. Part of that isn't your fault, the dbus interface is verbose, but there are things you can to to tame that as I did with ofud and dp_get.

The PEP-8 Style Guide for Python and PEP-20 The Zen of Python exist in large part to help you write code that make errors easier to see. In particular, the deep nesting of your conditionals could be cleaned up a lot:

if dp_get(ofud, 'DriveConnectionInterface') != 'usb':
    continue
if dp_get(ofud, 'DeviceIsPartition') and dp_get(oufd, 'DeviceIsMounted'):
     device_file = dp_get(oufd + '.Device', "DeviceFile")
     found.append(device_file)

Since your original code doesn't do anything with non-usb devices it is clearer to "fast fail" to the next iteration of the loop and skip an entire level of indentation. There are certainly other ways to make the intention of the code more obvious, I've just shown a few.

Readability matters.

Upvotes: 0

Martijn Pieters
Martijn Pieters

Reputation: 1121296

You are returning the first device you match. Build a list instead and return that list:

def find_usb(self):
    bus = dbus.SystemBus()
    ud_manager_obj = bus.get_object("org.freedesktop.UDisks", "/org/freedesktop/UDisks")
    ud_manager = dbus.Interface(ud_manager_obj, 'org.freedesktop.UDisks')

    found = []

    for dev in ud_manager.EnumerateDevices():
        device_obj = bus.get_object("org.freedesktop.UDisks", dev)
        device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
        if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") == "usb" and device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsPartition"):
            if device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsMounted"):
                device_file = device_props.Get('org.freedesktop.UDisks.Device', "DeviceFile")
                #print device_file
                found.append(device_file)

            else:
                print "Device not mounted"

    return found

A return statement immediately ends the function; by putting a return in your loop, you exit the function at that point and end the loop prematurely.

The alternative is to make this a generator function, by using a yield statement to produce your matches; you can then loop over the result of self.find_usb() once only, but you'll be producing device files on demand instead:

def find_usb(self):
    bus = dbus.SystemBus()
    ud_manager_obj = bus.get_object("org.freedesktop.UDisks", "/org/freedesktop/UDisks")
    ud_manager = dbus.Interface(ud_manager_obj, 'org.freedesktop.UDisks')

    for dev in ud_manager.EnumerateDevices():
        device_obj = bus.get_object("org.freedesktop.UDisks", dev)
        device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
        if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") == "usb" and device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsPartition"):
            if device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsMounted"):
                device_file = device_props.Get('org.freedesktop.UDisks.Device', "DeviceFile")
                #print device_file
                yield device_file

            else:
                print "Device not mounted"

    return found

Generators are a more advanced technique however.

Upvotes: 4

pjama
pjama

Reputation: 3044

You should create a list variable and append the devices to it. Then at the end of the function, return the list.

#!/usr/bin/python2.7
import dbus

def find_usb(self):
    devices = [] # instantiate empty list variable!
    bus = dbus.SystemBus()
    ud_manager_obj = bus.get_object("org.freedesktop.UDisks", "/org/freedesktop/UDisks")
    ud_manager = dbus.Interface(ud_manager_obj, 'org.freedesktop.UDisks')

    for dev in ud_manager.EnumerateDevices():
        device_obj = bus.get_object("org.freedesktop.UDisks", dev)
        device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
        if device_props.Get('org.freedesktop.UDisks.Device', "DriveConnectionInterface") == "usb" and device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsPartition"):
            if device_props.Get('org.freedesktop.UDisks.Device', "DeviceIsMounted"):
                device_file = device_props.Get('org.freedesktop.UDisks.Device', "DeviceFile")
                #print device_file
                devices.append(device_file)

            else:
                print "Device not mounted"
    return devices

find_usb("")

Upvotes: 1

Related Questions