Atozed Forums

Atozed Forums > Indy > Indy General Discussion > Porting old Delphi2007 application using TServerSocket....
Full Version: Porting old Delphi2007 application using TServerSocket....
You're currently viewing a stripped down version of our content. View the full version with proper formatting.
Pages: 1 2 3

rlebeau

07-09-2020, 06:47 PM
(07-08-2020, 07:43 PM)BosseB Wrote: [ -> ]If readln timeouts will it return an empty string to msg?

Yes.  And the ReadLnTimedOut property will be set to True.

(07-08-2020, 07:43 PM)BosseB Wrote: [ -> ]I.e. it will leave the data in the buffer until the line ending char arrives?

Yes.

(07-08-2020, 07:43 PM)BosseB Wrote: [ -> ]I don't want to have partial packets to deal with...

Unless you have other work to do, then just don't use a timeout at all, let ReadLn() wait until the ETX arrives.

(07-08-2020, 07:43 PM)BosseB Wrote: [ -> ]
Code:
msg := AContext.Connection.IOHandler.ReadLn(ETX, 10); //Timeout 10 ms

10ms is a very short timeout to use.

(07-08-2020, 07:43 PM)BosseB Wrote: [ -> ]I guess it must work that way because otherwise one can never know if a partial packet is present when processing it (the ETX char is "eaten" by ReadLn)...

Alternatively, you can use WaitFor() instead, which has an AInclusive parameter which you can set to True to include the terminator in the output.  However, WaitFor() will raise an exception on a timeout, rather than return an empty string.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]I can run the server on both platforms and connect to each by changing the host value, and when connecting to the Linux server the response is immediate. The login dialog appears instantly, whereas if I connect to the Windows based server there is a delay of maybe 5 seconds before that happens.

Then the delay is most likely outside of the server's code. I just could not tell you where exactly.

BosseB

08-28-2020, 08:53 PM
I managed to get the server working with your help! Thanks for that!

Now I have a new task and this is to port the client application to FreePascal/Lazarus from Delphi.
Again I need to replace an old socket component with an Indy10 implementation, i.e. I need to create a receive event for TIdTcpClient...

In this case I have found a number of examples which for some reason do not work (Lazarus complains) which use a thread to read incoming data and then use an event function to feed the main application.
So I decided to make a class that handles this with Indy10 and implements the functions used by the earlier solution.
This uses a read thread as described in several old forum posts...
Below is how far I have come but I have gotten stuck concerning how to best retrieve the data in the thread Execute and supply it to the client via the event procedure.

Questions:
What should I put into the thread execute function on data reception to transfer it into the event?
Should I read the data in the thread or should I just push a notification that data are available and let the implementation of the event procedure read from the socket?

Here is my current (incomplete) state of the code:
Code:
unit TcpClientComm;
{******************************************************************************
* This is an attempt to use Indy10 TIdTcpClient object as a TCP/IP connection
* while providing an event driven model for receiving data from the server.
*
*
*******************************************************************************}

{$IFDEF FPC}
  {$MODE Delphi}
{$ENDIF}

interface

uses
  Classes,
  SysUtils,
  IdBaseComponent,
  IdComponent,
  IdTCPConnection,
  IdTCPClient,
  IdGlobal
  ;

type
  TCommRxEvent = procedure (Sender: TObject; Data: string) of object;

  { TReadingThread }

  TReadingThread = class(TThread)
  protected
  FConn: TIdTCPConnection;
    FOnRxData: TCommRxEvent;
  procedure Execute; override;
  public
  constructor Create(ACon: TIdTCPConnection);
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
  end;

  { TTcpClientComm }

  TTcpClientComm = class(TObject)
  private
    FConn: TIdTCPClient;
    FReadThread: TReadingThread;
    FOnRxData: TCommRxEvent;
    FLastError: string;
    function GetConnected: boolean;
  public
    constructor Create;
    destructor  Destroy; override;
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
    property LastError: string read FLastError;
    property Connected: boolean read GetConnected;
    procedure Connect(Host: string; Port: word);
    procedure Disconnect;
    function  WriteString(Data: string): boolean; overload;
    function  WriteData(Data: TIdBytes): boolean; overload;
  end;

implementation

{ TTcpClientComm }

function TTcpClientComm.GetConnected: boolean;
begin
  Result := FConn.Connected;
end;

constructor TTcpClientComm.Create;
begin
  FConn := TIdTCPClient.Create(NIL);
  FReadThread := TReadingThread.Create(FConn);
end;

destructor TTcpClientComm.Destroy;
begin
  inherited Destroy;
end;

procedure TTcpClientComm.Connect(Host: string; Port: word);
begin
  FConn.Connect(Host, Port);
end;

procedure TTcpClientComm.Disconnect;
begin
  FConn.Disconnect;
end;

function TTcpClientComm.WriteString(Data: string): boolean;
var
  DataBytes: TIdBytes;
begin
  Result := false;
  FLastError := 'No data';
  if Length(Data) = 0 then exit;

  SetLength(DataBytes, Length(Data));
  Move(Data[1], DataBytes[0], Length(Data));
  Result := WriteData(DataBytes);
end;

function TTcpClientComm.WriteData(Data: TIdBytes): boolean;
begin
  Result := false;
  FLastError := 'No data';
  if Length(Data) = 0 then exit;

  if not FConn.Connected then
  begin
    FLastError := 'Not connected';
    exit;
  end;

  try
    FConn.IOHandler.Write(Data);
    Result := true;
  except
    on E: Exception do
      FLastError := E.Message;
  end;
end;

{ TReadingThread }

procedure TReadingThread.Execute;
begin
while not Terminated do
begin
  if FConn.IOHandler.CheckForDataOnSource(10) then
    if FConn.IOHandler.InputBuffer.Size > 0 then
    begin
      // read bytes from FConn up
      // to InputBuffer.Size bytes ...
      //
      // process bytes as needed ...
    end;
  end;
end;
constructor TReadingThread.Create(ACon: TIdTCPConnection);
begin
  FConn := ACon;
  inherited Create(False);
end;

rlebeau

08-28-2020, 10:51 PM
(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]In this case I have found a number of examples which for some reason do not work (Lazarus complains)

In what way exactly does it complain?

(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]which use a thread to read incoming data and then use an event function to feed the main application.

That is generally the way to do it.

(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]What should I put into the thread execute function on data reception to transfer it into the event?
Should I read the data in the thread or should I just push a notification that data are available and let the implementation of the event procedure read from the socket?

That is more of a design issue than a technical issue.  Use whichever design suits your needs.

I will say, though, that you should not create the reading thread until after the TIdTCPClient has been connected first.  And then terminate and free the thread when the TIdTCPClient is being disconnected.

(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]Here is my current (incomplete) state of the code:

If you are going to have the event pass a string, then the reading thread should handle all of the necessary reading and partial-message buffering and such, and pass only completed messages to the event.  Also, this would only work for text-based protocols.

Otherwise, I would suggest having the reading thread just read whatever raw bytes are on the socket at a time and pass the bytes as-is to the event, and let the event handler decide what to do with the bytes as needed.

(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]
Code:
SetLength(DataBytes, Length(Data));
Move(Data[1], DataBytes[0], Length(Data));

FYI, Indy has RawToBytes() and ToBytes(string) functions in the IdGlobal unit for that purpose, eg:

Code:
function TTcpClientComm.WriteString(Data: string): boolean;
begin
  Result := WriteData(ToBytes(Data));
end;

Of course, TIdIOHanlder.Write() has separate overloads for sending a string vs a TIdBytes.

BosseB

08-29-2020, 06:20 AM
(08-28-2020, 10:51 PM)rlebeau Wrote: [ -> ]
(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]In this case I have found a number of examples which for some reason do not work (Lazarus complains)

In what way exactly does it complain?

Cannot find identifiers etc...

But it seems like most examples are for earlier Indy versions than Indy10.

I managed to find these cases and fix them.


Quote:
(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]What should I put into the thread execute function on data reception to transfer it into the event?
Should I read the data in the thread or should I just push a notification that data are available and let the implementation of the event procedure read from the socket?

That is more of a design issue than a technical issue.  Use whichever design suits your needs.

I have tried to use an event function that contains the data but then I got an error from Lazarus.

With this in Execute it fails to compile:



Code:
type
  TCommRxEvent = procedure (Sender: TObject; Data: string) of object; // <= Is this a correct declaration?
...

  { TReadingThread }

  TReadingThread = class(TThread)
  protected
      FConn: TIdTCPConnection;
    FOnRxData: TCommRxEvent;
    FRxData: string;
      procedure Execute; override;
  public
      constructor Create(ACon: TIdTCPConnection);
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
  end;

...
procedure TReadingThread.Execute;
begin
  while not Terminated do
  begin
    if FConn.Connected then
    begin
      if FConn.IOHandler.CheckForDataOnSource(10) then
      begin
        if FConn.IOHandler.InputBuffer.Size > 0 then
        begin
          // read bytes from FConn up to InputBuffer.Size bytes ...
          FRxData := FConn.IOHandler.ReadString(FConn.IOHandler.InputBuffer.Size);
          //
          // process bytes as needed ...
          if Assigned(FOnRxData) then
            Synchronize(FOnRxData(Self, FRxData)); // <= Error here!
        end;
      end;
    end;
  end;
end;

The error message says:


Code:
tcpclientcomm.pas(141,47) Error: Incompatible type for arg no. 1: Got "untyped", expected "<procedure variable type of procedure of object;Register>"
And it does not mean much to me...



Quote:I will say, though, that you should not create the reading thread until after the TIdTCPClient has been connected first.  And then terminate and free the thread when the TIdTCPClient is being disconnected.

How can I do that, i.e. how do I know the connect and disconnect happened?


Quote:
(08-28-2020, 08:53 PM)BosseB Wrote: [ -> ]Here is my current (incomplete) state of the code:

If you are going to have the event pass a string, then the reading thread should handle all of the necessary reading and partial-message buffering and such, and pass only completed messages to the event.  Also, this would only work for text-based protocols.

Otherwise, I would suggest having the reading thread just read whatever raw bytes are on the socket at a time and pass the bytes as-is to the event, and let the event handler decide what to do with the bytes as needed.

Do you mean passing the data out in the event handler in a TIdBytes container?

Or should the event not contain the data and the reading be done by the handler itself?

I.e. the event is just a Synchronized notification and the socket read is done in the event implementation instead?


Seems like it would be better keep that from the caller and just provide the received data itself, that is at least what I thought a better encapsulation.

But then again, I have not managed to implement it correctly...

rlebeau

08-31-2020, 07:23 PM
(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]I have tried to use an event function that contains the data but then I got an error from Lazarus.

That is because you are not using TThread.Synchronize() correctly.  It needs to look like this instead:

Code:
type
  TCommRxEvent = procedure (Sender: TObject; Data: string) of object;

  { TReadingThread }

  TReadingThread = class(TThread)
  protected
    FConn: TIdTCPConnection;
    FOnRxData: TCommRxEvent;
    FRxData: string;
    procedure Execute; override;
    procedure DoOnRxData;
  public
    constructor Create(ACon: TIdTCPConnection);
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
  end;

...
procedure TReadingThread.Execute;
begin
  while not Terminated do
  begin
    if FConn.Connected then
    begin
      if FConn.IOHandler.CheckForDataOnSource(10) then
      begin
        if FConn.IOHandler.InputBuffer.Size > 0 then
        begin
          // read bytes from FConn up to InputBuffer.Size bytes ...
          FRxData := FConn.IOHandler.ReadString(FConn.IOHandler.InputBuffer.Size);
          //
          // process bytes as needed ...
          if Assigned(FOnRxData) then
            Synchronize(DoOnRxData);
        end;
      end;
    end;
  end;
end;

procedure TReadingThread.DoOnRxData;
begin
  if Assigned(FOnRxData) then
    FOnRxData(Self, FRxData);
end;

In Delphi ,you could do the following instead, but FreePasscal does not support this approach yet:

Code:
type
  TCommRxEvent = procedure (Sender: TObject; Data: string) of object;

  { TReadingThread }

  TReadingThread = class(TThread)
  protected
    FConn: TIdTCPConnection;
    FOnRxData: TCommRxEvent;
    FRxData: string;
    procedure Execute; override;
  public
    constructor Create(ACon: TIdTCPConnection);
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
  end;

...
procedure TReadingThread.Execute;
begin
  while not Terminated do
  begin
    if FConn.Connected then
    begin
      if FConn.IOHandler.CheckForDataOnSource(10) then
      begin
        if FConn.IOHandler.InputBuffer.Size > 0 then
        begin
          // read bytes from FConn up to InputBuffer.Size bytes ...
          FRxData := FConn.IOHandler.ReadString(FConn.IOHandler.InputBuffer.Size);
          //
          // process bytes as needed ...
          if Assigned(FOnRxData) then
          begin
            Synchronize(
              procedure
              begin
                if Assigned(FOnRxData) then
                  FOnRxData(Self, FRxData);
              end
            );
          end;
        end;
      end;
    end;
  end;
end;

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]
Quote:I will say, though, that you should not create the reading thread until after the TIdTCPClient has been connected first.  And then terminate and free the thread when the TIdTCPClient is being disconnected.

How can I do that, i.e. how do I know the connect and disconnect happened?

You are the one calling Connect() and Disconnect().  So, simply don't create the thread until you have called Connect() and it has exited without raising an exception.  Terminate() the thread before calling Disconnect(), and then finish freeing the thread after Disconnect() exits.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]Do you mean passing the data out in the event handler in a TIdBytes container?

Yes.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]Or should the event not contain the data and the reading be done by the handler itself?

No, the reading should be done before the event handler is called.  Let the thread do its job.  HOW the thread reads, and WHAT it passes to the event, depend on your particular needs.  For instance, given the STX/ETX example you provided earlier, the thread could wait for STX to arrive, then read until ETX arrives, and then pass everything in between to the event.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]I.e. the event is just a Synchronized notification and the socket read is done in the event implementation instead?

No.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]Seems like it would be better keep that from the caller and just provide the received data itself, that is at least what I thought a better encapsulation.

Yes.

BosseB

09-18-2020, 10:00 AM
Coming back to this after fixing GUI component problems in porting to FPC/Lazarus on Linux...

A few additional questions:
1. Data container
As you pointed out the old code uses string as the container even though the data is not necessarily textual.
So if I were to change that to use TIdBytes instead, what would the read function in the thread look like?
Now with FRxData declared as string:

Code:
FRxData: string;
...
FRxData := FConn.IOHandler.ReadString(FConn.IOHandler.InputBuffer.Size);

If I instead use TIdBytes is the read function going to look like this?
Code:
FRxData: TIdBytes;
...
FConn.IOHandler.ReadBytes(FRxData, FConn.IOHandler.InputBuffer.Size, false);
Do I have to set the size of FRxData before calling the read or is that handled inside the read?

2. Sending the data via the event
Is the synchronized call to the event handler getting a copy of FRxData or FRxData itself?
Does it have to set back the length of the container to zero after processing the data?

Right now I have the code at the bottom, which compiles but the application itself does not because I have not yet gotten rid of the TClientSocket stuff coming from the Delphi7 implementation...

3. How to handle the socket messages?
I am looking for how to handle the various events existing in the old code coming from TClientSocket and in need of an Indy implementation..

This is the section of code I ave problem with there. As you can see I have as yet just copied the Delphi code into a section inside a conditional.
Code:
    {$IFDEF FPC}
      procedure OnClientError(Sender: TObject; Socket: TIdTCPClient; ErrorEvent: TErrorEvent; var ErrorCode: Integer);
      procedure OnSocketConnect(Sender: TObject; Socket: TIdTCPClient);
      procedure OnSocketDisconnect(Sender: TObject; Socket: TIdTCPClient);
      procedure OnSocketRead(Sender: TObject; Socket: TIdTCPClient);
    {$ELSE}
      procedure OnClientError(Sender: TObject; Socket: TCustomWinSocket; ErrorEvent: TErrorEvent; var ErrorCode: Integer);
      procedure OnSocketConnect(Sender: TObject; Socket: TCustomWinSocket);
      procedure OnSocketDisconnect(Sender: TObject; Socket: TCustomWinSocket);
      procedure OnSocketRead(Sender: TObject; Socket: TCustomWinSocket);
    {$ENDIF}
The compiler stops at the first line complaining about the missing identifier:
Code:
class_SSRemoteClient.pas(110,82) Error: Identifier not found "TErrorEvent"

I have looked at the documentation and found that there is an event OnStatus that gets fired for any status change, so maybe that could replace all 4 of the above separate events?
Something like this:

Code:
procedure TTcpClientComm.OnStatusChange(ASender: TObject; const AStatus: TIdStatus; const AStatusText: string);
{This procedure parses the socket status change messages, what to do in the respective cases?}
begin
  case AStatus of
    hsResolving: ;
    hsConnecting: ;
    hsConnected: ;  //Create the read thread here?
    hsDisconnecting: ;
    hsDisconnected: ;  //Terminate the read thread here?
    hsStatusText: ;  //What is this accomplishing?
  end;
end;

constructor TTcpClientComm.Create;
begin
  FConn := TIdTCPClient.Create(NIL);
  FConn.OnStatus := OnStatusChange;
end;

However, I don't really know if this is the correct place.....
There seems also to be other events available when looking in Lazarus code completion, here are all I found:
Code:
OnAfterBind
OnBeforeBind
OnConnected
OnDisconnected
OnSocketAllocated
OnStatus
OnWork
OnWorkBegin
OnWorkEnd
Which one should I use for example in order to create and terminate the read thread?
Notice that I know when the socket is commanded to connect but not necessarily when it is disconnected since that can be commanded from the client side.


My current implementation of the event driven data reception class using TIdTcpClient shown below.
Does it look sensible now, especially how the thread is created and destroyed in connect/disconnect?

Code:
{$IFDEF FPC}
  {$MODE Delphi}
{$ENDIF}

interface

uses
  Classes,
  SysUtils,
  IdBaseComponent,
  IdComponent,
  IdTCPConnection,
  IdTCPClient,
  IdGlobal
  ;

type
  TCommRxEvent = procedure (Sender: TObject; Data: TIdBytes) of object;

  { TReadingThread }

  TReadingThread = class(TThread)
  protected
    FConn: TIdTCPConnection;
    FOnRxData: TCommRxEvent;
    FRxData: TIdBytes;
    procedure Execute; override;
    procedure DoOnRxData;
  public
    constructor Create(ACon: TIdTCPConnection);
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
  end;

  { TTcpClientComm }

  TTcpClientComm = class(TObject)
  private
    FConn: TIdTCPClient;
    FReadThread: TReadingThread;
    FOnRxData: TCommRxEvent;
    FLastError: string;
    function GetConnected: boolean;
    procedure OnStatusChange(ASender: TObject; const AStatus: TIdStatus; const AStatusText: string);
  public
    constructor Create;
    destructor  Destroy; override;
    property OnRxData: TCommRxEvent read FOnRxData write FOnRxData;
    property LastError: string read FLastError;
    property Connected: boolean read GetConnected;
    procedure Connect(Host: string; Port: word);
    procedure Disconnect;
    function  WriteString(Data: string): boolean; overload;
    function  WriteData(Data: TIdBytes): boolean; overload;
  end;

implementation

{ TTcpClientComm }


function TTcpClientComm.GetConnected: boolean;
begin
  Result := FConn.Connected;
end;

procedure TTcpClientComm.OnStatusChange(ASender: TObject; const AStatus: TIdStatus; const AStatusText: string);
{This procedure parses the socket status change messages, what to do in the respective cases?}
begin
  case AStatus of
    hsResolving: ;
    hsConnecting: ;
    hsConnected: ;
    hsDisconnecting: ;
    hsDisconnected: ;
    hsStatusText: ;
  end;
end;

constructor TTcpClientComm.Create;
begin
  FConn := TIdTCPClient.Create(NIL);
  FConn.OnStatus := OnStatusChange;
end;

destructor TTcpClientComm.Destroy;
begin
  if Connected then
    Disconnect;
  inherited Destroy;
end;

procedure TTcpClientComm.Connect(Host: string; Port: word);
begin
  FConn.Connect(Host, Port);
  FReadThread := TReadingThread.Create(FConn);
end;

procedure TTcpClientComm.Disconnect;
begin
  FReadThread.Terminate;
  FConn.Disconnect;
end;

function TTcpClientComm.WriteString(Data: string): boolean;
var
  DataBytes: TIdBytes;
begin
  Result := false;
  if Length(Data) = 0 then
  begin
    FLastError := 'No data';
    exit;
  end;
  SetLength(DataBytes, Length(Data));
  Move(Data[1], DataBytes[0], Length(Data));
  Result := WriteData(DataBytes);
end;

function TTcpClientComm.WriteData(Data: TIdBytes): boolean;
begin
  Result := false;
  if Length(Data) = 0 then
  begin
    FLastError := 'No data';
    exit;
  end;

  if not FConn.Connected then
  begin
    FLastError := 'Not connected';
    exit;
  end;

  FLastError := '';
  try
    FConn.IOHandler.Write(Data);
    Result := true;
  except
    on E: Exception do
      FLastError := E.Message;
  end;
end;

{ TReadingThread }

procedure TReadingThread.Execute;
begin
  while not Terminated do
  begin
    if FConn.Connected then
    begin
      if FConn.IOHandler.CheckForDataOnSource(10) then
      begin
        if FConn.IOHandler.InputBuffer.Size > 0 then
        begin
          // read bytes from FConn up to InputBuffer.Size bytes ...
          FConn.IOHandler.ReadBytes(FRxData, FConn.IOHandler.InputBuffer.Size, true);
          // process bytes as needed ...
          if Assigned(FOnRxData) then
            Synchronize(DoOnRxData);
        end;
      end;
    end;
  end;
end;

procedure TReadingThread.DoOnRxData;
begin
  if Assigned(FOnRxData) then
    FOnRxData(Self, FRxData);
end;

constructor TReadingThread.Create(ACon: TIdTCPConnection);
begin
  FConn := ACon;
  inherited Create(False);
end;

end.

rlebeau

09-18-2020, 07:29 PM
(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]As you pointed out the old code uses string as the container even though the data is not necessarily textual.
So if I were to change that to use TIdBytes instead, what would the read function in the thread look like?
Now with FRxData declared as string:

Code:
FRxData: string;
...
FRxData := FConn.IOHandler.ReadString(FConn.IOHandler.InputBuffer.Size);

If I instead use TIdBytes is the read function going to look like this?
Code:
FRxData: TIdBytes;
...
FConn.IOHandler.ReadBytes(FRxData, FConn.IOHandler.InputBuffer.Size, false);

Yes, or simpler:

Code:
FConn.IOHandler.ReadBytes(FRxData, -1, false);

Which will read everything that is currently available at that moment.

Though, in an STX/ETX scenario, I would suggest something more like this instead (as there is currently no TIdBytes version of TIdIOHandler.WaitFor() available):

Code:
LPos := 0;
repeat
  LPos := FConn.IOHandler.InputBuffer.IndexOf(STX, LPos);
  if LPos <> -1 then Break;
  LPos := FConn.IOHandler.InputBuffer.Size;
  FConn.IOHandler.ReadFromSource(True, FConn.IOHandler.ReadTimeout, True);
until False;

FConn.IOHandler.InputBuffer.Remove(LPos+1);

LPos := 0;
repeat
  LPos := FConn.IOHandler.InputBuffer.IndexOf(ETX, LPos);
  if LPos <> -1 then Break;
  LPos := FConn.IOHandler.InputBuffer.Size;
  FConn.IOHandler.ReadFromSource(True, FConn.IOHandler.ReadTimeout, True);
until False;

FConn.IOHandler.ReadBytes(FRxData, LPos-1, false);
FConn.IOHandler.InputBuffer.Remove(1);

// use FRxData as needed...

Or, if you need the STX and ETX bytes to be in the FRxData buffer:

Code:
LPos := 0;
repeat
  LPos := FConn.IOHandler.InputBuffer.IndexOf(STX, LPos);
  if LPos <> -1 then Break;
  LPos := FConn.IOHandler.InputBuffer.Size;
  FConn.IOHandler.ReadFromSource(True, FConn.IOHandler.ReadTimeout, True);
until False;

if LPos > 0 then
  FConn.IOHandler.InputBuffer.Remove(LPos);

LPos := 1;
repeat
  LPos := FConn.IOHandler.InputBuffer.IndexOf(ETX, LPos);
  if LPos <> -1 then Break;
  LPos := FConn.IOHandler.InputBuffer.Size;
  FConn.IOHandler.ReadFromSource(True, FConn.IOHandler.ReadTimeout, True);
until False;

FConn.IOHandler.ReadBytes(FRxData, LPos+1, false);

// use FRxData as needed...

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]Do I have to set the size of FRxData before calling the read or is that handled inside the read?

ReadBytes() will resize the TIdBytes for you.  If the AAppend parameter is True, the read bytes are appended to the end of the TIdBytes' current allocation.  If the AAppend parameter is False, the TIdBytes is set to just the read bytes by themselves.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]Is the synchronized call to the event handler getting a copy of FRxData or FRxData itself?

FRxData itself, since dynamic arrays are reference counted, same as with strings.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]Does it have to set back the length of the container to zero after processing the data?

If you set the AAppend parameter of ReadByes() to True, then yes.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]This is the section of code I ave problem with there.

Indy clients are not event driven, so there are no such events available, so just get rid of all those handlers altogether.

TIdTCPClient.Connect() is synchronous. It will raise an exception in the calling thread if it fails.

If a read/write IO operation fails, including due to a graceful disconnect by the peer, it will raise an exception in the calling thread.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]The compiler stops at the first line complaining about the missing identifier:
Code:
class_SSRemoteClient.pas(110,82) Error: Identifier not found "TErrorEvent"

As it shuld be, since that enum is specific to the old Socket components.  There is no equivalent in Indy.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]I have looked at the documentation and found that there is an event OnStatus that gets fired for any status change, so maybe that could replace all 4 of the above separate events?

Not all 4, no.  You could use it for OnConnect and OnDisconnect, but there are status events for pending data waiting to be read, or for errors.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]There seems also to be other events available when looking in Lazarus code completion, here are all I found:
...
Which one should I use for example in order to create and terminate the read thread?

OnConnected (or OnStatus(hsConnected)), and OnStatus(hsDisconnecting), respectively.

Though honestly, I wouldn't bother with the events at all.  As I mentioned earlier, I would simply create the thread after Connect() exits without error (which you are already doing), and then signal the thread to terminate before calling Disconnect() (which you are already doing), and wait for the thread to finish terminating after Disconnect() exits (which you are not doing), eg:

Code:
procedure TTcpClientComm.Disconnect;
begin
  if FReadThread <> nil then
    FReadThread.Terminate;
  try
    FConn.Disconnect;
  finally
    if FReadThread <> nil then
    begin
      FReadThread.WaitFor;
      FreeAndNil(FReadThread);
    end;
  end;
end;

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]Notice that I know when the socket is commanded to connect but not necessarily when it is disconnected since that can be commanded from the client side.

Then you do know when it would be commanded to disconnect, since that is when it would be calling Disconnect() for itself.

(09-18-2020, 10:00 AM)BosseB Wrote: [ -> ]My current implementation of the event driven data reception class using TIdTcpClient shown below.
Does it look sensible now, especially how the thread is created and destroyed in connect/disconnect?

I have a few notes:

- I would suggest making the Data parameter of TCommRxEvent be const to disable unnecessary reference counting.  Same with the WriteString() and WriteData() methods.

- there is no reason for TReadingThread to have its own FOnRxData member.  Give the thread a pointer to the owning TTcpClientComm instead, and then the thread can use the TTcpClientComm.FOnRxData member directly.

- it is not a good idea to use the TIdTCPConnection.Connected() method at all.  It performs a read operation to determine if the socket is still alive, and so that will risk corrupting your communications if the socket is read from by two separate threads at the same time.  If you must have a GetConnected() method in TTcpClientComm then I suggest giving it its own FConnected boolean member that you set to True after calling TIdTCPClient.Connect(), and then set to False when an IO error is detected and/or when you are disconnecting the TIdTCPClient.  And there is no good reason to call Connected() right before performing a read/write operation.  If the socket has been disconnected/lost, the operation will raise an exception.

- your WriteString() method will not work for Unicode strings (Delphi 2009+, or FPC in UnicodeStrings mode).  You should let Indy convert the Data string to a TIdBytes, eg:

Code:
DataBytes := ToBytes(Data, IndyTextEncoding_UTF8);

Alternatively:

Code:
DataBytes := IndyTextEncoding_UTF8.GetBytes(Data);

- your thread's Execute() method can be simplified to this:

Code:
procedure TReadingThread.Execute;
begin
  while not Terminated do
  begin
    // read bytes from FConn up to InputBuffer.Size bytes ...
    FConn.IOHandler.ReadBytes(FRxData, -1, False);
    // process bytes as needed ...
    if Assigned(FOnRxData) then
      Synchronize(DoOnRxData); 
    end;
  end;
end;

BosseB

09-18-2020, 09:12 PM
Thanks for your input!
I had been wondering about the transfer of the event function into the thread, but as you point out if I supply the owner as parameter then I could both reach the event function and FComm itself via that parameter.
I will digest and implement your simplifications now.

Regarding the protocol usage I wanted to make this class as general purpose as possible only implementing the event driven receive.
The first use is in fact a program which uses the protocol. But in future applications that might not be the case.
Anyway the application I am porting has a lot of code dealing with the protocol so I don't want to change that.

Re strings, I think FreePascal did not do what Delphi did and throw out the old string and replace with WideString still named string.
Instead FPC adheres to string = AnsiString as far as I know. I will probably change the ported app such that the containers are declared AnsiString instead, but generally I am switching to TBytes (or TIdBytes).
I will not go back to Delphi anyway, especially since I got my new notebook with Windows 10 (ouch!). I installed Delphi7 and Delpi2007 on it and will put Delphi XE7 only if needed.
Been doing Delphi since 1995 or so (Delphi1) until XE5, I only upgraded to XE7 once but never really used it.
But I am doing a lot of stuff on Raspberry Pi boxes these days and then FreePascal and Lazarus are what I use.

BosseB

09-19-2020, 07:49 AM
I have now looked over the code and I am not understanding why the OnStatusChange event could not be used to manage the connection...
It seems like it is there just in order to handle these things.
Will this not work:

Code:
    property Connected: boolean read FConnected;
...
procedure TTcpClientComm.Connect(Host: string; Port: word);
begin
  FConn.Connect(Host, Port); //Create read thread in OnStatus.hsConnected
end;

procedure TTcpClientComm.Disconnect;
begin
  FConn.Disconnect; //Dispose of read thread in OnStatus.hsDisconnected, will also handle client disconnect
end;

procedure TTcpClientComm.OnStatusChange(ASender: TObject; const AStatus: TIdStatus; const AStatusText: string);
{This procedure parses the socket status change messages and manages the read thread}
begin
  case AStatus of
    hsResolving: ;
    hsConnecting: ;
    hsConnected:
      begin
        FReadThread := TReadingThread.Create(Self);
        FConnected := true;
      end;
    hsDisconnecting: FConnected := false;
    hsDisconnected:
      begin
        FConnected := false;
        if FReadThread <> NIL then
          FReadThread.Terminate;
        if FReadThread <> NIL then
        begin
          FReadThread.WaitFor;
          FreeAndNil(FReadThread);
        end;
      end;
    hsStatusText: ;
  end;
end;

I changed the thread according to your suggestions, like using Connected as a property and reading all available bytes, but a question here:
I have set a read timeout of 100 ms, which means that the Execute will loop on that time if there are no data, right?
So the thread can be terminated properly within that time.
Is this OK:
Code:
TReadingThread = class(TThread)
  protected
    FRxData: TIdBytes;
    FOwner: TTcpClientComm;
    procedure Execute; override;
    procedure DoOnRxData;
  public
    constructor Create(Owner: TTcpClientComm);
  end;

implementation

constructor TTcpClientComm.Create;
begin
  FConnected := false;
  FHost := '';
  FPort := 0;
  FConn := TIdTCPClient.Create(NIL);
  FConn.OnStatus := OnStatusChange;
  FConn.ConnectTimeout := 5000; //5 second connect timeout
  FConn.ReadTimeout := 100; //Used in the read thread to not block
end;


{ TReadingThread }

procedure TReadingThread.Execute;
begin
  while not Terminated do
  begin
    //Do not use FOwner.FConn.Connected here because that triggers a read operation on the socket...
    if FOwner.Connected then //Connected is a property reading the boolean FConnected
    begin
      FOwner.FConn.IOHandler.ReadBytes(FRxData, -1, False); //Get all available data with timeout
      if Assigned(FOwner.FOnRxData) then
        Synchronize(DoOnRxData);
    end;
  end;
end;

procedure TReadingThread.DoOnRxData;
begin
  if Assigned(FOwner.FOnRxData) and (Length(FRxData) > 0) then
    FOwner.FOnRxData(Self, FRxData);
end;

constructor TReadingThread.Create(Owner: TTcpClientComm);
begin
  FOwner := Owner; //Set reference to Owner so we can use the FConn object directly
  inherited Create(False);
end;
If the ReadBytes call would be blocking forever, how can the thread end if it blocks in the execute procedure?
Is it even possible to quit it from the main application?
I believe there must be a timeout here, correct?

Edit:
I am programming this class without being able to test it because the main application it is part of does not build yet and there are tons of Delphi/Windows things that need to be fixed there first...
So the most I can do now with this is to do syntax checks.

rlebeau

09-21-2020, 06:31 PM
(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]Re strings, I think FreePascal did not do what Delphi did and throw out the old string and replace with WideString still named string.

Delphi did not change the string type to WideString.  It changed to a new UnicodeString type.  WideString still exists as a separate type.

FreePascal does the exact same thing under {$mode DelphiUnicode} and {$modeswitch UnicodeStrings}, for Delphi compatibility.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]Instead FPC adheres to string = AnsiString as far as I know.

By default, yes.  However, under Lazarus, the default AnsiString encoding is actually set to UTF-8.  So they didn't strictly keep the original ANSI behavior of AnsiString, either.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]I will probably change the ported app such that the containers are declared AnsiString instead

Please don't!  AnsiString is simply not intended for non-textual data, and it WILL break binary data under Delphi 2009+'s AnsiString codepage handling, and under FreePascal/Lazarus' AnsiString UTF-8 handling.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]but generally I am switching to TBytes (or TIdBytes).

That is a good idea.

(07-07-2020, 10:33 AM)BosseB Wrote: [ -> ]But I am doing a lot of stuff on Raspberry Pi boxes these days and then FreePascal and Lazarus are what I use.

Are you aware that Delphi Android apps and Linux apps can run on Raspberry Pi?
Pages: 1 2 3
Atozed Forums > Indy > Indy General Discussion > Porting old Delphi2007 application using TServerSocket....