From 88013b2f9f93c3220812cd3b624db1423ac081ed Mon Sep 17 00:00:00 2001 From: Vincent Vinet Date: Mon, 19 Oct 2009 17:13:52 +0000 Subject: apply code review induced fixes, fix a few discovered bugs, enjoy, rinse and repeat --- diff --git a/addons/bubblemessage.py b/addons/bubblemessage.py index a859ef8..b8f7405 100644 --- a/addons/bubblemessage.py +++ b/addons/bubblemessage.py @@ -18,9 +18,9 @@ from sugar.tutorius.actions import * class BubbleMessage(Action): message = TStringProperty("Message") # Create the position as an array of fixed-size 2 - position = TArrayProperty([0,0], 2, 2) + position = TArrayProperty((0,0), 2, 2) # Do the same for the tail position - tail_pos = TArrayProperty([0,0], 2, 2) + tail_pos = TArrayProperty((0,0), 2, 2) def __init__(self, message=None, pos=None, speaker=None, tailpos=None): """ @@ -94,7 +94,7 @@ class BubbleMessage(Action): def exit_editmode(self, *args): x,y = self._drag.position - self.position = [int(x), int(y)] + self.position = (int(x), int(y)) if self._drag: self._drag.draggable = False self._drag = None diff --git a/addons/dialogmessage.py b/addons/dialogmessage.py index 22a223b..bdd4bc3 100644 --- a/addons/dialogmessage.py +++ b/addons/dialogmessage.py @@ -20,7 +20,7 @@ from sugar.tutorius.actions import * class DialogMessage(Action): message = TStringProperty("Message") - position = TArrayProperty([0, 0], 2, 2) + position = TArrayProperty((0, 0), 2, 2) def __init__(self, message=None, pos=None): """ diff --git a/addons/gtkwidgeteventfilter.py b/addons/gtkwidgeteventfilter.py index f5bd0de..5497af4 100644 --- a/addons/gtkwidgeteventfilter.py +++ b/addons/gtkwidgeteventfilter.py @@ -13,8 +13,8 @@ # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -from sugar.tutorius.filters import * -from sugar.tutorius.properties import * +from sugar.tutorius.filters import EventFilter +from sugar.tutorius.properties import TUAMProperty, TStringProperty from sugar.tutorius.gtkutils import find_widget class GtkWidgetEventFilter(EventFilter): diff --git a/addons/readfile.py b/addons/readfile.py index 4aa054e..0d276b9 100644 --- a/addons/readfile.py +++ b/addons/readfile.py @@ -25,7 +25,8 @@ class ReadFile(Action): def __init__(self, filename=None): """ - Calls activity.read_file to load a specified state + Calls activity.read_file to restore a specified state to an activity + like when restored from the journal. @param filename Path to the file to read """ Action.__init__(self) diff --git a/tutorius/TProbe.py b/tutorius/TProbe.py index 6c0883a..9d19f1b 100644 --- a/tutorius/TProbe.py +++ b/tutorius/TProbe.py @@ -1,4 +1,5 @@ import logging +LOGGER = logging.getLogger("sugar.tutorius.TProbe") import os import gobject @@ -26,25 +27,15 @@ import copy -------------------- ---------- """ +#TODO Add stub error handling for remote calls in the classes so that it will +# be clearer how errors can be handled in the future. + class TProbe(dbus.service.Object): """ Tutorius Probe Defines an entry point for Tutorius into activities that allows performing actions and registering events onto an activity via a DBUS Interface. - - Exposes the following dbus methods: - void registered(string service) - string ping() -> status - string install(string action) -> address - void update(string address, string action_props) - void uninstall(string address) - string subscribe(string pickled_event) -> address - void unsubscribe(string address) - - Exposes the following dbus Events: - eventOccured(event): - """ def __init__(self, activity_name, activity): @@ -54,9 +45,9 @@ class TProbe(dbus.service.Object): @param activity_name unique activity_id @param activity activity reference, must be a gtk container """ - logging.debug("TProbe :: Creating TProbe for %s (%d)", activity_name, os.getpid()) - logging.debug("TProbe :: Current gobject context: %s", str(gobject.main_context_default())) - logging.debug("TProbe :: Current gobject depth: %s", str(gobject.main_depth())) + LOGGER.debug("TProbe :: Creating TProbe for %s (%d)", activity_name, os.getpid()) + LOGGER.debug("TProbe :: Current gobject context: %s", str(gobject.main_context_default())) + LOGGER.debug("TProbe :: Current gobject depth: %s", str(gobject.main_depth())) # Moving the ObjectStore assignment here, in the meantime # the reference to the activity shouldn't be share as a # global variable but passed by the Probe to the objects @@ -163,6 +154,9 @@ class TProbe(dbus.service.Object): @param pickled_event string pickled EventFilter @return string unique name of registered event """ + #TODO Perform event unmapping once Tutorials use abstract events + # instead of concrete EventFilters that are tied to their + # implementation. eventfilter = pickle.loads(str(pickled_event)) # The callback uses the event defined previously and each @@ -200,14 +194,8 @@ class TProbe(dbus.service.Object): # The actual method we will call on the probe to send events def notify(self, event): - logging.debug("TProbe :: notify event %s", str(event)) - #HACK: reinstanciate the event with it's properties, to clear - # any internal state from getting pickled - if isinstance(event, TPropContainer): - newevent = type(event)(**event._props) - else: - newevent = event - self.eventOccured(pickle.dumps(newevent)) + LOGGER.debug("TProbe :: notify event %s", str(event)) + self.eventOccured(pickle.dumps(event)) # Return a unique name for this action def _generate_action_reference(self, action): @@ -225,10 +213,13 @@ class TProbe(dbus.service.Object): def _generate_event_reference(self, event): # TODO elavoie 2009-07-25 Should return a universal address name = event.__class__.__name__ - suffix = 1 + #Keep the counter to avoid looping all the time + suffix = getattr(self, '_event_ref_suffix', 0 ) + 1 while self._subscribedEvents.has_key(name+str(suffix)): suffix += 1 + + #setattr(self, '_event_ref_suffix', suffix) return name + str(suffix) @@ -238,25 +229,15 @@ class ProbeProxy: It provides an object interface to the TProbe, which requires pickled strings, across a DBus communication. - - Public Methods: - ProbeProxy(string activityName) :: Constructor - string install(Action action) - void update(Action action) - void uninstall(Action action) - void uninstall_all() - string subscribe(Event event, callable callback) - void unsubscribe(Event event, callable callback) - void unsubscribe_all() """ def __init__(self, activityName): """ Constructor - @param activityName unique activity id + @param activityName unique activity id. Must be a valid dbus bus name. """ - logging.debug("ProbeProxy :: Creating ProbeProxy for %s (%d)", activityName, os.getpid()) - logging.debug("ProbeProxy :: Current gobject context: %s", str(gobject.main_context_default())) - logging.debug("ProbeProxy :: Current gobject depth: %s", str(gobject.main_depth())) + LOGGER.debug("ProbeProxy :: Creating ProbeProxy for %s (%d)", activityName, os.getpid()) + LOGGER.debug("ProbeProxy :: Current gobject context: %s", str(gobject.main_context_default())) + LOGGER.debug("ProbeProxy :: Current gobject depth: %s", str(gobject.main_depth())) bus = dbus.SessionBus() self._object = bus.get_object(activityName, "/tutorius/Probe") self._probe = dbus.Interface(self._object, "org.tutorius.ProbeInterface") @@ -269,20 +250,21 @@ class ProbeProxy: self._subscribedEvents = {} self._registeredCallbacks = {} + self._object.connect_to_signal("eventOccured", self._handle_signal, dbus_interface="org.tutorius.ProbeInterface") def _handle_signal(self, pickled_event): event = pickle.loads(str(pickled_event)) - logging.debug("ProbeProxy :: Received Event : %s %s", str(event), str(event._props.items())) + LOGGER.debug("ProbeProxy :: Received Event : %s %s", str(event), str(event._props.items())) - logging.debug("ProbeProxy :: Currently %d events registered", len(self._registeredCallbacks)) + LOGGER.debug("ProbeProxy :: Currently %d events registered", len(self._registeredCallbacks)) if self._registeredCallbacks.has_key(event): for callback in self._registeredCallbacks[event].itervalues(): callback(event) else: for event in self._registeredCallbacks.keys(): - logging.debug("==== %s", str(event._props.items())) - logging.debug("ProbeProxy :: Event does not appear to be registered") + LOGGER.debug("==== %s", str(event._props.items())) + LOGGER.debug("ProbeProxy :: Event does not appear to be registered") def isAlive(self): try: @@ -300,41 +282,38 @@ class ProbeProxy: """ Install an action on the TProbe's activity @param action Action to install + @param block Force a synchroneous dbus call if True @return None """ - remote_call(self._probe.install, (pickle.dumps(action),), + return remote_call(self._probe.install, (pickle.dumps(action),), save_args(self.__update_action, action), block=block) - def update(self, action, block=False): + def update(self, action, newaction, block=False): """ Update an already installed action's properties and run it again @param action Action to update + @param newaction Action to update it with + @param block Force a synchroneous dbus call if True @return None """ + #TODO review how to make this work well if not action in self._actions: raise RuntimeWarning("Action not installed") - return - remote_call(self._probe.update, (self._actions[action], pickle.dumps(action._props)), block=block) + #TODO Check error handling + return remote_call(self._probe.update, (self._actions[action], pickle.dumps(newaction._props)), block=block) def uninstall(self, action, block=False): """ Uninstall an installed action @param action Action to uninstall + @param block Force a synchroneous dbus call if True """ if action in self._actions: remote_call(self._probe.uninstall,(self._actions.pop(action),), block=block) - def uninstall_all(self, block=False): - """ - Uninstall all installed actions - @return None - """ - for action in self._actions.keys(): - self.uninstall(action, block) - def __update_event(self, event, callback, address): - logging.debug("ProbeProxy :: Registered event %s with address %s", str(event), str(address)) + LOGGER.debug("ProbeProxy :: Registered event %s with address %s", str(hash(event)), str(address)) # Since multiple callbacks could be associated to the same # event signature, we will store multiple callbacks # in a dictionary indexed by the unique address @@ -365,26 +344,33 @@ class ProbeProxy: return address def __clear_event(self, address): + LOGGER.debug("ProbeProxy :: Unregistering adress %s", str(address)) # Cleanup everything if self._subscribedEvents.has_key(address): event = self._subscribedEvents[address] if self._registeredCallbacks.has_key(event)\ and self._registeredCallbacks[event].has_key(address): + LOGGER.debug("ProbeProxy :: POP ") self._registeredCallbacks[event].pop(address) if self._registeredCallbacks[event] == {}: + LOGGER.debug("ProbeProxy :: POP2 ") self._registeredCallbacks.pop(event) self._subscribedEvents.pop(address) + else: + LOGGER.debug("ProbeProxy :: unsubsribe address %s inconsistency : not registered", address) def subscribe(self, event, callback, block=True): """ Register an event listener @param event Event to listen for @param callback callable that will be called when the event occurs + @param block Force a synchroneous dbus call if True (Not allowed yet) @return address identifier used for unsubscribing """ + LOGGER.debug("ProbeProxy :: Registering event %s", str(hash(event))) if not block: raise RuntimeError("This function does not allow non-blocking mode yet") @@ -395,32 +381,41 @@ class ProbeProxy: save_args(self.__update_event, event, callback), block=block) - def unsubscribe(self, address, block=False): + def unsubscribe(self, address, block=True): """ Unregister an event listener @param address identifier given by subscribe() + @param block Force a synchroneous dbus call if True @return None """ + LOGGER.debug("ProbeProxy :: Unregister adress %s issued", str(address)) + if not block: + raise RuntimeError("This function does not allow non-blocking mode yet") if address in self._subscribedEvents.keys(): remote_call(self._probe.unsubscribe, (address,), return_cb=save_args(self.__clear_event, address), block=block) else: - logging.debug("ProbeProxy :: unsubsribe address %s failed : not registered", address) + LOGGER.debug("ProbeProxy :: unsubsribe address %s failed : not registered", address) - def unsubscribe_all(self, block=False): + def detach(self, block=False): """ - Unregister all event listeners - @return None + Detach the ProbeProxy from it's TProbe. All installed actions and + subscribed events should be removed. """ + for action in self._actions.keys(): + self.uninstall(action, block) + for address in self._subscribedEvents.keys(): self.unsubscribe(address, block) + class ProbeManager(object): """ The ProbeManager provides multiplexing across multiple activity ProbeProxies For now, it only handles one at a time, though. + Actually it doesn't do much at all. But it keeps your encapsulation happy """ def __init__(self): self._probes = {} @@ -438,9 +433,9 @@ class ProbeManager(object): def attach(self, activity_id): if activity_id in self._probes: raise RuntimeWarning("Activity already attached") - return self._probes[activity_id] = ProbeProxy(activity_id) + #TODO what do we do with this? Raise something? if self._probes[activity_id].isAlive(): print "Alive!" else: @@ -449,48 +444,64 @@ class ProbeManager(object): def detach(self, activity_id): if activity_id in self._probes: probe = self._probes.pop(activity_id) - probe.unsubscribe_all() - probe.uninstall_all() + probe.detach() - def install(self, action): - if self.currentActivity: - return self._probes[self.currentActivity].install(action) - else: - raise RuntimeWarning("No activity attached") - - def update(self, action): + def install(self, action, block=False): + """ + Install an action on the current activity + @param action Action to install + @param block Force a synchroneous dbus call if True + @return None + """ if self.currentActivity: - return self._probes[self.currentActivity].update(action) + return self._probes[self.currentActivity].install(action, block) else: raise RuntimeWarning("No activity attached") - def uninstall(self, action): + def update(self, action, newaction, block=False): + """ + Update an already installed action's properties and run it again + @param action Action to update + @param newaction Action to update it with + @param block Force a synchroneous dbus call if True + @return None + """ if self.currentActivity: - return self._probes[self.currentActivity].uninstall(action) + return self._probes[self.currentActivity].update(action, newaction, block) else: raise RuntimeWarning("No activity attached") - def uninstall_all(self): + def uninstall(self, action, block=False): + """ + Uninstall an installed action + @param action Action to uninstall + @param block Force a synchroneous dbus call if True + """ if self.currentActivity: - return self._probes[self.currentActivity].uninstall_all() + return self._probes[self.currentActivity].uninstall(action, block) else: raise RuntimeWarning("No activity attached") def subscribe(self, event, callback): + """ + Register an event listener + @param event Event to listen for + @param callback callable that will be called when the event occurs + @return address identifier used for unsubscribing + """ if self.currentActivity: return self._probes[self.currentActivity].subscribe(event, callback) else: raise RuntimeWarning("No activity attached") def unsubscribe(self, address): + """ + Unregister an event listener + @param address identifier given by subscribe() + @return None + """ if self.currentActivity: return self._probes[self.currentActivity].unsubscribe(address) else: raise RuntimeWarning("No activity attached") - def unsubscribe_all(self): - if self.currentActivity: - return self._probes[self.currentActivity].unsubscribe_all() - else: - raise RuntimeWarning("No activity attached") - diff --git a/tutorius/core.py b/tutorius/core.py index f51c5fb..c583a1f 100644 --- a/tutorius/core.py +++ b/tutorius/core.py @@ -34,6 +34,9 @@ class Tutorial (object): """ Tutorial Class, used to run through the FSM. """ + #Properties + probeManager = property(lambda self: self._probeMgr) + activityId = property(lambda self: self._activity_id) def __init__(self, name, fsm, filename=None): """ @@ -53,9 +56,6 @@ class Tutorial (object): self._activity_id = None #Rest of initialisation happens when attached - probeManager = property(lambda self: self._probeMgr) - activityId = property(lambda self: self._activity_id) - def attach(self, activity_id): """ Attach to a running activity @@ -79,7 +79,6 @@ class Tutorial (object): # Uninstall the whole FSM self.state_machine.teardown() - #FIXME (Old) There should be some amount of resetting done here... if not self._activity_id is None: self._probeMgr.detach(self._activity_id) self._activity_id = None @@ -107,6 +106,7 @@ class Tutorial (object): readfile = addon.create("ReadFile", filename=filename) if readfile: self._probeMgr.install(readfile) + #Uninstall now while we have the reference handy self._probeMgr.uninstall(readfile) class State(object): diff --git a/tutorius/creator.py b/tutorius/creator.py index 46d4852..fabe879 100644 --- a/tutorius/creator.py +++ b/tutorius/creator.py @@ -114,7 +114,6 @@ class Creator(object): """ self.introspecting = False eventfilter = addon.create('GtkWidgetEventFilter', - next_state=None, object_id=self._selected_widget, event_name=event_name) # undo actions so they don't persist through step editing @@ -244,7 +243,7 @@ class Creator(object): Quit editing and cleanup interface artifacts. """ self.introspecting = False - eventfilter = filters.EventFilter(None) + eventfilter = filters.EventFilter() # undo actions so they don't persist through step editing for action in self._tutorial.current_actions: action.exit_editmode() @@ -400,7 +399,9 @@ class EditToolBox(gtk.Window): def _list_prop_changed(self, widget, evt, action, propname, idx): try: - getattr(action, propname)[idx] = int(widget.get_text()) + attr = list(getattr(action, propname)) + attr[idx] = int(widget.get_text()) + setattr(action, propname, attr) except ValueError: widget.set_text(str(getattr(action, propname)[idx])) self.__parent._creator._action_refresh_cb(None, None, action) diff --git a/tutorius/dbustools.py b/tutorius/dbustools.py index ce28d98..1b685d7 100644 --- a/tutorius/dbustools.py +++ b/tutorius/dbustools.py @@ -1,4 +1,5 @@ import logging +LOGGER = logging.getLogger("sugar.tutorius.dbustools") def save_args(callable, *xargs, **xkwargs): def __call(*args, **kwargs): @@ -9,16 +10,32 @@ def save_args(callable, *xargs, **xkwargs): return __call def ignore(*args): - logging.debug("Unhandled asynchronous dbus call response with arguments: %s", str(args)) + LOGGER.debug("Unhandled asynchronous dbus call response with arguments: %s", str(args)) def logError(error): - logging.error("Unhandled asynchronous dbus call error: %s", error) + LOGGER.error("Unhandled asynchronous dbus call error: %s", error) def remote_call(callable, args, return_cb=None, error_cb=None, block=False): reply_cb = return_cb or ignore errhandler_cb = error_cb or logError if block: - return reply_cb(callable(*args)) + try: + ret_val = callable(*args) + LOGGER.debug("remote_call return arguments: %s", str(ret_val)) + except Exception, e: + #Use the specified error handler even for blocking calls + errhandler_cb(e) + + #Return value signature might be : + if ret_val is None: + #Nothing + return reply_cb() + elif type(ret_val) in (list, tuple): + #Several parameters + return reply_cb(*ret_val) + else: + #One parameter + return reply_cb(ret_val) else: callable(*args, reply_handler=reply_cb, error_handler=errhandler_cb) diff --git a/tutorius/filters.py b/tutorius/filters.py index 430b708..44621d5 100644 --- a/tutorius/filters.py +++ b/tutorius/filters.py @@ -26,10 +26,9 @@ class EventFilter(properties.TPropContainer): Base class for an event filter """ - def __init__(self, next_state=None): + def __init__(self): """ Constructor. - @param next_state name of the next state """ super(EventFilter, self).__init__() self._callback = None diff --git a/tutorius/overlayer.py b/tutorius/overlayer.py index 6b1b948..0a3d542 100644 --- a/tutorius/overlayer.py +++ b/tutorius/overlayer.py @@ -157,7 +157,7 @@ class TextBubble(gtk.Widget): A CanvasDrawableWidget drawing a round textbox and a tail pointing to a specified widget. """ - def __init__(self, text, speaker=None, tailpos=[0,0]): + def __init__(self, text, speaker=None, tailpos=(0,0)): """ Creates a new cairo rendered text bubble. @@ -199,7 +199,7 @@ class TextBubble(gtk.Widget): # TODO fetch speaker coordinates # draw bubble tail if present - if self.tailpos != [0,0]: + if self.tailpos != (0,0): context.move_to(xradius-width/4, yradius) context.line_to(self.tailpos[0], self.tailpos[1]) context.line_to(xradius+width/4, yradius) @@ -228,7 +228,7 @@ class TextBubble(gtk.Widget): context.fill() # bubble painting. Redrawing the inside after the tail will combine - if self.tailpos != [0,0]: + if self.tailpos != (0,0): context.move_to(xradius-width/4, yradius) context.line_to(self.tailpos[0], self.tailpos[1]) context.line_to(xradius+width/4, yradius) diff --git a/tutorius/properties.py b/tutorius/properties.py index b1c6361..9639010 100644 --- a/tutorius/properties.py +++ b/tutorius/properties.py @@ -98,12 +98,10 @@ class TPropContainer(object): # Providing the hash methods necessary to use TPropContainers # in a dictionary, according to their properties def __hash__(self): - try: - #Return a hash of properties (key, value) sorted by key - return hash(tuple(map(tuple,sorted(self._props.items(), cmp=lambda x, y: cmp(x[0], y[0]))))) - except TypeError: - #FIXME For list properties (and maybe others), hashing will fail, fallback to id - return id(self) + #Return a hash of properties (key, value) sorted by key + #We need to transform the list of property key, value lists into + # a tuple of key, value tuples + return hash(tuple(map(tuple,sorted(self._props.items(), cmp=lambda x, y: cmp(x[0], y[0]))))) def __eq__(self, e2): return self._props.items() == e2._props.items() @@ -230,6 +228,18 @@ class TArrayProperty(TutoriusProperty): self.min_size_limit = MinSizeConstraint(min_size_limit) self.default = self.validate(value) + #Make this thing hashable + def __setstate__(self, state): + self.max_size_limit = MaxSizeConstraint(state["max_size_limit"]) + self.min_size_limit = MinSizeConstraint(state["min_size_limit"]) + self.value = state["value"] + + def __getstate__(self): + return dict( + max_size_limit=self.max_size_limit.limit, + min_size_limit=self.min_size_limit.limit, + value=self.value, + ) class TColorProperty(TutoriusProperty): """ Represents a RGB color with 3 8-bit integer values. @@ -308,8 +318,10 @@ class TUAMProperty(TutoriusProperty): """ Represents a widget of the interface by storing its UAM. """ - # TODO : Pending UAM check-in (LP 355199) - pass + def __init__(self, value=None): + TutoriusProperty.__init__(self) + + self.type = "uam" class TAddonProperty(TutoriusProperty): """ diff --git a/tutorius/service.py b/tutorius/service.py index c52b7cd..21f0cf1 100644 --- a/tutorius/service.py +++ b/tutorius/service.py @@ -29,7 +29,7 @@ class Service(dbus.service.Object): @dbus.service.method(_DBUS_SERVICE_IFACE, in_signature="s", out_signature="") def launch(self, tutorialID): - """ Launch a tutorial + """ Launch a tutorial @param tutorialID unique tutorial identifier used to retrieve it from the disk """ if self._engine == None: @@ -59,12 +59,19 @@ class ServiceProxy: self._service = dbus.Interface(self._object, _DBUS_SERVICE_IFACE) def launch(self, tutorialID): + """ Launch a tutorial + @param tutorialID unique tutorial identifier used to retrieve it from the disk + """ remote_call(self._service.launch, (tutorialID, ), block=False) def stop(self): + """ Stop the current tutorial + """ remote_call(self._service.stop, (), block=False) def pause(self): + """ Interrupt the current tutorial and save its state in the journal + """ remote_call(self._service.pause, (), block=False) if __name__ == "__main__": -- cgit v0.9.1