Am I reading indy client data properly?

[djjd47130]

I've seen a lot of debate on the subject of an Indy Server sending data to the Client and the Client needing to recognize this data. Indy doesn't necessarily have an event on the client which tells you that data has been received - you need to do this manually. So based on all the advice I've seen, I built myself a listener thread which checks the client for incoming data. Unfortunately, there weren't any good concrete examples, so I'm afraid I might be doing something majorly wrong here, even though it appears to be working.

Here's my thread's class:

type
  TJDRMClientThread = class(TThread)
  private
    FOwner: TIdTCPClient; //Client to listen in on
    FActive: Bool; //Whether thread should be be listening or not
    FBuffer: String; //Master incoming string
    FPackets: TThreadStringList; //Thread protected string list for parsed packets
    FPacket: TJDRMPacket; //Custom class representing a complete packet
    FOnPacket: TJDRMClientPacketEvent; //Event triggered when a new packet is ready
    procedure SetActive(const Value: Bool); //Activate/Deactivate thread
    procedure SYNC_OnPacket; //Synchronized method when packet is ready
  protected
    procedure Execute; override;
  public
    constructor Create(AOwner: TIdTCPClient);
    destructor Destroy; override;
    procedure Start; //Activate thread
    procedure Stop; //Deactivate thread
    property Owner: TIdTCPClient read FOwner; //Client to listen in on
    property Active: Bool read FActive write SetActive; //Whether thread should be listening or not
    property OnPacket: TJDRMClientPacketEvent read FOnPacket write FOnPacket; //Event triggered when a new packet is ready
  end;

The thread is started at the time the TIdTCPClient triggers the OnConnected event, and stopped with OnDisconnected. The execute procedure seems very messy, and I'm contemplating making another thread just for doing the parsing/events, and leave this one to ONLY read the buffer.

Here's the implementation for the above:

constructor TJDRMClientThread.Create(AOwner: TIdTCPClient);
begin
  inherited Create(True); //Create suspended
  FOwner:= AOwner; //Assign client to listen to
  FPackets:= TThreadStringList.Create; //Queue of parsed packets
  FPacket:= TJDRMPacket.Create; //Temporary packet for triggering events
  FActive:= False; //Start inactive
  FBuffer:= ''; //Clear buffer
  Resume; //Start thread
end;

destructor TJDRMClientThread.Destroy;
begin
  Stop; //Stop thread
  FPackets.Free; //Free packet queue
  FPacket.Free; //Free temporary packet
  inherited;
end;

procedure TJDRMClientThread.Execute;
var
  Z: Integer;
  T: String;
  L: TStringList;
  X: Integer;
begin
  //Keep going until free'd
  while not Terminated do begin
    //Is thread active?
    if FActive then begin
      //Is client assigned?
      if Assigned(FOwner) then begin
        //Is something waiting in queue?
        FOwner.IOHandler.CheckForDataOnSource(5000);
        if not FOwner.IOHandler.InputBufferIsEmpty then begin
          T:= FOwner.IOHandler.InputBufferAsString; //Read data
          FBuffer:= FBuffer + T; //Write data to buffer
        end;
        //Do we have any waiting data?
        if Length(FBuffer) > 0 then begin
          //Is a packet size defined?
          if Pos('#', FBuffer) > 0 then begin
            //Identify packet size...
            T:= CopyToDelim(FBuffer, '#', False); //my own version of Copy which stops at a given deliminator
            Z:= StrToIntDef(T, 0); //Convert packet size from string to integer
            if Z > 0 then begin
              //Does the buffer contain at least the given size?
              if Length(FBuffer) >= Z then begin
                //First, delete size definition...
                CopyToDelim(FBuffer, '#');
                //Now grab the rest of it up to the size...
                T:= CopyDel(FBuffer, 1, Z);
                //Add this packet to the queue
                L:= FPackets.Lock;
                try
                  L.Add(T);
                finally
                  FPackets.Unlock;
                end;
              end else begin
                Sleep(1);
              end;
            end else begin
              Sleep(1);
            end;
          end else begin
            Sleep(1);
          end;
        end else begin
          Sleep(1);
        end;
        //Are there any packets in queue?
        L:= FPackets.Lock;
        try
          while L.Count > 0 do begin
            FPacket.AsString:= L[0];
            L.Delete(0);
            Synchronize(SYNC_OnPacket);
          end;
        finally
          FPackets.Unlock;
        end;
      end else begin
        Sleep(1);
      end;
    end else begin
      Sleep(1); //I do this so thread doesn't go crazy when not in use
    end;
  end;
end;

procedure TJDRMClientThread.SetActive(const Value: Bool);
begin
  if Value then Start else Stop;
end;

procedure TJDRMClientThread.Start;
begin
  FActive:= True;
end;

procedure TJDRMClientThread.Stop;
begin
  FActive:= False;
  FBuffer:= '';
end;

procedure TJDRMClientThread.SYNC_OnPacket;
begin
  if assigned(FOnPacket) then
    FOnPacket(Self, FPacket);
end;

JD Solutions