Search code examples
delphiusbhidjedi

Using Delphi + Jedi, losing USB data when device sends it "too fast"


using Delphi XE2 and TJvHidDevice class from Jedi library, I managed to successfully communicate with a USB device (pic32mx7 board, with my code running on it). The usual way of "send request, wait for single response" works.

The problem is with a command that results in a larger number of consecutive responses. If those responses are sent by the device as fast as possible - or even if I add a small delay between them like 5ms - I lose packets (reports? frames?). The OnDeviceData event simply doesn't seem to fire for all of them. If I add larger delays in the device's code, the problem goes away.

I used USBPcap program to capture USB data and dump it to a file which, once I open it in WireShark, contains all of the data sent by the device (I send 255 packets as a test, with all zeroes and one "1" shifting its place by 1 position in every packet). So, I think both the device and Windows are doing their job.

To make sure my Delphi code is not faulty, I tried the Jedi example project "DevReader" (here is the main.pas code) which dumps data on screen and it is missing packets as well.

I feel like there should be more information on the net about Jedi's USB classes but I am having trouble finding it.

I may be able to avoid this problem by aggregating/condensing the device's responses, but would still like to know what's going on.

Edit:

  1. Tried from a console app: packets were not lost anymore.
  2. Modified the Jedi demo app to only count received packets and update a counter label on screen (no forced window repaint) - no lost packets.
  3. Added sleep(1) in the OnData event - no lost packets.
  4. Added sleep(2) in the OnData event - losing packets again.

This looks like the Jedi thread that reads data must not be delayed by any processing - shouldn't there be some buffering of data going on (by Windows?) that would allow for this type of processing delays? Judging by the packet loss "pattern" it seems as if there is buffering, but it is insufficient because I can receive e.g. 30 packets then lose 5 then receive another 20 etc.

I will modify my code to copy the data and exit the OnData event as quickly as possible so that the thread has minimum "downtime" and I will report the outcome.


Solution

  • Since the cause of the problem appears to be related to the amount of time the USB reading thread is blocked by Synchronise, i.e. the data processing carried out by the main thread, I made changes in the thread code, (TJvHidDeviceReadThread class, JvHidControllerClass.pas unit). Any code that used this unit and the classes contained should still work without any modifications, nothing public was changed.

    New behavior: every time the data is read, it is placed in a thread safe list. Instead of Synchronise it now uses Queue, but only if it is not queued already. The Queued method reads from the thread safe list until it is empty. It fires an event (same event as in the old code) for each buffered report in the list. Once the list is empty, the "Queued" flag is reset and the next read will cause Queuing again.

    In the tests so far I did not encounter lost packets.

    The thread class was extended:

    
      TJvHidDeviceReadThread = class(TJvCustomThread)
      private
        FErr: DWORD;
    
        // start of additions
        ReceivedReports : TThreadList;
        Queued: boolean;
        procedure PushReceivedReport(const bytes: array of byte; const NumBytesRead: cardinal);
        function PopReceivedReport(var ReportID: byte; var ReportBytes: TBytes): boolean;
        procedure FlushBuffer;
        // end of additions
    
        procedure DoData;
        procedure DoDataError;
        constructor CtlCreate(const Dev: TJvHidDevice);
      protected
        procedure Execute; override;
      public
        Device: TJvHidDevice;
        NumBytesRead: Cardinal;
        Report: array of Byte;
        constructor Create(CreateSuspended: Boolean);
        //added destructor:
        destructor Destroy; override;
      end;
    

    In the implementation section, the following was modified:

    constructor TJvHidDeviceReadThread.CtlCreate(const Dev: TJvHidDevice);
    begin
      inherited Create(False);
      // start of changes
      ReceivedReports := TThreadList.Create; 
      // end of changes
      Device := Dev;
      NumBytesRead := 0;
      SetLength(Report, Dev.Caps.InputReportByteLength);
    end;
    
    procedure TJvHidDeviceReadThread.Execute;
    ...
    ...
    ...
        //replaced: Synchronize(DoData); with:
        PushReceivedReport (Report, NumBytesRead);
    ...
    

    And the following was added:

    type
    
    TReport = class
      ID: byte;
      Bytes: TBytes;
    end;
    
    destructor TJvHidDeviceReadThread.Destroy;
    var
      l: TList;
    begin
      RemoveQueuedEvents (self);
      try
        l := ReceivedReports.LockList;
        while l.Count>0 do
          begin
            TReport(l[0]).Free;
            l.Delete(0);
          end;
      finally
        ReceivedReports.UnlockList;
        FreeAndNil (ReceivedReports);
      end;
    
      inherited;
    end;
    
    procedure TJvHidDeviceReadThread.FlushBuffer;
    var
      ReportID: byte;
      ReportBytes: TBytes;
    begin
      while PopReceivedReport (ReportID, ReportBytes) do
            Device.OnData(Device, ReportID, ReportBytes, length(ReportBytes));
    end;
    
    function TJvHidDeviceReadThread.PopReceivedReport(var ReportID: byte; var ReportBytes: TBytes): boolean;
    var
      l: TList;
      rep: TReport;
    begin
      l := ReceivedReports.LockList;
      rep := nil;
      try
        result := l.Count>0;
        if result
          then
            begin
              rep := l[0];
              l.Delete(0);
            end
          else Queued := false;
      finally
        ReceivedReports.UnlockList;
      end;
    
      if result then
        begin
          ReportID := rep.ID;
          SetLength(ReportBytes, length(rep.Bytes));
          System.move (rep.Bytes[0], ReportBytes[0], length(rep.Bytes));
          rep.Free;
        end;
    end;
    
    procedure TJvHidDeviceReadThread.PushReceivedReport(const bytes: array of byte; const NumBytesRead: cardinal);
    var
      rep: TReport;
    begin
      rep := TReport.Create;
      setlength (rep.Bytes, NumBytesRead-1);
      rep.ID := Bytes[0];
      System.move (Bytes[1], rep.Bytes[0], NumBytesRead-1);
    
      // explicitely lock the list just to provide a locking mechanism for the Queue flag as well
      ReceivedReports.LockList;
      try
        if not Queued then
          begin
            Queued := true;
            Queue (FlushBuffer);
          end;
        ReceivedReports.Add(rep);
      finally
        ReceivedReports.UnlockList;
      end;
    end;