Clean up buddy checking & retrieval; make constructor choice between name & real buddy explicit.

shell/model/BuddyModel.py

@@ -2,25 +2,35 @@ from sugar.presence import PresenceService
 from sugar.canvas.IconColor import IconColor
 
 class BuddyModel:
-	def __init__(self, buddy=None):
+	def __init__(self, name=None, buddy=None):
+		if name and buddy:
+			raise RuntimeError("Must specify only _one_ of name or buddy.")
+
 		self._cur_activity = None
+		self._buddy_appeared_handler = None
 		self._pservice = PresenceService.get_instance()
 
 		self._buddy = buddy
+
+		# If given just a name, try to get the buddy from the PS first
+		if not self._buddy:
+			self._name = name
+			# FIXME: use public key, not name
+			self._buddy = self._pservice.get_buddy_by_name(self._name)
+
+		# If successful, copy properties from the PS buddy object
 		if self._buddy:
-			self.set_name(self._buddy.get_name())
+			self.__update_buddy(buddy)
-			self.set_color(self._buddy.get_color())
-			self._buddy.connect('property-changed',
-					self.__buddy_property_changed_cb)
 		else:
-			# if we don't have a buddy yet, connect to the PS
+			# Otherwise, connect to the PS's buddy-appeared signal and
-			# and wait until the buddy pops up on the network
+			# wait for the buddy to appear
-			self._pservice.connect('buddy-appeared', self.__buddy_appeared_cb)
+			self._buddy_appeared_handler = self._pservice.connect('buddy-appeared',
+					self.__buddy_appeared_cb)
+			self._name = name
+			# Set color to 'inactive'/'disconnected'
+			self.__set_color_from_string("#888888,#BBBBBB")
 
-	def set_name(self, name):
+	def __set_color_from_string(self, color_string):
-		self._name = name
-
-	def set_color(self, color_string):
 		self._color = IconColor(color_string)
 
 	def get_name(self):
@@ -30,21 +40,28 @@ class BuddyModel:
 		return self._color
 
 	def get_buddy(self):
-		# If we have a buddy already, just return
-		if self._buddy:
-			return self._buddy
-
-		# Otherwise try to get the buddy from the PS
-		self._buddy = self._pservice.get_buddy_by_name(self._name)
-		if self._buddy:
-			self._buddy.connect('property-changed',
-					self.__buddy_property_changed_cb)
 		return self._buddy
 
+	def __update_buddy(self, buddy):
+		if not buddy:
+			raise ValueError("Buddy cannot be None.")
+		self._buddy = buddy
+		self._name = self._buddy.get_name()
+		self.__set_color_from_string(self._buddy.get_color())
+		self._buddy.connect('property-changed', self.__buddy_property_changed_cb)
+
 	def __buddy_appeared_cb(self, pservice, buddy):
 		# FIXME: use public key rather than buddy name
-		if not self._buddy and buddy.get_name() == self._name:
+		if self._buddy or buddy.get_name() != self._name:
-			self.get_buddy()
+			return
+
+		if self._buddy_appeared_handler:
+			# Once we have the buddy, we no longer need to
+			# monitor buddy-appeared events
+			self._pservice.disconnect(self._buddy_appeared_handler)
+			self._buddy_appeared_handler = None
+
+		self.__update_buddy(buddy)
 
 	def __buddy_property_changed_cb(self, buddy, keys):
 		# all we care about right now is current activity

shell/model/Friends.py

@@ -50,9 +50,7 @@ class Friends(gobject.GObject):
 			success = cp.read([self._path])
 			if success:
 				for name in cp.sections():
-					buddy = BuddyModel()
+					buddy = BuddyModel(name)
-					buddy.set_name(name)
-					buddy.set_color(cp.get(name, 'color'))
 					self.add_friend(buddy)
 		except Exception, exc:
 			logging.error("Error parsing friends file: %s" % exc)