More presence service rework

9 files changed, 382 insertions, 226 deletions

diff --git a/sugar/Makefile.am b/sugar/Makefile.am
index cb4eb03..8123484 100644
--- a/sugar/Makefile.am
+++ b/sugar/Makefile.am
@@ -1,4 +1,4 @@
-SUBDIRS = chat browser p2p shell session
+SUBDIRS = chat browser p2p shell session presence
 
 bin_SCRIPTS = sugar
 
diff --git a/sugar/chat/chat.py b/sugar/chat/chat.py
index 0604038..fbbdaf6 100755
--- a/sugar/chat/chat.py
+++ b/sugar/chat/chat.py
@@ -445,27 +445,12 @@ class BuddyChat(Chat):
 		del self._chats[self._buddy]
 
 
-class BuddyIconRequestHandler(object):
-	def __init__(self, group, stream):
-		self._group = group
-		self._stream = stream
-		self._stream.register_handler(self._handle_buddy_icon_request, "get_buddy_icon")
-
-	def _handle_buddy_icon_request(self):
-		"""XMLRPC method, return the owner's icon encoded with base64."""
-		icon = self._group.get_owner().get_icon()
-		if icon:
-			return base64.b64encode(icon)
-		return ''
-
-
 class GroupChat(Chat):
 	def __init__(self):
 		self._group = Group.get_from_id('local')
 		self._act_name = "Chat"
 		self._chats = {}
-		self._buddy_icon_tries = 0
-		
+
 		Chat.__init__(self, self)
 
 	def get_group(self):
@@ -482,7 +467,6 @@ class GroupChat(Chat):
 		# specific buddy chats
 		buddy_service = Service(name, CHAT_SERVICE_TYPE, CHAT_SERVICE_PORT)
 		self._buddy_stream = Stream.new_from_service(buddy_service, self._group)
-		self._buddy_icon_handler = BuddyIconRequestHandler(self._group, self._buddy_stream)
 		self._buddy_stream.set_data_listener(getattr(self, "_buddy_recv_message"))
 		buddy_service.register(self._group)
 
diff --git a/sugar/p2p/Stream.py b/sugar/p2p/Stream.py
index 375c66e..45d61a5 100644
--- a/sugar/p2p/Stream.py
+++ b/sugar/p2p/Stream.py
@@ -1,44 +1,49 @@
 import xmlrpclib
 import socket
 import traceback
+import random
 
 import network
 from MostlyReliablePipe import MostlyReliablePipe
+from sugar.presence import Service
 
 class Stream(object):
-	def __init__(self, service, group):
-		if not service:
-			raise ValueError("service must be valid")
+	def __init__(self, service):
+		if not isinstance(service, Service.Service):
+			raise ValueError("service must be valid.")
+		if not service.get_port():
+			raise ValueError("service must have an address.")
 		self._service = service
-		self._group = group
-		self._owner_nick_name = self._group.get_owner().get_nick_name()
-		self._port = self._service.get_port()
+		self._reader_port = self._service.get_port()
+		self._writer_port = self._reader_port
 		self._address = self._service.get_address()
 		self._callback = None
 
-	def new_from_service(service, group):
-		if service.is_multicast():
-			return MulticastStream(service, group)
+	def new_from_service(service, start_reader=True):
+		if not isinstance(service, Service.Service):
+			raise ValueError("service must be valid.")
+		if service.is_multicast_service():
+			return MulticastStream(service)
 		else:
-			return UnicastStream(service, group)
+			return UnicastStream(service, start_reader)
 	new_from_service = staticmethod(new_from_service)
 
 	def set_data_listener(self, callback):
 		self._callback = callback
 
-	def recv(self, nick_name, data):
-		if nick_name != self._owner_nick_name:
-			if self._callback:
-				self._callback(self._group.get_buddy(nick_name), data)
+	def _recv(self, address, data):
+		if self._callback:
+			self._callback(data)
 
 
 class UnicastStreamWriter(object):
-	def __init__(self, stream, service, owner_nick_name):
+	def __init__(self, stream, service):
 		# set up the writer
-		if not service:
+		if not isinstance(service, Service.Service):
 			raise ValueError("service must be valid")
 		self._service = service
-		self._owner_nick_name = owner_nick_name
+		if not service.get_address():
+			raise ValueError("service must have a valid address.")
 		self._address = self._service.get_address()
 		self._port = self._service.get_port()
 		self._xmlrpc_addr = "http://%s:%d" % (self._address, self._port)
@@ -47,7 +52,7 @@ class UnicastStreamWriter(object):
 	def write(self, xmlrpc_data):
 		"""Write some data to the default endpoint of this pipe on the remote server."""
 		try:
-			self._writer.message(None, None, self._owner_nick_name, xmlrpc_data)
+			self._writer.message(None, None, xmlrpc_data)
 			return True
 		except (socket.error, xmlrpclib.Fault, xmlrpclib.ProtocolError):
 			traceback.print_exc()
@@ -65,58 +70,79 @@ class UnicastStreamWriter(object):
 
 
 class UnicastStream(Stream):
-	def __init__(self, service, group):
-		Stream.__init__(self, service, group)
-		self._setup()
-
-	def _setup(self):
+	def __init__(self, service, start_reader=True):
+		"""Initializes the stream.  If the 'start_reader' argument is True,
+		the stream will initialize and start a new stream reader, if it
+		is False, no reader will be created and the caller must call the
+		start_reader() method to start the stream reader and be able to
+		receive any data from the stream."""
+		Stream.__init__(self, service)
+		if start_reader:
+			self.start_reader()
+
+	def start_reader(self, update_service_port=True):
+		"""Start the stream's reader, which for UnicastStream objects is
+		and XMLRPC server.  If there's a port conflict with some other
+		service, the reader will try to find another port to use instead.
+		Returns the port number used for the reader."""
 		# Set up the reader
 		started = False
 		tries = 10
-		port = self._service.get_port()
 		self._reader = None
 		while not started and tries > 0:
 			try:
-				self._reader = network.GlibXMLRPCServer(("", port))
+				self._reader = network.GlibXMLRPCServer(("", self._reader_port))
 				self._reader.register_function(self._message, "message")
+				if update_service_port:
+					self._service.set_port(self._reader_port)  # Update the service's port
 				started = True
 			except(socket.error):
-				port = port + 1
+				self._reader_port = random.randint(self._reader_port + 1, 65500)
 				tries = tries - 1
 		if self._reader is None:
-			print 'Could not start xmlrpc server.'
-		self._service.set_port(port)
+			print 'Could not start stream reader.'
+		return self._reader_port
 
-	def _message(self, nick_name, message):
+	def _message(self, message):
 		"""Called by the XMLRPC server when network data arrives."""
-		self.recv(nick_name, message)
+		address = network.get_authinfo()
+		self._recv(address, message)
 		return True
 
-	def register_handler(self, handler, name):
+	def register_reader_handler(self, handler, name):
+		"""Register a custom message handler with the reader.  This call
+		adds a custom XMLRPC method call with the name 'name' to the reader's
+		XMLRPC server, which then calls the 'handler' argument back when
+		a method call for it arrives over the network."""
 		if name == "message":
 			raise ValueError("Handler name 'message' is a reserved handler.")
 		self._reader.register_function(handler, name)
 
 	def new_writer(self, service):
-		return UnicastStreamWriter(self, service, self._owner_nick_name)
+		"""Return a new stream writer object."""
+		return UnicastStreamWriter(self, service)
 
 
 class MulticastStream(Stream):
-	def __init__(self, service, group):
-		Stream.__init__(self, service, group)
-		self._address = self._service.get_group_address()
-		self._setup()
-
-	def _setup(self):
-		self._pipe = MostlyReliablePipe('', self._address, self._port, self._recv_data_cb)
+	def __init__(self, service):
+		Stream.__init__(self, service)
+		self._internal_start_reader()
+
+	def start_reader(self):
+		return self._reader_port
+
+	def _internal_start_reader(self):
+		if not service.get_address():
+			raise ValueError("service must have a valid address.")
+		self._pipe = MostlyReliablePipe('', self._address, self._reader_port,
+				self._recv_data_cb)
 		self._pipe.start()
 
 	def write(self, data):
-		self._pipe.send(self._owner_nick_name + " |**| " + data)
+		self._pipe.send(data)
 
-	def _recv_data_cb(self, addr, data, user_data=None):
-		[ nick_name, data ] = data.split(" |**| ", 2)
-		self.recv(nick_name, data)
+	def _recv_data_cb(self, address, data, user_data=None):
+		self._recv(address, data)
 
 	def new_writer(self, service=None):
 		return self
diff --git a/sugar/presence/Buddy.py b/sugar/presence/Buddy.py
index 5201a04..9d94b9d 100644
--- a/sugar/presence/Buddy.py
+++ b/sugar/presence/Buddy.py
@@ -1,46 +1,108 @@
 import pwd
 import os
+import base64
 
 import pygtk
 pygtk.require('2.0')
-import gtk
+import gtk, gobject
 
+from sugar.p2p import Stream
+from sugar.p2p import network
 
-#from sugar import env
 
 PRESENCE_SERVICE_TYPE = "_presence_olpc._tcp"
 
 
-class Buddy(object):
+class Buddy(gobject.GObject):
 	"""Represents another person on the network and keeps track of the
 	activities and resources they make available for sharing."""
+	__gsignals__ = {
+		'icon-changed': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+						 ([])),
+		'service-added': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+						 ([gobject.TYPE_PYOBJECT])),
+		'service-removed': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+						 ([gobject.TYPE_PYOBJECT]))
+	}
+
 	def __init__(self, service):
+		gobject.GObject.__init__(self)
 		self._services = {}
 		self._nick_name = service.get_name()
 		self._address = service.get_address()
 		self._valid = False
 		self._icon = None
+		self._icon_tries = 0
+		self._owner = False
 		self.add_service(service)
 
+	def _request_buddy_icon_cb(self, result_status, response, user_data):
+		"""Callback when icon request has completed."""
+		icon = response
+		service = user_data
+		if result_status == network.RESULT_SUCCESS:
+			if icon and len(icon):
+				icon = base64.b64decode(icon)
+				print "Buddy icon for '%s' is size %d" % (self._nick_name, len(icon))
+				self.set_icon(icon)
+
+		if (result_status == network.RESULT_FAILED or not icon) and self._icon_tries < 3:
+			self._icon_tries = self._icon_tries + 1
+			print "Failed to retrieve buddy icon for '%s' on try %d of %d" % (self._nick_name, \
+					self._icon_tries, 3)
+			gobject.timeout_add(1000, self._request_buddy_icon, service)
+		return False
+
+	def _request_buddy_icon(self, service):
+		"""Contact the buddy to retrieve the buddy icon."""
+		buddy_stream = Stream.Stream.new_from_service(service, start_reader=False)
+		writer = buddy_stream.new_writer(service)
+		success = writer.custom_request("get_buddy_icon", self._request_buddy_icon_cb, service)
+		if not success:
+			del writer, buddy_stream
+			gobject.timeout_add(1000, self._request_buddy_icon, service)
+		return False
+
 	def add_service(self, service):
-		"""Adds a new service to this buddy's service list."""
+		"""Adds a new service to this buddy's service list, returning
+		True if the service was successfully added, and False if it was not."""
+		if service.get_name() != self._nick_name:
+			return False
+		if service.get_address() != self._address:
+			return False
 		if service.get_type() in self._services.keys():
-			return
-		self._services.keys[service.get_type()] = service
-		# FIXME: send out signal for new service found
+			return False
+		self._services[service.get_type()] = service
+		if self._valid:
+			self.emit("service-added", service)
 		if service.get_type() == PRESENCE_SERVICE_TYPE:
 			# A buddy isn't valid until its official presence
 			# service has been found and resolved
 			self._valid = True
+			self._request_buddy_icon(service)
+		return True
 
 	def remove_service(self, service):
 		"""Remove a service from a buddy; ie, the activity was closed
 		or the buddy went away."""
-		if service.get_type() in self._services.keys():
+		if service.get_address() != self._address:
+			return
+		if service.get_name() != self._nick_name:
+			return
+		if self._services.has_key(service.get_type()):
+			if self._valid:
+				self.emit("service-removed", service)
 			del self._services[service.get_type()]
 		if service.get_type() == PRESENCE_SERVICE_TYPE:
 			self._valid = False
 
+	def get_service_of_type(self, stype):
+		"""Return a service of a certain type, or None if the buddy
+		doesn't provide that service."""
+		if self._services.has_key(stype):
+			return self._services[stype]
+		return None
+
 	def is_valid(self):
 		"""Return whether the buddy is valid or not.  A buddy is
 		not valid until its official presence service has been found
@@ -63,65 +125,23 @@ class Buddy(object):
 	def get_address(self):
 		return self._address
 
-	def add_service(self, service):
-		if service.get_name() != self._nick_name:
-			return False
-		if service.get_address() != self._address:
-			return False
-		if self._services.has_key(service.get_type()):
-			return False
-		self._services[service.get_type()] = service
-
-	def remove_service(self, stype):
-		if self._services.has_key(stype):
-			del self._services[stype]
-
-	def get_service(self, stype):
-		if self._services.has_key(stype):
-			return self._services[stype]
-		return None
-		
 	def get_nick_name(self):
 		return self._nick_name
 
 	def set_icon(self, icon):
 		"""Can only set icon for other buddies.  The Owner
 		takes care of setting it's own icon."""
-		self._icon = icon
-		# FIXME: do callbacks for icon-changed
-		
+		if icon != self._icon:
+			self._icon = icon
+			self.emit("icon-changed")
 
-class Owner(Buddy):
-	"""Class representing the owner of this machine/instance."""
-	def __init__(self):
-		nick = env.get_nick_name()
-		if not nick:
-			nick = pwd.getpwuid(os.getuid())[0]
-		if not nick or not len(nick):
-			nick = "n00b"
-
-		Buddy.__init__(self)
-
-		user_dir = env.get_user_dir()
-		if not os.path.exists(user_dir):
-			try:
-				os.makedirs(user_dir)
-			except OSError:
-				print 'Could not create user directory.'
-
-		for fname in os.listdir(user_dir):
-			if not fname.startswith("buddy-icon."):
-				continue
-			fd = open(os.path.join(user_dir, fname), "r")
-			self._icon = fd.read()
-			fd.close()
-			break
+	def is_owner(self):
+		return self._owner
 
-	def set_icon(self, icon):
-		"""Can only set icon in constructor for now."""
-		pass
-		
-	def add_service(self, service):
-		"""Do nothing here, since all services we need to know about
-		are registered with us by our group."""
-		pass
+
+class Owner(Buddy):
+	"""Class representing the owner of the machine.  This is the client
+	portion of the Owner, paired with the server portion in Owner.py."""
+	def __init__(self, service):
+		Buddy.__init__(self, service)
+		self._owner = True
diff --git a/sugar/presence/PresenceService.py b/sugar/presence/PresenceService.py
index 4dbbf0e..c759fb4 100644
--- a/sugar/presence/PresenceService.py
+++ b/sugar/presence/PresenceService.py
@@ -2,16 +2,40 @@ import threading
 import avahi, dbus, dbus.glib, dbus.dbus_bindings, gobject
 import Buddy
 import Service
+import Group
 import os
 
 
-ACTION_SERVICE_APPEARED = 'appeared'
-ACTION_SERVICE_DISAPPEARED = 'disappeared'
+def _get_local_ip_address(ifname):
+	"""Call Linux specific bits to retrieve our own IP address."""
+	import socket
+	import sys
+	import fcntl
 
-class PresenceService(object):
+	addr = None
+	SIOCGIFADDR = 0x8915
+	sockfd = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+	try:
+		ifreq = (ifname + '\0'*32)[:32]
+		result = fcntl.ioctl(sockfd.fileno(), SIOCGIFADDR, ifreq)
+		addr = socket.inet_ntoa(result[20:24])
+	except IOError, exc:
+		print "Error getting IP address: %s" % exc
+	sockfd.close()
+	return addr
+
+
+class PresenceService(gobject.GObject):
 	"""Object providing information about the presence of Buddies
 	and what activities they make available to others."""
 
+	__gsignals__ = {
+		'buddy-appeared': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+						 ([gobject.TYPE_PYOBJECT])),
+		'buddy-disappeared': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE,
+						 ([gobject.TYPE_PYOBJECT]))
+	}
+
 	__lock = threading.Lock()
 	__instance = None
 
@@ -26,12 +50,20 @@ class PresenceService(object):
 	get_instance = staticmethod(get_instance)
 
 	def __init__(self, debug=False):
+		gobject.GObject.__init__(self)
+
 		self._debug = debug
 		self._lock = threading.Lock()
 		self._started = False
 
+		# interface -> IP address: interfaces we've gotten events on so far
+		self._local_addrs = {}
+
 		# nick -> Buddy: buddies we've found
 		self._buddies = {}
+		# Our owner object
+		self._owner = None
+
 		# group UID -> Group: groups we've found
 		self._groups = {}
 
@@ -77,6 +109,10 @@ class PresenceService(object):
 		if self._debug:
 			print "PresenceService(%d): %s" % (os.getpid(), msg)
 
+	def get_owner(self):
+		"""Return the owner of this machine/instance, if we've recognized them yet."""
+		return self._owner
+
 	def _resolve_service_error_handler(self, err):
 		self._log("error resolving service: %s" % err)
 
@@ -97,6 +133,49 @@ class PresenceService(object):
 			found.append(service)
 		return found
 
+	def _is_special_service_type(self, stype):
+		"""Return True if the service type is a special, internal service
+		type, and False if it's not."""
+		if stype == Buddy.PRESENCE_SERVICE_TYPE:
+			return True
+		if Group.is_group_service_type(stype):
+			return True
+		return False
+
+	def _handle_new_service_for_buddy(self, service):
+		"""Deal with a new discovered service object."""
+		# Once a service is resolved, we match it up to an existing buddy,
+		# or create a new Buddy if this is the first service known about the buddy
+		added = was_valid = False
+		name = service.get_name()
+		buddy = None
+		try:
+			buddy = self._buddies[name]
+			was_valid = buddy.is_valid()
+			added = buddy.add_service(service)
+		except KeyError:
+			# Should this service mark the owner?
+			if service.get_address() in self._local_addrs.values():
+				buddy = Buddy.Owner(service)
+				self._owner = buddy
+			else:
+				buddy = Buddy.Buddy(service)
+			self._buddies[name] = buddy
+			added = True
+		if not was_valid and buddy.is_valid():
+			self.emit("buddy-appeared", buddy)
+		return buddy
+
+	def _handle_new_service_for_group(self, service, buddy):
+		# If the serivce is a group service, merge it into our groups list
+		if not buddy:
+			return
+		group = None
+		if not self._groups.has_key(service.get_type()):
+			group = Group.Group(service)
+		else:
+			group = self._groups[service.get_type()]
+
 	def _resolve_service_reply_cb(self, interface, protocol, name, stype, domain, host, aprotocol, address, port, txt, flags):
 		"""When the service discovery finally gets here, we've got enough information about the
 		service to assign it to a buddy."""
@@ -108,7 +187,6 @@ class PresenceService(object):
 				stype=stype, domain=domain)
 		if not len(found):
 			return False
-
 		for service in found:
 			self._unresolved_services.remove(service)
 
@@ -118,23 +196,11 @@ class PresenceService(object):
 		service.set_port(port)
 		service.set_properties(txt)
 
-		# Once a service is resolved, we match it up to an existing buddy,
-		# or create a new Buddy if this is the first service known about the buddy
-		added = was_valid = False
-		try:
-			buddy = self._buddies[name]
-			was_valid = buddy.is_valid()
-			added = buddy.add_service(service)
-		except KeyError:
-			buddy = Buddy.Buddy(service)
-			self._buddies[name] = buddy
-			added = True
-		if not was_valid and buddy.is_valid():
-			# FIXME: send out "new buddy" signals
-			pass
-		if added:
-			# FIXME: send out buddy service added signals
-			pass
+		# Merge the service into our buddy and group lists, if needed
+		buddy = self._handle_new_service_for_buddy(service)
+		if service.is_group_service():
+			self._handle_new_service_for_group(service, buddy)
+
 		return False
 
 	def _resolve_service_reply_cb_glue(self, interface, protocol, name, stype, domain, host, aprotocol, address, port, txt, flags):
@@ -160,8 +226,16 @@ class PresenceService(object):
 			service = Service.Service(name, stype, domain)
 			self._unresolved_services.append(service)
 
+		# Find out the IP address of this interface, if we haven't already
+		if interface not in self._local_addrs.keys():
+			ifname = self._server.GetNetworkInterfaceNameByIndex(interface)
+			if ifname:
+				addr = _get_local_ip_address(ifname)
+				if addr:
+					self._local_addrs[interface] = addr
+
 		# If we care about the service right now, resolve it
-		if stype in self._allowed_service_types or stype == Buddy.PRESENCE_SERVICE_TYPE:
+		if stype in self._allowed_service_types or self._is_special_service_type(stype):
 			gobject.idle_add(self._resolve_service, interface, protocol, name, stype, domain, flags)
 		return False
 
@@ -170,23 +244,18 @@ class PresenceService(object):
 
 	def _service_disappeared_cb(self, interface, protocol, name, stype, domain, flags):
 		self._log("service '%s' of type '%s' in domain '%s' on %i.%i disappeared." % (name, stype, domain, interface, protocol))
-		# Remove the service from our unresolved services list
-		found = self._find_service(self._unresolved_services, name=name,
-				stype=stype, domain=domain)
-
-		buddy = None
 		try:
+			# Remove the service from the buddy
 			buddy = self._buddies[name]
-		except KeyError:
-			pass
-
-		# Remove the service from the buddy
-		if buddy:
-			buddy.remove_service(found[0])
-			# FIXME: send buddy service remove signals
+			# FIXME: need to be more careful about how we remove services
+			# from buddies; this could be spoofed
+			service = buddy.get_service_of_type(stype)
+			buddy.remove_service(service)
 			if not buddy.is_valid():
+				self.emit("buddy-disappeared", buddy)
 				del self._buddies[name]
-				# FIXME: send buddy disappeared message
+		except KeyError:
+			pass
 
 		for service in found:
 			self._unresolved_services.remove(service)
@@ -246,9 +315,11 @@ class PresenceService(object):
 	def track_service_type(self, stype):
 		"""Requests that the Presence service look for and recognize
 		a certain mDNS service types."""
+		if not self._started:
+			raise RuntimeError("presence service must be started first.")
 		if not type(stype) == type(""):
 			raise ValueError("service type must be a string.")
-		if stype == Buddy.PRESENCE_SERVICE_TYPE:
+		if self._is_special_service_type(stype):
 			return
 		if stype in self._allowed_service_types:
 			return
@@ -263,6 +334,8 @@ class PresenceService(object):
 
 	def untrack_service_type(self, stype):
 		"""Stop tracking a certain mDNS service."""
+		if not self._started:
+			raise RuntimeError("presence service must be started first.")
 		if not type(stype) == type(""):
 			raise ValueError("service type must be a string.")
 		if name in self._allowed_service_types:
@@ -270,19 +343,25 @@ class PresenceService(object):
 
 	def register_service(self, service):
 		"""Register a new service, advertising it to other Buddies on the network."""
+		if not self._started:
+			raise RuntimeError("presence service must be started first.")
+
 		rs_name = service.get_name()
 		rs_stype = service.get_type()
 		rs_port = service.get_port()
 		if type(rs_port) != type(1) and rs_port <= 1024:
 			raise ValueError("invalid service port.")
 		rs_props = service.get_properties()
+		rs_domain = service.get_domain()
+		if not rs_domain or not len(rs_domain):
+			rs_domain = ""
 		self._log("registered service name '%s' type '%s' on port %d with args %s" % (rs_name, rs_stype, rs_port, rs_props))
 
 		try:
 			group = dbus.Interface(self._bus.get_object(avahi.DBUS_NAME, self._server.EntryGroupNew()), avahi.DBUS_INTERFACE_ENTRY_GROUP)
 			info = ["%s=%s" % (k, v) for k, v in rs_props.items()]
 			group.AddService(avahi.IF_UNSPEC, avahi.PROTO_UNSPEC, 0, rs_name, rs_stype,
-					"", "", # domain, host (let the system figure it out)
+					rs_domain, "", # let Avahi figure the 'host' out
 					dbus.UInt16(rs_port), info,)
 			group.Commit()
 		except dbus.dbus_bindings.DBusException, exc:
@@ -295,11 +374,19 @@ class PresenceService(object):
 		return group
 
 	def get_buddy_by_nick_name(self, nick_name):
+		"""Look up and return a buddy by nickname."""
 		if self._buddies.has_key(nick_name):
 			return self._buddies[nick_name]
 		return None
 
+	def get_buddy_by_address(self, address):
+		for buddy in self._buddies.values():
+			if buddy.get_address == address:
+				return buddy
+		return None
+
 	def get_buddies(self):
+		"""Return the entire buddy list."""
 		return self._buddies.values()
 
 #################################################################
@@ -347,6 +434,7 @@ class PresenceServiceTestCase(unittest.TestCase):
 		buddy = ps.get_buddy_by_nick_name("Paul")
 		assert buddy, "The registered buddy was not found after 2 seconds!"
 		assert buddy.is_valid(), "The buddy was invalid, since no presence was advertised."
+		assert buddy.is_owner() == True, "The buddy was not the owner, but it should be!"
 
 	def addToSuite(suite):
 		suite.addTest(PresenceServiceTestCase("testNoServices"))
diff --git a/sugar/presence/Service.py b/sugar/presence/Service.py
index c709dd8..868edcb 100644
--- a/sugar/presence/Service.py
+++ b/sugar/presence/Service.py
@@ -1,4 +1,5 @@
 import avahi
+import Group
 
 def _txt_to_dict(txt):
 	"""Convert an avahi-returned TXT record formatted
@@ -17,6 +18,18 @@ def _txt_to_dict(txt):
 		prop_dict[key] = value
 	return prop_dict
 
+def _is_multicast_address(address):
+	"""Simple numerical check for whether an IP4 address
+	is in the range for multicast addresses or not."""
+	if not address:
+		return False
+	if address[3] != '.':
+		return False
+	first = int(address[:3])
+	if first >= 224 and first <= 239:
+		return True
+	return False
+
 class Service(object):
 	"""Encapsulates information about a specific ZeroConf/mDNS
 	service as advertised on the network."""
@@ -30,11 +43,15 @@ class Service(object):
 		if not stype.endswith("._tcp") and not stype.endswith("._udp"):
 			raise ValueError("must specify a TCP or UDP service type.")
 
-		if not domain or (type(domain) != type("") and type(domain) != type(u"")):
+		if type(domain) != type("") and type(domain) != type(u""):
 			raise ValueError("must specify a domain.")
-		if domain != "local" and domain != u"local":
+		if len(domain) and domain != "local" and domain != u"local":
 			raise ValueError("must use the 'local' domain (for now).")
 
+		# Group services must have multicast addresses
+		if Group.is_group_service_type(stype) and address and not _is_multicast_address(address):
+			raise ValueError("group service type specified, but address was not multicast.")
+		
 		self._name = name
 		self._stype = stype
 		self._domain = domain
@@ -46,17 +63,38 @@ class Service(object):
 		self.set_properties(properties)
 
 	def get_name(self):
+		"""Return the service's name, usually that of the
+		buddy who provides it."""
 		return self._name
 
+	def is_multicast_service(self):
+		"""Return True if the service's address is a multicast address,
+		False if it is not."""
+		return _is_multicast_address(self._address)
+
+	def is_group_service(self):
+		"""Return True if the service represents a Group,
+		False if it does not."""
+		return Group.is_group_service_type(self._stype)
+
 	def get_one_property(self, key):
+		"""Return one property of the service, or None
+		if the property was not found.  Cannot distinguish
+		between lack of a property, and a property value that
+		actually is None."""
 		if key in self._properties.keys():
 			return self._properties[key]
 		return None
 
 	def get_properties(self):
+		"""Return a python dictionary of all the service's
+		properties."""
 		return self._properties
 
 	def set_properties(self, properties):
+		"""Set the service's properties from either an Avahi
+		TXT record (a list of lists of integers), or a
+		python dictionary."""
 		self._properties = {}
 		if type(properties) == type([]):
 			self._properties = _txt_to_dict(properties)
@@ -64,6 +102,7 @@ class Service(object):
 			self._properties = properties
 
 	def get_type(self):
+		"""Return the service's service type."""
 		return self._stype
 
 	def get_port(self):
@@ -83,16 +122,14 @@ class Service(object):
 				raise ValueError("must specify a valid address.")
 			if not len(address):
 				raise ValueError("must specify a valid address.")
+			if Group.is_group_service_type(self._stype) and not _is_multicast_address(address):
+				raise ValueError("group service type specified, but address was not multicast.")
 		self._address = address
 
 	def get_domain(self):
+		"""Return the ZeroConf/mDNS domain the service was found in."""
 		return self._domain
 
-	def is_olpc_service(self):
-		if self._stype.endswith("._olpc._udp") or self._stype.endswith(".olpc._tcp"):
-			return True
-		return False
-
 
 #################################################################
 # Tests
@@ -138,6 +175,10 @@ class ServiceTestCase(unittest.TestCase):
 		# Only accept local for now
 		self._test_init_fail(self._DEF_NAME, self._DEF_STYPE, "foobar", self._DEF_ADDRESS,
 				self._DEF_PORT, self._DEF_PROPS, "invalid domain")
+		# Make sure "" works
+		service = Service(self._DEF_NAME, self._DEF_STYPE, "", self._DEF_ADDRESS,
+				self._DEF_PORT, self._DEF_PROPS)
+		assert service, "Empty domain was not accepted!"
 
 	def testAddress(self):
 		self._test_init_fail(self._DEF_NAME, self._DEF_STYPE, self._DEF_DOMAIN, [],
@@ -186,6 +227,30 @@ class ServiceTestCase(unittest.TestCase):
 		value = service.get_one_property(key)
 		assert value is not None and value == expected_value, "service properties weren't correct after init."
 
+	def testGroupService(self):
+		# Valid group service type, non-multicast address
+		group_stype = "_af5e5a7c998e89b9a_group_olpc._udp"
+		self._test_init_fail(self._DEF_NAME, group_stype, self._DEF_DOMAIN, self._DEF_ADDRESS,
+				self._DEF_PORT, self._DEF_PROPS, "group service type, but non-multicast address")
+
+		# Valid group service type, None address
+		service = Service(self._DEF_NAME, group_stype, self._DEF_DOMAIN, None,
+				self._DEF_PORT, self._DEF_PROPS)
+		assert service.get_address() == None, "address was not None as expected!"
+		# Set address to invalid multicast address
+		try:
+			service.set_address(self._DEF_ADDRESS)
+		except ValueError, exc:
+			pass
+		else:
+			self.fail("expected a ValueError for invalid address.")
+
+		# Valid group service type and multicast address, ensure it works
+		mc_addr = "224.0.0.34"
+		service = Service(self._DEF_NAME, group_stype, self._DEF_DOMAIN, mc_addr,
+				self._DEF_PORT, self._DEF_PROPS)
+		assert service.get_address() == mc_addr, "address was not expected address!"
+
 	def addToSuite(suite):
 		suite.addTest(ServiceTestCase("testName"))
 		suite.addTest(ServiceTestCase("testType"))
@@ -195,6 +260,7 @@ class ServiceTestCase(unittest.TestCase):
 		suite.addTest(ServiceTestCase("testGoodInit"))
 		suite.addTest(ServiceTestCase("testAvahiProperties"))
 		suite.addTest(ServiceTestCase("testBoolProperty"))
+		suite.addTest(ServiceTestCase("testGroupService"))
 	addToSuite = staticmethod(addToSuite)
 
 
diff --git a/sugar/shell/Makefile.am b/sugar/shell/Makefile.am
index 387838d..54da99c 100644
--- a/sugar/shell/Makefile.am
+++ b/sugar/shell/Makefile.am
@@ -3,4 +3,5 @@ sugar_PYTHON =		\
 	__init__.py	\
 	activity.py	\
 	shell.py        \
-	PresenceWindow.py
+	PresenceWindow.py \
+	Owner.py
diff --git a/sugar/shell/PresenceWindow.py b/sugar/shell/PresenceWindow.py
index 783d23c..d5eddc2 100644
--- a/sugar/shell/PresenceWindow.py
+++ b/sugar/shell/PresenceWindow.py
@@ -3,8 +3,8 @@ pygtk.require('2.0')
 import gtk
 import gobject
 
-from sugar.p2p.Group import Group
 from sugar.p2p.Stream import Stream
+from sugar.presence.PresenceService import PresenceService
 
 class PresenceWindow(gtk.Window):
 	_MODEL_COL_NICK = 0
@@ -14,10 +14,11 @@ class PresenceWindow(gtk.Window):
 	def __init__(self):
 		gtk.Window.__init__(self)
 
-		self._group = Group.get_from_id('local')
-		self._group.add_presence_listener(self._on_group_presence_event)
-		self._group.add_service_listener(self._on_group_service_event)
-		self._group.join()
+		self._pservice = PresenceService.get_instance()
+		self._pservice.connect("buddy-appeared", self._on_buddy_appeared_cb)
+		self._pservice.connect("buddy-disappeared", self._on_buddy_disappeared_cb)
+		self._pservice.set_debug(True)
+		self._pservice.start()
 		
 		self._setup_ui()
 
@@ -79,62 +80,25 @@ class PresenceWindow(gtk.Window):
 			self._chats[buddy] = chat
 			chat.connect_to_shell()
 
-	def _request_buddy_icon_cb(self, result_status, response, user_data):
-		icon = response
-		buddy = user_data
-		if result_status == network.RESULT_SUCCESS:
-			if icon and len(icon):
-				icon = base64.b64decode(icon)
-				print "Buddy icon for '%s' is size %d" % (buddy.get_nick_name(), len(icon))
-				buddy.set_icon(icon)
-
-		if (result_status == network.RESULT_FAILED or not icon) and self._buddy_icon_tries < 3:
-			self._buddy_icon_tries = self._buddy_icon_tries + 1
-			print "Failed to retrieve buddy icon for '%s' on try %d of %d" % (buddy.get_nick_name(), \
-					self._buddy_icon_tries, 3)
-			gobject.timeout_add(1000, self._request_buddy_icon, buddy)
-		return False
-
-	def _request_buddy_icon(self, buddy):
-		# FIXME need to use the new presence service when it's done
-		service = buddy.get_service('_olpc_chat._tcp')
-		buddy_stream = Stream.new_from_service(service, self._group)
-		writer = buddy_stream.new_writer(service)
-		icon = writer.custom_request("get_buddy_icon", self._request_buddy_icon_cb, buddy)
-
-	def _on_group_service_event(self, action, service):
-		if action == Group.SERVICE_ADDED:
-			# Look for the olpc chat service
-			# FIXME need to use the new presence service when it's done
-			if service.get_type() == '_olpc_chat._tcp':
-				# Find the buddy this service belongs to
-				buddy = self._group.get_buddy(service.get_name())
-				if buddy and buddy.get_address() == service.get_address():
-					# Try to get the buddy's icon
-					if buddy.get_nick_name() != self._group.get_owner().get_nick_name():
-						print "Requesting buddy icon from '%s'." % buddy.get_nick_name()
-						gobject.idle_add(self._request_buddy_icon, buddy)
-		elif action == Group.SERVICE_REMOVED:
-			pass
-			
 	def __buddy_icon_changed_cb(self, buddy):
 		it = self._get_iter_for_buddy(buddy)
 		self._buddy_list_model.set(it, self._MODEL_COL_ICON, buddy.get_icon_pixbuf())
 
-	def _on_group_presence_event(self, action, buddy):
-		if buddy.get_nick_name() == self._group.get_owner().get_nick_name():
+	def _on_buddy_appeared_cb(self, pservice, buddy):
+		if buddy.is_owner():
 			# Do not show ourself in the buddy list
-			pass
-		elif action == Group.BUDDY_JOIN:
-			aniter = self._buddy_list_model.append(None)
-			self._buddy_list_model.set(aniter,
-									   self._MODEL_COL_NICK, buddy.get_nick_name(),
-									   self._MODEL_COL_BUDDY, buddy)
-			buddy.connect('icon-changed', self.__buddy_icon_changed_cb)
-		elif action == Group.BUDDY_LEAVE:
-			aniter = self._get_iter_for_buddy(buddy)
-			if aniter:
-				self._buddy_list_model.remove(aniter)
+			return
+
+		aniter = self._buddy_list_model.append(None)
+		self._buddy_list_model.set(aniter,
+								   self._MODEL_COL_NICK, buddy.get_nick_name(),
+								   self._MODEL_COL_BUDDY, buddy)
+		buddy.connect('icon-changed', self.__buddy_icon_changed_cb)
+
+	def _on_buddy_disappeared_cb(self, pservice, buddy):
+		aniter = self._get_iter_for_buddy(buddy)
+		if aniter:
+			self._buddy_list_model.remove(aniter)
 
 	def _get_iter_for_buddy(self, buddy):
 		aniter = self._buddy_list_model.get_iter_first()
diff --git a/sugar/shell/shell.py b/sugar/shell/shell.py
index 2567491..04d2fa0 100755
--- a/sugar/shell/shell.py
+++ b/sugar/shell/shell.py
@@ -8,6 +8,7 @@ import gtk
 import pango
 
 from sugar.shell.PresenceWindow import PresenceWindow
+from sugar.shell.Owner import ShellOwner
 
 activity_counter = 0
 
@@ -240,6 +241,9 @@ class ActivityContainer(dbus.service.Object):
 		
 		self.current_activity = None
 
+		# Create our owner service
+		self._owner = ShellOwner()
+
 	def show(self):
 		self.window.show()
 
@@ -359,7 +363,10 @@ def main():
 	presence_window.show()
 	
 	console.set_parent_window(activity_container.window)
-	
+	try:
+		gtk.main()
+	except KeyboardInterrupt:
+		pass
+
 if __name__ == "__main__":
 	main()
-	gtk.main()