[Sugar-devel] [sugar 0.98 PATCH] sl#3800: In "My Settings" -> "ModemConfiguration", convert the procedure to process the results of "connection.get_secrets('gsm', _secrets_cb, _secrets_err_cb)" from pseudo-asynchronous to asynchronous.

[James Cameron]

I like this change.  I have review comments below.

On Mon, Aug 13, 2012 at 03:18:33AM +0530, Ajay Garg wrote:
> In the method "get_modem_settings()", we call
> "connection.get_secrets('gsm', _secrets_cb, _secrets_err_cb)". This is
> an asynchronous call.
> 
> As of now, we busy wait, till the specified "result" callback is called
> (which in this case happens to be "def _secrets_cb(secrets)". After this
> is called, we return the modem-settings.
> 
> Instead of busy waiting, pass a callabck from the callee function.

                                      ^ typo

> Thereafter, when "def _secrets_cb(secrets)" is hit, execute this
> callback function, passing the modem-settings.
> 
>  extensions/cpsection/modemconfiguration/model.py |   15 ++++++++-------
>  extensions/cpsection/modemconfiguration/view.py  |    4 +++-
>  2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/extensions/cpsection/modemconfiguration/model.py b/extensions/cpsection/modemconfiguration/model.py
> index 969b5d9..8c900c9 100755
> --- a/extensions/cpsection/modemconfiguration/model.py
> +++ b/extensions/cpsection/modemconfiguration/model.py
> @@ -26,7 +26,7 @@ def get_connection():
>      return network.find_gsm_connection()
>  
>  
> -def get_modem_settings():
> +def get_modem_settings(callback):
>      modem_settings = {}
>      connection = get_connection()
>      if not connection:
> @@ -48,6 +48,10 @@ def get_modem_settings():
>          modem_settings['password'] = gsm_secrets.get('password', '')
>          modem_settings['pin'] = gsm_secrets.get('pin', '')
>  
> +        # sl#3800: We return the settings, via the "_secrets_cb()
> +        #          method", instead of busy-waiting.

The usefulness of this comment is obvious for patch review, but I
don't think it needs to be in the final.

> +        callback(modem_settings)
> +
>      def _secrets_err_cb(err):
>          secrets_call_done[0] = True
>          if isinstance(err, dbus.exceptions.DBusException) and \
> @@ -57,14 +61,11 @@ def get_modem_settings():
>              logging.error('Error retrieving GSM secrets: %s', err)
>  
>      # must be called asynchronously as this re-enters the GTK main loop

         ^ the above is no longer true, it no longer re-enters the
         main loop.

> +    #
> +    # sl#3800: We return the settings, via the "_secrets_cb()" method,
> +    #          instead of busy-waiting.

As before, comment adds nothing for eventual reader of code.

>      connection.get_secrets('gsm', _secrets_cb, _secrets_err_cb)
>  
> -    # wait til asynchronous execution completes
> -    while not secrets_call_done[0]:
> -        gtk.main_iteration()
> -
> -    return modem_settings
> -
>  
>  def _set_or_clear(_dict, key, value):
>      """Update a dictionary value for a specific key. If value is None or
> diff --git a/extensions/cpsection/modemconfiguration/view.py b/extensions/cpsection/modemconfiguration/view.py
> index 4ce6c0d..39a30c0 100644
> --- a/extensions/cpsection/modemconfiguration/view.py
> +++ b/extensions/cpsection/modemconfiguration/view.py
> @@ -118,7 +118,9 @@ class ModemConfiguration(SectionView):
>          entry.handler_unblock_by_func(self.__entry_changed_cb)
>  
>      def setup(self):
> -        settings = self._model.get_modem_settings()
> +        self._model.get_modem_settings(self.populate_entries)
> +
> +    def populate_entries(self, settings):
>          self._populate_entry(self._username_entry,
>              settings.get('username', ''))
>          self._populate_entry(self._number_entry, settings.get('number', ''))
> -- 
> 1.7.4.4
> 
> _______________________________________________
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
> http://lists.sugarlabs.org/listinfo/sugar-devel

Well done.

-- 
James Cameron
http://quozl.linux.org.au/