TComPort data loss when trying to read ASTM 1391 data using TComDataPacket

Question 1

You have a Typo in your Code.

procedure TdmInstrument.cdpPacketCustomStop ...

begin
  ....

      Pos := System.Pos(cCR + cLF, Str) + 1;
      if Pos = 0 then
        Pos := System.Pos(cEOT, Str);
  ....
end;

if Pos = 0 then . Pos never can be 0

You should not use Pos as your variable. And using it to competition to System.Pos.

Some code optimization

procedure TdmInstrument.cdpPacketPacket(Sender: TObject; const Str: string);

begin
  if not FInitInfo.IsASTM then
  begin
    RawRecord := '';
    if Pos(#0, Str) > 0 then Str:=Stringreplace(Str,#0,' ',[]);
    RawRecord := RawRecord + Str;
  end else begin
    ASTMStr := ASTMStr + Str;
    if (Pos(cENQ, ASTMStr) + Pos(cEOT, ASTMStr) + Pos(cCR + cLF,Str) = 0)  then 
      Exit; // ASTM packet is not yet complete - exit
            // Do Not exit if there is a `cCR + cLF`
    if Pos(cEOT, Str) = 0 then cpCom.WriteStr(cACK);
            // write only when one of  `cENQ , cCR + cLF` is present
    RawRecord := ASTMStr;
  end;

 // we have a packet, so parse it
  ParsePacket;
  end;

Question 2

There are a few problems here.

First, your custom data packet handler is re-entrant. This is bad. Calling Application.ProcessMessages inside the packet handler will break out of the handler and start running through the function from the start if a new data packet is received in the same time, only to continue from where it left off after the subsequent package has been fully processed (unless another packet comes in during that time in which case it will restart again). This is not the behaviour you want since it will lead to out-of-order packet processing and is probably related to your need for sleep.

What I might suggest is to configure separate data packets for each command being sent from the instrument - ie :

Data Packet 1 : Start String ENQ, End String #13#10 {CRLF}
Data Packet 2 : Start String STX, End String #13#10 {CRLF}
Data Packet 3 : Start String EOT, End String #13#10 {CRLF}

You will need either some class variables, a record, an object, etc, to track the progress of the packet. Something like this, as an example :

TASTMPkt = record
  Started : boolean;
  Complete : boolean;
  Data : TStringList;
end;

In the ENQ packet handler you would do something like :

if FASTMPkt.Data = nil then FASTMPkt.Data := TStringList.Create();
FASTMData.Clear();
FASTMPkt.Started := true;
FASTMPkt.Complete := false;
cpCom.WriteStr(cACK);

in the STX handler :

if (not FASTMPkt.Started) or (FASTMPkt.Complete) then begin 
  // Raise exception, etc
end else begin
  FASTMPkt.Data.Add(Str);
  cpCom.WriteStr(cACK);
end;

in the EOT handler :

if (not FASTMPkt.Started) or (FASTMPkt.Complete) then begin 
  // Raise exception, etc
end else begin
  cpCom.WriteStr(cACK);
  FASTMPacket.Complete := true;
  ProcessPacket;
end;

Where ProcessPacket could then work through the string list data and do whatever. This avoids clogging up the UI thread waiting for possibly incoming packets, allows you to use timers to check for FASTMPkt completion in a timely manner (you could start a timeout timer in the ENQ packet handler, reset it in the STX handler, and stop it in the EOT handler, for example). It also avoids sleep and ProcessMessages, and generally gives you ways to handle errors and ensure proper process flow.

As an aside, I've never used TComPort or TDataPacket but I highly recommend AsyncPro (TApdComPort, TApdDataPacket, etc) - these are great components that can be configured as visual or non-visual components and I've found them to be very competent and reliable. My answer here assumes that these two components work in generally the same way (which I think they do).

http://sourceforge.net/projects/tpapro/

Question 3

In my opinion, creating a successful data exchange using TComPort (or any serial library) is the hardest thing of all. TComPort gives you good routines but there is no good example of a simple 'Send and wait until a reply' and almost all of the serial communications that I've done with Delphi need some sort of 'wait until a terminating condition'. I used to use AsyncPro and although it is still available it too does not have clear examples of how to set up a two-way serial with send and reply. Thus you are tempted to create something that uses Application.ProcessMessages to 'fetch' the response characters and as pointed out by J.... this brings other problems.

To solve this I made my own addition to TComPort as follows. This is probably not optimal but it works with a number of serial instruments with many different protocols.

First, configure TComPort as follows - the key bit is the FStopEvent...

constructor TArtTComPort.Create( const APort : string);
begin
  inherited Create;

  FTimeoutMS := 3000;

  FComPort := TComPort.Create( nil );

  FComPort.Events := [];  // do not create monitoring thread

  FComPort.Port := APort;

  FComPortParametersStr := sDefaultSerialPortParameters;

  FFlowControl := sfcNone;

  // Prepare a stop event for killing a waiting communication wait.
  FStopEvent := TEvent.Create(
    nil, //ManualReset
    false, //InitialState
    false,
    'StopEvent' );

end;

To send characters you simply call FComPort.WriteStr.

While you wait for some response I use the following. This allows me to specify what the terminating character is, and to ignore (or process) trailing characters after. On success, it simply returns the response. It does NOT call Application.ProcessMessages, so there are no rentrancy issues and it allows for a timeout.

function TArtTComPort.SerialPort_AwaitChars(AMinLength: integer;
  ATerminator: char; AQtyAfterTerm: integer; ARaise: boolean): string;
var
  fDueBy : TDateTime;

  function IsEndOfReplyOrTimeout( var AStr : string ) : boolean;
  var
   I : integer;
  begin
    Result := False;
    If ATerminator <> #0 then
      begin
      I := Length( AStr ) - AQtyAfterTerm;
      If I > 0 then
        Result := AStr[I] = ATerminator;
      end;
    If not Result then
      Result := Length(AStr) >= AMinLength;


    // Un-comment this next line to disable the timeout.
    //Exit;

    If not Result then
      begin
      Result := Now > fDueBy;
      If Result then
        If ARaise then
          raise EArtTComPort.Create( 'Serial port reply timeout' )
        else
          AStr := '';
      end;
  end;

var
  Events : TComEvents;
  iCount : integer;
  S : string;
begin
  Assert( AMinLength > 0, 'Invalid minimum length' );

  If not FComPort.Connected then
    begin
    Result := '';
    Exit;
    end;

  fDueBy := Now + (FTimeoutMS * TDMSec );

  Result := '';

  Repeat

    // Setup events to wait for:
    Events := [evRxChar, evTxEmpty, evRxFlag, evRing, evBreak,
             evCTS, evDSR, evError, evRLSD, evRx80Full];

    // Wait until at least one event happens.
    FComPort.WaitForEvent(
      Events,
      FStopEvent.Handle,
      FTimeOutMS);

    If Events = [] then // timeout
      begin
      If ARaise then
        raise EArtTComPort.Create( 'Serial port reply timeout' )
      end
     else
      begin
      If evRxChar in Events then
        begin
        iCount := FComport.InputCount;
        FComPort.ReadStr( S, iCount );
        Result := Result + S;
        end;
      end;

  until IsEndOfReplyOrTimeout( Result );


end;

Note that this code has some other small dependencies not shown but it should give you a good start. If anyone can show me how this can be achieved within TComPort code I will be grateful.