[bitbake-devel] [PATCH 1/2] hob2: update DeployImageDialog for seperated tool
Darren Hart
dvhart at linux.intel.com
Tue Jun 5 17:38:11 UTC 2012
On 06/04/2012 08:37 PM, Kang Kai wrote:
> Part of [Yocto 2388]
>
> Update class DeployImageDialog that could be used by a standalone
> deploy image tool.
>
> Update the method to get all usb device to avoid runtime error, and
> get the deploy image process exit status then give user a information
> dialog.
These are functionally different tasks that should probably broken up
into a couple patches. This allows us to revert one while maintaining
the other should a problem arise. It also makes reviewing the code easier.
>
> Remove some extra spaces at same time.
>
> Signed-off-by: Kang Kai <kai.kang at windriver.com>
> ---
> bitbake/lib/bb/ui/crumbs/hig.py | 82 ++++++++++++++++++++++++++++++++-------
> 1 files changed, 68 insertions(+), 14 deletions(-)
>
> diff --git a/bitbake/lib/bb/ui/crumbs/hig.py b/bitbake/lib/bb/ui/crumbs/hig.py
> index 7c9e73f..7d8daad 100644
> --- a/bitbake/lib/bb/ui/crumbs/hig.py
> +++ b/bitbake/lib/bb/ui/crumbs/hig.py
> @@ -20,6 +20,7 @@
> # with this program; if not, write to the Free Software Foundation, Inc.,
> # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>
> +import glob
> import gtk
> import gobject
> import hashlib
> @@ -63,7 +64,7 @@ class CrumbsMessageDialog(CrumbsDialog):
> """
> def __init__(self, parent=None, label="", icon=gtk.STOCK_INFO):
> super(CrumbsMessageDialog, self).__init__("", parent, gtk.DIALOG_DESTROY_WITH_PARENT)
> -
> +
> self.set_border_width(6)
> self.vbox.set_property("spacing", 12)
> self.action_area.set_property("spacing", 12)
> @@ -748,21 +749,28 @@ class DeployImageDialog (CrumbsDialog):
>
> __dummy_usb__ = "--select a usb drive--"
>
> - def __init__(self, title, image_path, parent, flags, buttons=None):
> + def __init__(self, title, image_path, parent, flags, buttons=None, singleton=False):
> super(DeployImageDialog, self).__init__(title, parent, flags, buttons)
>
> self.image_path = image_path
> + self.singleton = singleton
>
> self.create_visual_elements()
> self.connect("response", self.response_cb)
>
> def create_visual_elements(self):
> + self.set_size_request(600, 400)
> label = gtk.Label()
> label.set_alignment(0.0, 0.5)
> markup = "<span font_desc='12'>The image to be written into usb drive:</span>"
> label.set_markup(markup)
> self.vbox.pack_start(label, expand=False, fill=False, padding=2)
>
> + table = gtk.Table(2, 10, False)
> + table.set_col_spacings(5)
> + table.set_row_spacings(5)
> + self.vbox.pack_start(table, expand=True, fill=True)
> +
It isn't obvious to me why things like setting the size request and
creating a new table are required to make this stand alone. Do these
changes fit under the "Update class DeployImageDialog that could be used
by a standalone deploy image tool." ? If so, fine. If not, they might be
better done in a separate patch.
> scroll = gtk.ScrolledWindow()
> scroll.set_policy(gtk.POLICY_NEVER, gtk.POLICY_AUTOMATIC)
> scroll.set_shadow_type(gtk.SHADOW_IN)
> @@ -770,11 +778,26 @@ class DeployImageDialog (CrumbsDialog):
> tv.set_editable(False)
> tv.set_wrap_mode(gtk.WRAP_WORD)
> tv.set_cursor_visible(False)
> - buf = gtk.TextBuffer()
> - buf.set_text(self.image_path)
> - tv.set_buffer(buf)
> + self.buf = gtk.TextBuffer()
> + self.buf.set_text(self.image_path)
> + tv.set_buffer(self.buf)
> scroll.add(tv)
> - self.vbox.pack_start(scroll, expand=True, fill=True)
> + table.attach(scroll, 0, 10, 0, 1)
> +
> + if self.singleton:
> + gobject.signal_new("select_image_clicked", self, gobject.SIGNAL_RUN_FIRST,
> + gobject.TYPE_NONE, ())
> + icon = gtk.Image()
> + pix_buffer = gtk.gdk.pixbuf_new_from_file(hic.ICON_IMAGES_DISPLAY_FILE)
> + icon.set_from_pixbuf(pix_buffer)
> + button = gtk.Button("Select Image")
> + button.set_image(icon)
> + button.set_size_request(140, 50)
> + table.attach(button, 9, 10, 1, 2, gtk.FILL, 0, 0, 0)
> + button.connect("clicked", self.select_image_button_clicked_cb)
> +
> + separator = gtk.HSeparator()
> + self.vbox.pack_start(separator, expand=False, fill=False, padding=10)
What does the self.singleton indicate? How is it related to running
stand-alone or within Hob?
> self.usb_desc = gtk.Label()
> self.usb_desc.set_alignment(0.0, 0.5)
> @@ -789,7 +812,7 @@ class DeployImageDialog (CrumbsDialog):
> for usb in self.find_all_usb_devices():
> self.usb_combo.append_text("/dev/" + usb)
> self.usb_combo.set_active(0)
> - self.vbox.pack_start(self.usb_combo, expand=True, fill=True)
> + self.vbox.pack_start(self.usb_combo, expand=False, fill=False)
This would appear to be a superfluous layout change unrelated to the
patch topic...
> self.vbox.pack_start(self.usb_desc, expand=False, fill=False, padding=2)
>
> self.progress_bar = HobProgressBar()
> @@ -800,13 +823,19 @@ class DeployImageDialog (CrumbsDialog):
> self.vbox.show_all()
> self.progress_bar.hide()
>
> + def set_image_text_buffer(self, image_path):
> + self.buf.set_text(image_path)
> +
> + def set_image_path(self, image_path):
> + self.image_path = image_path
> +
> def popen_read(self, cmd):
> tmpout, errors = bb.process.run("%s" % cmd)
> return tmpout.strip()
>
> def find_all_usb_devices(self):
> usb_devs = [ os.readlink(u)
> - for u in self.popen_read('ls /dev/disk/by-id/usb*').split()
> + for u in glob.glob('/dev/disk/by-id/usb*')
Secondary to the goal of the patch, but why are we restricting ourselves
to sub drives? Why not read /proc/partitions and omit a configurable
blacklist of devices (like /dev/sda for example). Again, secondary to
this patch series.
> if not re.search(r'part\d+', u) ]
> return [ '%s' % u[u.rfind('/')+1:] for u in usb_devs ]
>
> @@ -815,6 +844,9 @@ class DeployImageDialog (CrumbsDialog):
> (self.popen_read('cat /sys/class/block/%s/device/vendor' % dev),
> self.popen_read('cat /sys/class/block/%s/device/model' % dev))
>
> + def select_image_button_clicked_cb(self, button):
> + self.emit('select_image_clicked')
> +
> def usb_combo_changed_cb(self, usb_combo):
> combo_item = self.usb_combo.get_active_text()
> if not combo_item or combo_item == self.__dummy_usb__:
> @@ -826,12 +858,34 @@ class DeployImageDialog (CrumbsDialog):
>
> def response_cb(self, dialog, response_id):
> if response_id == gtk.RESPONSE_YES:
> + lbl = ''
> combo_item = self.usb_combo.get_active_text()
> - if combo_item and combo_item != self.__dummy_usb__:
> + if combo_item and combo_item != self.__dummy_usb__ and self.image_path:
> cmdline = bb.ui.crumbs.utils.which_terminal()
> if cmdline:
> - cmdline += "\"sudo dd if=" + self.image_path + " of=" + combo_item + "\""
> - bb.process.Popen(shlex.split(cmdline))
> + tmpname = os.tmpnam()
> + cmdline += "\"sudo dd if=" + self.image_path + " of=" + combo_item + "; echo $? > " + tmpname + "\""
> + deploy_process = bb.process.Popen(shlex.split(cmdline))
> + deploy_process.wait()
> + tmpfile = open(tmpname)
> + if int(tmpfile.readline().strip()) == 0:
> + lbl = "<b>Deploy image successfully</b>"
> + else:
> + lbl = "<b>Deploy image failed</b>\nPlease try again"
> + tmpfile.close()
> + os.remove(tmpname)
> +
> + else:
> + if not self.image_path:
> + lbl = "<b>No selection made</b>\nYou have not selected image to deploy"
"<b>No selection made.</b>\nPlease select an image."
Is <br> appropriate here instead of \n?
> + else:
> + lbl = "<b>No selection made</b>\nYou have not selected USB device"
"<b>No selection made.</b>\nPlease select a device."
Again, why are we limiting devices to USB devices?
> + if len(lbl):
> + crumbs_dialog = CrumbsMessageDialog(self, lbl, gtk.STOCK_DIALOG_INFO)
> + button = crumbs_dialog.add_button("Close", gtk.RESPONSE_OK)
> + HobButton.style_button(button)
> + crumbs_dialog.run()
> + crumbs_dialog.destroy()
>
> def update_progress_bar(self, title, fraction, status=None):
> self.progress_bar.update(fraction)
> @@ -1035,7 +1089,7 @@ class LayerSelectionDialog (CrumbsDialog):
> # create visual elements on the dialog
> self.create_visual_elements()
> self.connect("response", self.response_cb)
> -
> +
> def create_visual_elements(self):
> layer_widget, self.layer_store = self.gen_layer_widget(self.layers, self.all_layers, self, None)
> layer_widget.set_size_request(450, 250)
> @@ -1210,7 +1264,7 @@ class ImageSelectionDialog (CrumbsDialog):
> if f.endswith('.' + real_image_type):
> imageset.add(f.rsplit('.' + real_image_type)[0].rsplit('.rootfs')[0])
> self.image_list.append(f)
> -
> +
> for image in imageset:
> self.image_store.set(self.image_store.append(), 0, image, 1, False)
>
> @@ -1226,7 +1280,7 @@ class ImageSelectionDialog (CrumbsDialog):
> for f in self.image_list:
> if f.startswith(self.image_store[path][0] + '.'):
> self.image_names.append(f)
> - break
> + break
> iter = self.image_store.iter_next(iter)
>
> class ProxyDetailsDialog (CrumbsDialog):
--
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
More information about the bitbake-devel
mailing list