commit 4533309490bb30fb5b2827d0f1140afa339799bf
Author: Marc Dequènes (Duck) <Duck@DuckCorp.org>
Date:   Mon Nov 8 03:33:57 2010 +0100

    [fix/evol] conversation/bot protocol rework §8 (refs #30)

diff --git a/bin/mapmaker_client b/bin/mapmaker_client
index a2c6166..8fed10f 100755
--- a/bin/mapmaker_client
+++ b/bin/mapmaker_client
@@ -38,9 +38,10 @@ module CyborgHood
       def start_work
         contact_peer("MapMaker") do |conv|
           if conv
-            logger.info "Yo ! Conversation ready with MapMaker (#{conv.identifier}) !"
+            logger.info "Yo ! Conversation ready with MapMaker (#{conv.peer_name}) !"
           else
             logger.error "Could not connect to MapMaker"
+            stop
           end
         end
       end
diff --git a/lib/cyborghood/cyborg.rb b/lib/cyborghood/cyborg.rb
index 8619c58..c9d4855 100644
--- a/lib/cyborghood/cyborg.rb
+++ b/lib/cyborghood/cyborg.rb
@@ -25,9 +25,12 @@ module CyborgHood
     include I18nTranslation
     bindtextdomain("cyborghood", {:path => Config::L10N_DIR, :charset => "UTF-8"})
 
+    attr_reader :name
+
     def initialize
+      @name = self.class.name.split("::").last
+
       @config = Config.instance
-      @config.bot_id = self.class.name
 
       # setup logs
       unless @config.log.nil?
@@ -55,6 +58,8 @@ module CyborgHood
       EventMachine.run do
         start_work
       end
+    rescue EventMachine::ConnectionNotBound
+      logger.fatal "Internal error (EventMachine::ConnectionNotBound)"
     end
 
     def start_work
@@ -63,6 +68,11 @@ module CyborgHood
 
     def ask_to_stop
       logger.info "Bot was asked to stop..."
+      stop
+    end
+
+    def stop
+      logger.info "Bot stopping"
       EventMachine.stop
     end
 
diff --git a/lib/cyborghood/cyborg/botnet.rb b/lib/cyborghood/cyborg/botnet.rb
index 6be0b56..778b71a 100644
--- a/lib/cyborghood/cyborg/botnet.rb
+++ b/lib/cyborghood/cyborg/botnet.rb
@@ -28,6 +28,10 @@ module CyborgHood
   end
 
   module BotNetClientUNIXSocket
+    def identifier_prefix
+      "unix_socket"
+    end
+
     def peer_socket(peer)
       File.join(Config::RUN_DIR, peer.downcase + ".sock")
     end
@@ -37,7 +41,7 @@ module CyborgHood
         block.call @comm_list[peer]
       else
         begin
-          EventMachine.connect_unix_domain(peer_socket(peer), Conversation, self, &block)
+          EventMachine.connect_unix_domain(peer_socket(peer), Conversation, self, block)
         rescue
           block.call false
         end
diff --git a/lib/cyborghood/cyborg/botnet_server.rb b/lib/cyborghood/cyborg/botnet_server.rb
index d82b8a6..6d4a0ab 100644
--- a/lib/cyborghood/cyborg/botnet_server.rb
+++ b/lib/cyborghood/cyborg/botnet_server.rb
@@ -40,11 +40,6 @@ module CyborgHood
     def capabilities
       super + []
     end
-
-    # TODO: should be common to the future BotClientUNIXSocket
-    def identifier_prefix
-      "unix_socket"
-    end
   end
 
   module BotNetServer
diff --git a/lib/cyborghood/cyborg/conversation.rb b/lib/cyborghood/cyborg/conversation.rb
index e603918..424e323 100644
--- a/lib/cyborghood/cyborg/conversation.rb
+++ b/lib/cyborghood/cyborg/conversation.rb
@@ -35,7 +35,7 @@ module CyborgHood
     end
 
     def new_message(action_code, parameters = nil, action_id = nil)
-      Message.new(self, action_code, parameters)
+      Message.new(self, action_code, parameters, action_id)
     end
 
     def next_action_id
@@ -50,49 +50,51 @@ module CyborgHood
   end
 
   class Message
-    private_class_method :new
-
-    attr_reader :conv_thread, :action_code, :action_parameters, :action_id
+    attr_reader :conv_thread, :action_code, :action_parameters, :action_id, :reply_to_action_id
 
     def initialize(conv_thread, action_code, action_parameters = nil, action_id = nil)
       @conv_thread = conv_thread
       @action_code = action_code
       @action_parameters = action_parameters
-      @action_id = action_id
+      @reply_to_action_id = action_id
+
+      @action_id = nil
     end
 
     def new?
+      @reply_to_action_id.nil?
+    end
+
+    def sent?
       @action_id.nil?
     end
 
     def send
-      raise CyberError.new(:unrecoverable, "bot/conversation", "Not sending twice the same message") unless self.new?
+      raise CyberError.new(:unrecoverable, "bot/conversation", "Not sending twice the same message") unless self.sent?
       @action_id = @conv_thread.next_action_id
       @conv_thread.conversation.send_message(self)
     end
 
-    def create_reply(action_code, parameters)
+    def create_reply(action_code, parameters = nil)
       raise CyberError.new(:unrecoverable, "bot/conversation", "Cannot reply to a newly created message") if self.new?
-      new(self, action_code, parameters, @action_id)
+      self.class.new(@conv_thread, action_code, parameters, @action_id)
     end
   end
 
   class Conversation < EventMachine::Protocols::LineAndTextProtocol
-    private_class_method :new
-
     # don't rely on EventMachine's default, it may change one day
     MaxLineLength = 16*1024
 
     EOD = "\033[0J"
-    BOT_ID_PATTERN = "[a-zA-Z0-9]+"
+    BOT_NAME_PATTERN = "[A-Z][a-zA-Z0-9]+"
     ACTION_WORD_PATTERN = "[a-zA-Z0-9]+"
-    ACTION_PATTERN = "^(#{BOT_ID_PATTERN})(\d{4})-(\d{4})([+]?) (#{ACTION_WORD_PATTERN}( #{ACTION_WORD_PATTERN})*)$"
+    ACTION_PATTERN = "^(#{BOT_NAME_PATTERN})-([0-9]{4})-([0-9]{4})([+]?) (#{ACTION_WORD_PATTERN}( #{ACTION_WORD_PATTERN})*)$"
     MAXIMUM_ERROR_COUNT = 3
     MAXIMUM_LINES = 1024
 
-    attr_reader :bot
+    attr_reader :bot, :peer_name, :peer_capabilities
 
-    def initialize(bot, &block)
+    def initialize(bot, block = nil)
       @bot = bot
       @comm_logic_block = block
 
@@ -100,30 +102,37 @@ module CyborgHood
 
       @config = Config.instance
 
-      @receive_error_count = 0
+      @error_count = 0
       @split_data_mode = false
       @split_data_message = nil
       @split_data = []
 
       # associated conversation threads
       @conv_threads = {}
+      @conv_threads_index = {}
       # thread 0 is reserved
       @next_thread_id = 0
       @system_thread = self.thread('system')
 
       # post-negociation peer info
-      @peer_id = nil
+      @peer_name = nil
       @peer_capabilities = []
 
+      @comm_stop = false
+
       @protocol = BotProtocol.new(self)
     end
 
     def post_init
-      logger.debug "New conversation with #{identifier}"
+      logger.info "New conversation with #{identifier}"
+      @protocol.send_announce_helo unless @comm_logic_block.nil?
+    rescue
+      logger.error "Conversation post_init: " + $!
+      connection_close
     end
 
     def receive_line(data)
-      return if data.empty?
+      return if @comm_stop or data.empty?
 
       clear_receive_info
 
@@ -143,10 +152,12 @@ module CyborgHood
           logger.debug "Received data [#{identifier}]: #{data}"
 
           if data =~ Regexp.new(ACTION_PATTERN)
-            conv_thread_id = $1
-            action_id = $2
-            flags = $3 || ""
-            action_code = $4
+            # TODO: pass peer_name into Message and use it in Protocol to check replies match
+            peer_name = $1
+            conv_thread_id = $2.to_i
+            action_id = $3.to_i
+            flags = $4 || ""
+            action_code = $5
 
             conv_thread = self.thread_by_id(conv_thread_id)
             message = conv_thread.new_message(action_code, nil, action_id)
@@ -162,13 +173,11 @@ module CyborgHood
         end
       end
 
-      send_reply
-
       check_errors
     end
 
     def receive_message(message)
-      logger.debug "Received message '#{action}' [#{identifier}]"
+      logger.debug "Received message '#{message.action_code}' [#{identifier}]"
 
       @protocol.process_received_message(message)
     end
@@ -179,23 +188,33 @@ module CyborgHood
     end
 
     def unbind
-      logger.debug "Conversation finished with #{identifier}"
-      @bot.unregister_communication @peer_id unless @peer_id.nil?
-      @conv_threads.each_values {|s| s.close }
+      logger.info "Conversation finished with #{identifier} (#{@peer_name})"
+      @bot.unregister_communication @peer_name unless @peer_name.nil?
+      @conv_threads.each_value {|s| s.close }
+      @comm_logic_block.call false unless @comm_logic_block.nil? or @protocol.negociation_ok?
     end
 
     def bye
-      send_quit_leaving
+      @protocol.send_quit_leaving
     end
 
-    def set_peer_info(id, capabilities)
-      @peer_id = id
+    def set_peer_info(name, capabilities)
+      @peer_name = name
       @peer_capabilities = capabilities || []
-      @bot.register_communication @peer_id, self
+      logger.info "Peer name for #{identifier}: #{@peer_name}"
+      logger.info "Peer capabilities for #{identifier}: #{@peer_capabilities.join(", ")}"
+      @bot.register_communication @peer_name, self
     end
 
     def set_comm_ready
-      @comm_logic_block.call self
+      logger.info "Protocol negociation with '#{@peer_name}' on #{identifier} succeeded"
+      @comm_logic_block.call self unless @comm_logic_block.nil?
+    end
+
+    def set_comm_stop
+      @comm_stop = true
+      yield if block_given?
+      close_connection_after_writing
     end
 
     def set_error_status(fatal = false)
@@ -216,13 +235,14 @@ module CyborgHood
     def send_message(message)
       raise CyberError.new(:unrecoverable, "bot/conversation", "Cannot send message without action id") if message.action_id.nil?
 
-      send_line "#{@config.bot_id}-#{message.conv_thread.id}-#{message.action_id}" + (message.action_parameters.nil? ? "" : "+") + " #{message.action_code}"
+      flags = ""
+      flags += "+" unless message.action_parameters.nil?
+
+      send_line sprintf("%s-%04d-%04d%s %s", @bot.name, message.conv_thread.id, message.action_id, flags, message.action_code)
       unless message.action_parameters.nil?
-        message.action_parameters.to_yaml.split.each {|l| send_line l }
+        message.action_parameters.to_yaml.each_line {|l| send_line l }
         send_line EOD
       end
-
-      action_id
     end
 
     protected
@@ -249,11 +269,7 @@ module CyborgHood
         msg_quit = "previous fatal error"
       end
 
-      unless msg_quit.nil?
-        send_quit_decline msg_quit
-        @comm_logic_block.call false
-        close_connection_after_writing
-      end
+      @protocol.send_quit_decline msg_quit unless msg_quit.nil?
     end
 
     def new_thread(name, id = nil)
@@ -298,10 +314,10 @@ module CyborgHood
       if @split_data_mode
         logger.debug "Protocol info [#{identifier}]: quit split mode for action '#{@split_data_message.action_id}'"
 
-        parameters = YAML.parse(@split_data.join("\n"))
+        parameters = YAML.load(@split_data.join("\n"))
         reply_syntax_error("bad parameters format") if parameters.nil?
 
-        message = @split_data_message.conv_thread.new_message(@split_data_message.action_code, parameters, @split_data_message.action_id)
+        message = @split_data_message.conv_thread.new_message(@split_data_message.action_code, parameters, @split_data_message.reply_to_action_id)
         receive_message(message)
       else
         reply_fatal_error "not in split mode"
diff --git a/lib/cyborghood/cyborg/protocol.rb b/lib/cyborghood/cyborg/protocol.rb
index b4d010c..44dee36 100644
--- a/lib/cyborghood/cyborg/protocol.rb
+++ b/lib/cyborghood/cyborg/protocol.rb
@@ -21,7 +21,7 @@
 
 
 module CyborgHood
-  module BotProtocol
+  class BotProtocol
     VERSION = "0.1"
     CAPABILITIES = []
 
@@ -63,14 +63,17 @@ module CyborgHood
       unless message.conv_thread.id == 0
         return send_quit_decline "bad negociation"
       end
-      unless message.parameters[:bot_name] =~ Conversation::BOT_ID_PATTERN
+      if message.action_parameters.nil?
+        return send_quit_decline "missing parameters"
+      end
+      unless message.action_parameters[:bot_name] =~ Regexp.new(Conversation::BOT_NAME_PATTERN)
         return send_quit_decline "bad bot name"
       end
-      unless message.parameters[:protocol_version] == VERSION
+      unless message.action_parameters[:protocol_version] == VERSION
         return send_quit_decline "protocol version does not match"
       end
       @negociation_received = true
-      @conversation.set_peer_info(message.parameters[:bot_name], message.parameters[:capabilities])
+      @conversation.set_peer_info(message.action_parameters[:bot_name], message.action_parameters[:capabilities])
 
       if @negociation_sent
         send_announce_ok(message)
@@ -78,7 +81,6 @@ module CyborgHood
         @conversation.set_comm_ready
       else
         send_announce_helo(message)
-        @negociation_sent = true
       end
     end
 
@@ -94,7 +96,10 @@ module CyborgHood
     end
 
     def receive_request_call(message)
-      unless @conversation.bot.interface.is_node? message.parameters[:node]
+      if message.action_parameters.nil?
+        return send_error_action(message, "missing parameters")
+      end
+      unless @conversation.bot.interface.is_node? message.action_parameters[:node]
         return send_error_action(message, "bad node")
       end
       send_reply_ack(message)
@@ -104,8 +109,8 @@ module CyborgHood
         }
         begin
           result[:action_result] = @conversation.bot.interface.call(message.conv_thread.session,
-                                                                    message.parameters[:node],
-                                                                    message.parameters[:data])
+                                                                    message.action_parameters[:node],
+                                                                    message.action_parameters[:data])
         rescue
           result[:error] = $!
         end
@@ -113,14 +118,17 @@ module CyborgHood
     end
 
     def receive_request_exists(message)
-      unless @conversation.bot.interface.is_node? message.parameters[:node]
+      if message.action_parameters.nil?
+        return send_error_action(message, "missing parameters")
+      end
+      unless @conversation.bot.interface.is_node? message.action_parameters[:node]
         return send_error_action(message, "bad node")
       end
       send_reply_ack(message)
       @conversation.bot.schedule_task(@@request_callback) do
         {
           :reply_message => message,
-          :action_result => @conversation.bot.interface.has_node?(message.parameters[:node])
+          :action_result => @conversation.bot.interface.has_node?(message.action_parameters[:node])
         }
       end
     end
@@ -132,9 +140,15 @@ module CyborgHood
 
     def send_announce_helo(recv_message = nil)
       action_code = "ANNOUNCE HELO"
-      message = (recv_message.nil? ? @conversation.thread('system').new_message(action_code) :
-                 recv_message.create_reply(action_code))
+      action_parameters = {
+        :bot_name         => @conversation.bot.name,
+        :protocol_version => VERSION
+      }
+
+      message = (recv_message.nil? ? @conversation.thread('system').new_message(action_code, action_parameters) :
+                 recv_message.create_reply(action_code, action_parameters))
       message.send
+      @negociation_sent = true
     end
 
     def send_announce_ok(recv_message)
@@ -179,11 +193,15 @@ module CyborgHood
     end
 
     def send_quit_decline(reason)
-      @conversation.thread('system').new_message("QUIT LEAVING", { :reason => reason }).send
+      @conversation.set_comm_stop do
+        @conversation.thread('system').new_message("QUIT DECLINE", { :reason => reason }).send
+      end
     end
 
     def send_quit_leaving
-      @conversation.thread('system').new_message("QUIT LEAVING").send
+      @conversation.set_comm_stop do
+        @conversation.thread('system').new_message("QUIT LEAVING").send
+      end
     end
   end
 end
