Reputation: 3890
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
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
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
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