Python - Returning list/ arry from function

Question

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("")

Have not used dbus but from, what I can see, with return on device_file, you are just returning on first execution and it does not let loop over all available results. Use list here and that will work. What does device_file anyway? name seems confusing. Please clarify? — NullException
– NullException, Commented Sep 11, 2013 at 17:35
device_file is the partition of the inserted usb. In the script above it is just a string. When you execute the script you get output like this "/dev/sdc1" — sundar_ima
– sundar_ima, Commented Sep 11, 2013 at 17:41

Martijn Pieters · Accepted Answer · 2013-09-11 17:36:22Z

4

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.

edited Sep 11, 2013 at 17:36

answered Sep 11, 2013 at 17:31

Martijn Pieters

1.1m326 gold badges4.2k silver badges3.4k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

georg Over a year ago

A generator would be more pythonic here.

Martijn Pieters Over a year ago

@thg435: It would be; but I fear that might be a step too far for the OP.

pjama Over a year ago

I think a generator would be suitable when memory efficiency were an issue (e.g. iterating over a large collection). In this case a list is probably sufficient.

sundar_ima Over a year ago

@ Martijn Pieters and pjama Both of your solution worked. I did not expect the answer to y question in few min after posting. Thank you very much.

pjama · Accepted Answer · 2013-09-11 17:32:56Z

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("")

msw · Accepted Answer · 2013-09-11 18:53:54Z

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.

Collectives™ on Stack Overflow

Python - Returning list/ arry from function

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related