Patchwork [bitbake-devel,1/2] hob2: update DeployImageDialog for seperated tool

login
register
mail settings
Submitter Kang Kai
Date June 5, 2012, 3:37 a.m.
Message ID <bbe60345d083624fd2fd6bc4a60c2cb948fe2993.1338866958.git.kai.kang@windriver.com>
Download mbox | patch
Permalink /patch/29223/
State New
Headers show

Comments

Kang Kai - June 5, 2012, 3:37 a.m.
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.

Remove some extra spaces at same time.

Signed-off-by: Kang Kai <kai.kang@windriver.com>
---
 bitbake/lib/bb/ui/crumbs/hig.py |   82 ++++++++++++++++++++++++++++++++-------
 1 files changed, 68 insertions(+), 14 deletions(-)
Darren Hart - June 5, 2012, 5:38 p.m.
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@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):
Kang Kai - June 6, 2012, 2:33 a.m.
On 2012?06?06? 01:38, Darren Hart wrote:
>
> 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.

Hi Darren,

Thanks a lot for your detail comments.

One thing I want to address that the difference between standaonle 
widget and called by hob is that the standalone widget add a image 
selection button.
Maybe I use the wrong word "singleton" but that why I update the deploy 
image dialog layout.

> 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.
Ok, I'll break up it.

>
>> Remove some extra spaces at same time.
>>
>> Signed-off-by: Kang Kai<kai.kang@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.

Without set the size request, the scroll bar doesn't show the text when 
the deploy dialog shows.
It is ok to 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?

As I mentioned above, I just add a image selection button when the tool 
is run separately. It is convenience So I need to adjust the layout.


>
>>           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.

This update just to fix runtime error after commit 
094742bed2fc01d55f572da946fcfa7a48521401 when there is no USB device 
then the program crashes.
If we want to deploy to other device, I'll update it.

>
>>               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?

Ok, thanks.

>
>> +                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?
Fine, if need I would update this part.

Regards,
Kai
>
>> +            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):
Kang Kai - June 6, 2012, 3:01 a.m.
On 2012?06?06? 10:33, Kang Kai wrote:
> On 2012?06?06? 01:38, Darren Hart wrote:
>>
>> 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.
>
> Hi Darren,
>
> Thanks a lot for your detail comments.
>
> One thing I want to address that the difference between standaonle 
> widget and called by hob is that the standalone widget add a image 
> selection button.
> Maybe I use the wrong word "singleton" but that why I update the 
> deploy image dialog layout.
>
>> 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.
> Ok, I'll break up it.
>
>>
>>> Remove some extra spaces at same time.
>>>
>>> Signed-off-by: Kang Kai<kai.kang@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.
>
> Without set the size request, the scroll bar doesn't show the text 
> when the deploy dialog shows.
> It is ok to 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?
>
> As I mentioned above, I just add a image selection button when the 
> tool is run separately. It is convenience So I need to adjust the layout.
>
>
>>
>>>           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.
>
> This update just to fix runtime error after commit 
> 094742bed2fc01d55f572da946fcfa7a48521401 when there is no USB device 
> then the program crashes.
> If we want to deploy to other device, I'll update it.
>
>>
>>>               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?
>
> Ok, thanks.

I am afraid the <br> is not supported here. It obeys pango markup 
language and <br> is not  supported.

Regards,
Kai
>
>>
>>> +                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?
> Fine, if need I would update this part.
>
> Regards,
> Kai
>>
>>> +            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):
>
>
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/bitbake-devel

Patch

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)
+
         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)
 
         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)
         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*')
             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"
+                else:
+                    lbl = "<b>No selection made</b>\nYou have not selected USB device"
+            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):