wjasper / Linux_Drivers

Open source Linux device drivers
GNU General Public License v3.0
111 stars 64 forks source link

Reading scans with usb-ctr.c #30

Closed MarkRivers closed 4 years ago

MarkRivers commented 4 years ago

I am working on an EPICS Linux driver for the UCB-CTR04/08 modules. I already have Windows drivers based on the UL from MC. It looks like you have been working on the driver recently, in particular in the areas that I am having trouble with.

I want to scan for relatively long times and periodically read out the device. The problem is that usbScanRead_USB_CTR always returns an error if the number of bytes transmitted does not match the number requested.

Here is the diagnostic output of my program:

mcUSB_CTR::cbCInScan calling usbScanConfigW_USB_CTR lastElement=15, list[0]=0, list[1]=8
mcUSB_CTR::cbCInScan calling usbScanStart_USB_CTR scanCount=0, *Rate=100, scanOptions=0x0
mcUSB_CTR::readThread Calling usbScanRead_USB_CTR count=20, lastElement=15, rawBuffer=0x7f25500008c0
usbScanRead_USB_CTR: error in usb_bulk_transfer.: Resource temporarily unavailable
usbAInScanRead_USB_CTR: number of bytes transferred = 512, nbytes = 640
mcUSB_CTR::readThread usbScanRead_USB_CTR returned -8
mcUSB_CTR::readThread Error calling usbScanRead_USB_CTR
mcUSB_CTR::readThread Calling usbScanRead_USB_CTR count=20, lastElement=15, rawBuffer=0x7f25500008c0
usbScanRead_USB_CTR: error in usb_bulk_transfer.: Resource temporarily unavailable
usbAInScanRead_USB_CTR: number of bytes transferred = 512, nbytes = 640
mcUSB_CTR::readThread usbScanRead_USB_CTR returned -8

Note that am asking usbScanRead_USB_CTR for 20 points, each with 16 elements, for a size of 20 16 2 = 640. But it is only getting 512 bytes, so it returns an error and no way of knowing that 512 bytes were actually read OK. Ideally I would like it to wait until 640 bytes have been received, but I would be willing to handle the data in 512 byte chunks if that is all libusb will do.

It seems like a good idea to enhance this function to not return an error if at least some data was returned, and to return the number of bytes transmitted either as the function return (>0) or as an additional argument.

I also have a question about usbScanStart_USB_CTR. The comments at the beginning say this:

void usbScanStart_USB_CTR(libusb_device_handle *udev, uint32_t count, uint32_t retrig_count, double frequency, uint8_t options)
{
  /*
    count:         the total number of scans to perform (0 for continuous scan)
    retrig_count:  the number of scans to perform for each trigger in retrigger mode
    pacer_period:  pacer timer period value (0 for external clock)
    packet_size:   number of samples - 1 to transfer at a time.
    options:       bit field that controls various options
      bit 0:   1 = Maintain counter value on scan start, 0 = Clear counter value on scan start
      bit 1:   Reserved
      bit 2:   Reserved
      bit 3:   1 = use trigger
      bit 4:   Reserved
      bit 5:   Reserved
      bit 6:   1 = retrigger mode,  0 = normal trigger
      bit 7:   Reserved

However, there is actually no packet_size argument. I am not sure if it would help with my problem, since 640 is still bigger than the 512 bytes that were apparently available when I made my request.

Thanks, Mark

MarkRivers commented 4 years ago

Something seems wrong with my driver. If I set the read count so the data to be read is less than 512 bytes libusb_bulk_transfer is returning 0. If I set it larger than 512 bytes libusb_bulk_transfer is only returning 512 bytes. This happens even if I wait so I am sure that there are more than 512 bytes available.

Your test program test-usb-ctr.c does not have this problem, libusb_bulk_transfer is reading 3200 bytes. I added a diagnostic printf in usbScanRead_USB_CTR showing the number of bytes actually transferred.

Testing scan input
Connect Timer 1 to Counter 1
libusb_bulk_transfer transferred 3200 bytes
Scan: 0     0  0  0  0
Scan: 1     1000  10  1  0
Scan: 2     2000  20  2  0
Scan: 3     3000  30  3  0
Scan: 4     4000  40  4  0
Scan: 5     5000  50  5  0
Scan: 6     6000  60  6  0
Scan: 7     7000  70  7  0
Scan: 8     8000  80  8  0
Scan: 9     9000  90  9  0
wjasper commented 4 years ago

Mark, can you modify my test program and send it to me so I can duplicate your error? Are you changing packet_size? I think there is a limit on what it should be, but I don't recall. I've rewritten the driver in python that may be more robust. If you request 640 bytes and the device only return 512, the FIFOs will eventually get screwed up and it is a really bad idea to keep going, since things will fail and it will be difficult to figure out were things went south. In the code I'm setting packet_size to wMaxPacketSize, which I think is correct.

MarkRivers commented 4 years ago

Warren, that was a great suggestion, and indeed I can duplicate the error. I made these 2 changes to test-usb-ctr.c.

corvette:measComp/measCompApp/Linux_Drivers>git diff test-usb-ctr.c
diff --git a/measCompApp/Linux_Drivers/test-usb-ctr.c b/measCompApp/Linux_Drivers/test-usb-ctr.c
index a084154..295cc74 100644
--- a/measCompApp/Linux_Drivers/test-usb-ctr.c
+++ b/measCompApp/Linux_Drivers/test-usb-ctr.c
@@ -153,7 +153,7 @@ int main (int argc, char **argv)
       case 'i':
        printf("Testing scan input\n");
        printf("Connect Timer 1 to Counter 1\n");
-       count = 100;       // total number of scans to perform
+       count = 20;       // total number of scans to perform
        frequency = 1000;  // scan rate at 1000 Hz

        // Set up the scan list (use 4 counter 0-3)
@@ -185,7 +185,7 @@ int main (int argc, char **argv)
        usbTimerDelayW_USB_CTR(udev, timer, 0);
        usbTimerControlW_USB_CTR(udev, timer, 0x1);

-       usbScanStart_USB_CTR(udev, count, 0, frequency, 0);
+       usbScanStart_USB_CTR(udev, 0, 0, frequency, 0);
         usbScanRead_USB_CTR(udev, count, scanList.lastElement, data);
        usbTimerControlW_USB_CTR(udev, timer, 0x0);

Changing count from 100 to 20 is not needed to create the problem, you will just see different numbers in the error message from usbScanRead_USB_CTR. The important change is changing from passing 0 for the count in the call to usbScanStart_USB_CTR. This is what is required for a continuous scan, which is what I want to use.

This is the output when I run the modified test program.

i
Testing scan input
Connect Timer 1 to Counter 1
usbScanRead_USB_CTR: error in usb_bulk_transfer.: Resource temporarily unavailable
usbAInScanRead_USB_CTR: number of bytes transferred = 512, nbytes = 640
Scan: 0     0  0  0  0
Scan: 1     1  10  0  0
Scan: 2     2  20  0  0
Scan: 3     3  30  0  0
Scan: 4     4  40  0  0
Scan: 5     5  50  0  0
Scan: 6     6  60  0  0
Scan: 7     7  70  0  0
Scan: 8     8  80  0  0
Scan: 9     9  90  0  0
Scan: 10     10  100  0  0
Scan: 11     11  110  0  0
Scan: 12     12  120  0  0
Scan: 13     13  130  0  0
Scan: 14     14  140  0  0
Scan: 15     15  150  0  0
Scan: 16     0  0  0  0
Scan: 17     0  0  0  0
Scan: 18     0  0  0  0
Scan: 19     0  0  0  0

Have you tried continuous scans like this?

I will see what is needed to be able to read the data when doing continuous scans.

wjasper commented 4 years ago

Hi Mark, OK, I think I understand now. Yes, continuous mode is a little strange, and I think I'm going to refactor my code a little. I did not write the firmware, but what I think happens is that in continuous mode, the device sends data back in chunks of wMaxPacketSize and it is important to acquire data in those quantities and make sure not to overflow the FIFO. That is why I need to find wMaxPacketSize. Since you are sampling "forever" in continuous mode, the amount of data you get back in a read is not critical. If you want just 20 samples, you should set the count to 20. If you want to sample continuously, you get wMaxPacketSize samples back forever and the program needs to keep the buffers from overrunning which is frequency dependent. I split up usbAInScanStart from usbAInScanRead but need to handle continuous mode a little differently. I'll push something out next week. It is handled better in Python.

MarkRivers commented 4 years ago

Hi Warren, Now that I understand what is happening it is easy to fix. Here are my changes to the test program to work in continuous mode. Note that I set count back to its original value of 100.

corvette:measComp/measCompApp/Linux_Drivers>git diff .
diff --git a/measCompApp/Linux_Drivers/test-usb-ctr.c b/measCompApp/Linux_Drivers/test-usb-ctr.c
index a084154..221ab3a 100644
--- a/measCompApp/Linux_Drivers/test-usb-ctr.c
+++ b/measCompApp/Linux_Drivers/test-usb-ctr.c
@@ -185,8 +185,17 @@ int main (int argc, char **argv)
        usbTimerDelayW_USB_CTR(udev, timer, 0);
        usbTimerControlW_USB_CTR(udev, timer, 0x1);

-       usbScanStart_USB_CTR(udev, count, 0, frequency, 0);
-        usbScanRead_USB_CTR(udev, count, scanList.lastElement, data);
+       usbScanStart_USB_CTR(udev, 0, 0, frequency, 0);
+       int wordsToRead = count * (scanList.lastElement+1);
+       int readingsNeeded = (wordsToRead-1)/256 + 1;
+       uint16_t *pData = data;
+       printf("wordsToRead=%d, readingsNeeded = %d\n", wordsToRead, readingsNeeded);
+       for (i=0; i<readingsNeeded; i++) {
+      int status = usbScanRead_USB_CTR(udev, 0, scanList.lastElement, pData);
+      if (status != 0) printf("Error calling usbScanRead_USB_CTR = %d\n", status);
+      pData += 256;
+       }
+       usbScanStop_USB_CTR(udev);
        usbTimerControlW_USB_CTR(udev, timer, 0x0);

        for (i = 0; i < count; i++) {

Note that I think there is a bug in usb-ctr.c when count is 0. nbytes should be wMaxPacketSize, not wMaxPacketSize/2.

index 8f4aaae..929da4c 100644
--- a/measCompApp/Linux_Drivers/usb-ctr.c
+++ b/measCompApp/Linux_Drivers/usb-ctr.c
@@ -712,7 +712,7 @@ int usbScanRead_USB_CTR(libusb_device_handle *udev, int count, int lastElement,
   if (count > 0) {
     nbytes = count*(lastElement+1)*2;
   } else {             // continuous mode
-    nbytes = wMaxPacketSize/2;
+    nbytes = wMaxPacketSize;
   }

   ret = libusb_bulk_transfer(udev, LIBUSB_ENDPOINT_IN|6, (unsigned char *) data, nbytes, &transferred, HS_DELAY);

With those changes this is the output from the test program.

i
Testing scan input
Connect Timer 1 to Counter 1
wordsToRead=1600, readingsNeeded = 7
Scan: 0     0  0  0  0
Scan: 1     1  10  0  0
Scan: 2     2  20  0  0
Scan: 3     3  30  0  0
Scan: 4     4  40  0  0
Scan: 5     5  50  0  0
Scan: 6     6  60  0  0
Scan: 7     7  70  0  0
Scan: 8     8  80  0  0
Scan: 9     9  90  0  0
Scan: 10     10  100  0  0
Scan: 11     11  110  0  0
Scan: 12     12  120  0  0
Scan: 13     13  130  0  0
Scan: 14     14  140  0  0
Scan: 15     15  150  0  0
Scan: 16     16  160  0  0
Scan: 17     17  170  0  0
Scan: 18     18  180  0  0
Scan: 19     19  190  0  0
Scan: 20     20  200  0  0
Scan: 21     21  210  0  0
Scan: 22     22  220  0  0
Scan: 23     23  230  0  0
Scan: 24     24  240  0  0
Scan: 25     25  250  0  0
Scan: 26     26  260  0  0
Scan: 27     27  270  0  0
Scan: 28     28  280  0  0
Scan: 29     29  290  0  0
Scan: 30     30  300  0  0
Scan: 31     31  310  0  0
Scan: 32     32  320  0  0
Scan: 33     33  330  0  0
Scan: 34     34  340  0  0
Scan: 35     35  350  0  0
Scan: 36     36  360  0  0
Scan: 37     37  370  0  0
Scan: 38     38  380  0  0
Scan: 39     39  390  0  0
Scan: 40     40  400  0  0
Scan: 41     41  410  0  0
Scan: 42     42  420  0  0
Scan: 43     43  430  0  0
Scan: 44     44  440  0  0
Scan: 45     45  450  0  0
Scan: 46     46  460  0  0
Scan: 47     47  470  0  0
Scan: 48     48  480  0  0
Scan: 49     49  490  0  0
Scan: 50     50  500  0  0
Scan: 51     51  510  0  0
Scan: 52     52  520  0  0
Scan: 53     53  530  0  0
Scan: 54     54  540  0  0
Scan: 55     55  550  0  0
Scan: 56     56  560  0  0
Scan: 57     57  570  0  0
Scan: 58     58  580  0  0
Scan: 59     59  590  0  0
Scan: 60     60  600  0  0
Scan: 61     61  610  0  0
Scan: 62     62  620  0  0
Scan: 63     63  630  0  0
Scan: 64     64  640  0  0
Scan: 65     65  650  0  0
Scan: 66     66  660  0  0
Scan: 67     67  670  0  0
Scan: 68     68  680  0  0
Scan: 69     69  690  0  0
Scan: 70     70  700  0  0
Scan: 71     71  710  0  0
Scan: 72     72  720  0  0
Scan: 73     73  730  0  0
Scan: 74     74  740  0  0
Scan: 75     75  750  0  0
Scan: 76     76  760  0  0
Scan: 77     77  770  0  0
Scan: 78     78  780  0  0
Scan: 79     79  790  0  0
Scan: 80     80  800  0  0
Scan: 81     81  810  0  0
Scan: 82     82  820  0  0
Scan: 83     83  830  0  0
Scan: 84     84  840  0  0
Scan: 85     85  850  0  0
Scan: 86     86  860  0  0
Scan: 87     87  870  0  0
Scan: 88     88  880  0  0
Scan: 89     89  890  0  0
Scan: 90     90  900  0  0
Scan: 91     91  910  0  0
Scan: 92     92  920  0  0
Scan: 93     93  930  0  0
Scan: 94     94  940  0  0
Scan: 95     95  950  0  0
Scan: 96     96  960  0  0
Scan: 97     97  970  0  0
Scan: 98     98  980  0  0
Scan: 99     99  990  0  0
MarkRivers commented 4 years ago

Hi Warren, I have a question about the logic in usbScanStart_USB_CTR. It contains this line:

  data[12] = wMaxPacketSize - 1;

However, data[] is a uint8_t array, and wMaxPacketSize - 1 = 511, which does not fit in a uint8_t?

MarkRivers commented 4 years ago

I have a much cleaner solution for reading the data during continuous scan. It is actually the method I have been using in their Universal Library on Windows. They have a flag to cbCInScan called SINGLEIO. It tells the device to send the values for just a single reading in each data packet. It is not as efficient, but it makes the code much simpler since one does not need to deal with the 512 byte chunking.

I have implemented that in your library by adding the packet_size argument to usbScanStart_USB_CTR.

corvette:measComp/measCompApp/Linux_Drivers>git diff usb-ctr.*
diff --git a/measCompApp/Linux_Drivers/usb-ctr.c b/measCompApp/Linux_Drivers/usb-ctr.c
index 8f4aaae..08a16ba 100644
--- a/measCompApp/Linux_Drivers/usb-ctr.c
+++ b/measCompApp/Linux_Drivers/usb-ctr.c
@@ -585,7 +585,7 @@ void usbScanConfigW_USB_CTR(libusb_device_handle *udev, uint8_t lastElement, Sca
   }
 }

-void usbScanStart_USB_CTR(libusb_device_handle *udev, uint32_t count, uint32_t retrig_count, double frequency, uint8_t options)
+void usbScanStart_USB_CTR(libusb_device_handle *udev, uint32_t count, uint32_t retrig_count, double frequency, uint8_t packet_size, uint8_t options)
 {
   /*
     count:         the total number of scans to perform (0 for continuous scan)
@@ -661,7 +661,8 @@ void usbScanStart_USB_CTR(libusb_device_handle *udev, uint32_t count, uint32_t r
   memcpy(&data[0], &count, 4);
   memcpy(&data[4], &retrig_count, 4);
   memcpy(&data[8], &pacer_period, 4);
-  data[12] = wMaxPacketSize - 1;
+  if (packet_size == 0) packet_size = wMaxPacketSize/2;
+  data[12] = packet_size-1;
   data[13] = options;

   ret = libusb_control_transfer(udev, requesttype, SCAN_START, 0x0, 0x0,(unsigned char *) data, sizeof(data), HS_DELAY);
@@ -712,7 +713,7 @@ int usbScanRead_USB_CTR(libusb_device_handle *udev, int count, int lastElement,
   if (count > 0) {
     nbytes = count*(lastElement+1)*2;
   } else {             // continuous mode
-    nbytes = wMaxPacketSize/2;
+    nbytes = wMaxPacketSize;
   }

   ret = libusb_bulk_transfer(udev, LIBUSB_ENDPOINT_IN|6, (unsigned char *) data, nbytes, &transferred, HS_DELAY);
diff --git a/measCompApp/Linux_Drivers/usb-ctr.h b/measCompApp/Linux_Drivers/usb-ctr.h
index eb78920..48ff917 100644
--- a/measCompApp/Linux_Drivers/usb-ctr.h
+++ b/measCompApp/Linux_Drivers/usb-ctr.h
@@ -138,7 +138,7 @@ void usbCounterParamsW_USB_CTR(libusb_device_handle *udev, uint8_t counter, Coun

 void usbScanConfigR_USB_CTR(libusb_device_handle *udev, uint8_t lastElement, ScanList *scanList);
 void usbScanConfigW_USB_CTR(libusb_device_handle *udev, uint8_t lastElement, ScanList scanList);
-void usbScanStart_USB_CTR(libusb_device_handle *udev, uint32_t count, uint32_t retrig_count, double frequency, uint8_t options);
+void usbScanStart_USB_CTR(libusb_device_handle *udev, uint32_t count, uint32_t retrig_count, double frequency, uint8_t packet_size, uint8_t options);
 void usbScanStop_USB_CTR(libusb_device_handle *udev);
 void usbScanClearFIFO_USB_CTR(libusb_device_handle *udev);
 void usbScanBulkFlush_USB_CTR(libusb_device_handle *udev, uint8_t count);

These are the changes to test-usb-ctr.c.

diff --git a/measCompApp/Linux_Drivers/test-usb-ctr.c b/measCompApp/Linux_Drivers/test-usb-ctr.c
index a084154..90404c9 100644
--- a/measCompApp/Linux_Drivers/test-usb-ctr.c
+++ b/measCompApp/Linux_Drivers/test-usb-ctr.c
@@ -162,7 +162,8 @@ int main (int argc, char **argv)
            scanList.scanList[4*counter + bank] = (counter & 0x7) | (bank & 0x3) << 3 | (0x2 << 5);
          }
        }
-       scanList.lastElement = 15;
+       int wordsPerReading = 16;
+       scanList.lastElement = wordsPerReading-1;
        usbScanConfigW_USB_CTR(udev,scanList.lastElement, scanList);
        usbScanConfigR_USB_CTR(udev, scanList.lastElement, &scanList);

@@ -185,8 +186,14 @@ int main (int argc, char **argv)
        usbTimerDelayW_USB_CTR(udev, timer, 0);
        usbTimerControlW_USB_CTR(udev, timer, 0x1);

-       usbScanStart_USB_CTR(udev, count, 0, frequency, 0);
-        usbScanRead_USB_CTR(udev, count, scanList.lastElement, data);
+       usbScanStart_USB_CTR(udev, 0, 0, frequency, wordsPerReading, 0);
+       uint16_t *pData = data;
+       for (i=0; i<count; i++) {
+      int status = usbScanRead_USB_CTR(udev, 1, scanList.lastElement, pData);
+      if (status != 0) printf("Error calling usbScanRead_USB_CTR = %d\n", status);
+      pData += wordsPerReading;
+       }
+       usbScanStop_USB_CTR(udev);
        usbTimerControlW_USB_CTR(udev, timer, 0x0);

        for (i = 0; i < count; i++) {

So now it just grabs a single set of counter values at a time, and the loop is just over "count".

I like the flexibility to tell the system how much data to return per read. If packet_size is set to 0 the previous behavior is retained.

MarkRivers commented 4 years ago

Hi Warren, My EPICS application uses the USB-CTR in 2 modes. Both modes are working fine with the Windows version of the driver, using the Measurement Computing UL library.

The first mode is a "preset-scaler". It counts for a preset amount of time. This is a screen shot when it is in the process of counting. image

This is a screen shot when counting is complete after 10 seconds. image

In this mode I do the following:

This is a version of your test program that uses this mode. It works fine, as does my EPICS Linux driver in this mode. https://github.com/epics-modules/measComp/blob/master/measCompApp/Linux_Drivers/test-usb-ctr-scaler.c

It requires a modified version of usb-ctr.c that takes the packet_size argument in usbScanStart_USB_CTR as shown in the previous message. I also made the following fix to usbScanRead_USB_CTR. It is needed because if the initial read of the device returns ret<0 but nbytes is a multiple of wMaxPacketSize the return value is 0, which is incorrect.

corvette:measComp/measCompApp/Linux_Drivers>git diff usb-ctr.c
diff --git a/measCompApp/Linux_Drivers/usb-ctr.c b/measCompApp/Linux_Drivers/usb-ctr.c
index 53990eb..4adf413 100644
--- a/measCompApp/Linux_Drivers/usb-ctr.c
+++ b/measCompApp/Linux_Drivers/usb-ctr.c
@@ -729,9 +729,10 @@ int usbScanRead_USB_CTR(libusb_device_handle *udev, int count, int lastElement,
     return ret;
   }

+  int dummy;
   // if nbytes is a multiple of wMaxPacketSize the device will send a zero byte packet.
   if ((nbytes%wMaxPacketSize) == 0) {
-    libusb_bulk_transfer(udev, LIBUSB_ENDPOINT_IN|6, (unsigned char *) value, 2, &ret, 100);
+    libusb_bulk_transfer(udev, LIBUSB_ENDPOINT_IN|6, (unsigned char *) value, 2, &dummy, 100);
   }

   status = usbStatus_USB_CTR(udev);

The second mode is Multi-Channel Scaler (MCS) mode. In this mode we acquire a time series of counts. The Channel Advance between time bins (dwell-time) can be from an internal clock, or via an external clock with optional divide by N. We use the external clock mode to trigger the next point via pulses to a motor controller, for example.

In this mode I set the total number of time points to be acquired (count). I then read out the scan data periodically in "chunks" in order to be able to provide a live update of the scan in progress.

This is a screen shot of the control screen for this mode. image

This is a screen shot of the live plotting update for each of the counters. image

This is a version of your test program that uses this mode.

https://github.com/epics-modules/measComp/blob/master/measCompApp/Linux_Drivers/test-usb-ctr-mcs.c

It collects 2048 scan points, and reads the data in 128 point chunks. It works fine if the frequency is 100 Hz. However, it fails if the frequency is 1000 Hz.

This is the output of test-usb-ctr-mcs with frequency=100. It works fine.

i
Testing scan input
Connect Timer 1 to Counter 1
Read 128 points OK, points remaining = 2048
Read 128 points OK, points remaining = 1920
Read 128 points OK, points remaining = 1792
Read 128 points OK, points remaining = 1664
Read 128 points OK, points remaining = 1536
Read 128 points OK, points remaining = 1408
Read 128 points OK, points remaining = 1280
Read 128 points OK, points remaining = 1152
Read 128 points OK, points remaining = 1024
Read 128 points OK, points remaining = 896
Read 128 points OK, points remaining = 768
Read 128 points OK, points remaining = 640
Read 128 points OK, points remaining = 512
Read 128 points OK, points remaining = 384
Read 128 points OK, points remaining = 256
Read 128 points OK, points remaining = 128
Scan: 0     0  0  0  0
Scan: 10     100000  1000  0  0
Scan: 20     200000  2000  0  0
Scan: 30     300000  3000  0  0
Scan: 40     400000  4000  0  0
Scan: 50     500000  5000  0  0
Scan: 60     600000  6000  0  0
Scan: 70     700000  7000  0  0
Scan: 80     800000  8000  0  0
Scan: 90     900000  9000  0  0
Scan: 100     1000000  10000  0  0
Scan: 110     1100000  11000  0  0
Scan: 120     1200000  12000  0  0
Scan: 130     1300000  13000  0  0
Scan: 140     1400000  14000  0  0
Scan: 150     1500000  15000  0  0
Scan: 160     1600000  16000  0  0
Scan: 170     1700000  17000  0  0
Scan: 180     1800000  18000  0  0
Scan: 190     1900000  19000  0  0
Scan: 200     2000000  20000  0  0
Scan: 210     2100000  21000  0  0
Scan: 220     2200000  22000  0  0
Scan: 230     2300000  23000  0  0
Scan: 240     2400000  24000  0  0
Scan: 250     2500000  25000  0  0
Scan: 260     2600000  26000  0  0
Scan: 270     2700000  27000  0  0
Scan: 280     2800000  28000  0  0
Scan: 290     2900000  29000  0  0
Scan: 300     3000000  30000  0  0
Scan: 310     3100000  31000  0  0
Scan: 320     3200000  32000  0  0
Scan: 330     3300000  33000  0  0
Scan: 340     3400000  34000  0  0
Scan: 350     3500000  35000  0  0
Scan: 360     3600000  36000  0  0
Scan: 370     3700000  37000  0  0
Scan: 380     3800000  38000  0  0
Scan: 390     3900000  39000  0  0
Scan: 400     4000000  40000  0  0
Scan: 410     4100000  41000  0  0
Scan: 420     4200000  42000  0  0
Scan: 430     4300000  43000  0  0
Scan: 440     4400000  44000  0  0
Scan: 450     4500000  45000  0  0
Scan: 460     4600000  46000  0  0
Scan: 470     4700000  47000  0  0
Scan: 480     4800000  48000  0  0
Scan: 490     4900000  49000  0  0
Scan: 500     5000000  50000  0  0
Scan: 510     5100000  51000  0  0
Scan: 520     5200000  52000  0  0
Scan: 530     5300000  53000  0  0
Scan: 540     5400000  54000  0  0
Scan: 550     5500000  55000  0  0
Scan: 560     5600000  56000  0  0
Scan: 570     5700000  57000  0  0
Scan: 580     5800000  58000  0  0
Scan: 590     5900000  59000  0  0
Scan: 600     6000000  60000  0  0
Scan: 610     6100000  61000  0  0
Scan: 620     6200000  62000  0  0
Scan: 630     6300000  63000  0  0
Scan: 640     6400000  64000  0  0
Scan: 650     6500000  65000  0  0
Scan: 660     6600000  66000  0  0
Scan: 670     6700000  67000  0  0
Scan: 680     6800000  68000  0  0
Scan: 690     6900000  69000  0  0
Scan: 700     7000000  70000  0  0
Scan: 710     7100000  71000  0  0
Scan: 720     7200000  72000  0  0
Scan: 730     7300000  73000  0  0
Scan: 740     7400000  74000  0  0
Scan: 750     7500000  75000  0  0
Scan: 760     7600000  76000  0  0
Scan: 770     7700000  77000  0  0
Scan: 780     7800000  78000  0  0
Scan: 790     7900000  79000  0  0
Scan: 800     8000000  80000  0  0
Scan: 810     8100000  81000  0  0
Scan: 820     8200000  82000  0  0
Scan: 830     8300000  83000  0  0
Scan: 840     8400000  84000  0  0
Scan: 850     8500000  85000  0  0
Scan: 860     8600000  86000  0  0
Scan: 870     8700000  87000  0  0
Scan: 880     8800000  88000  0  0
Scan: 890     8900000  89000  0  0
Scan: 900     9000000  90000  0  0
Scan: 910     9100000  91000  0  0
Scan: 920     9200000  92000  0  0
Scan: 930     9300000  93000  0  0
Scan: 940     9400000  94000  0  0
Scan: 950     9500000  95000  0  0
Scan: 960     9600000  96000  0  0
Scan: 970     9700000  97000  0  0
Scan: 980     9800000  98000  0  0
Scan: 990     9900000  99000  0  0
Scan: 1000     10000000  100000  0  0
Scan: 1010     10100000  101000  0  0
Scan: 1020     10200000  102000  0  0
Scan: 1030     10300000  103000  0  0
Scan: 1040     10400000  104000  0  0
Scan: 1050     10500000  105000  0  0
Scan: 1060     10600000  106000  0  0
Scan: 1070     10700000  107000  0  0
Scan: 1080     10800000  108000  0  0
Scan: 1090     10900000  109000  0  0
Scan: 1100     11000000  110000  0  0
Scan: 1110     11100000  111000  0  0
Scan: 1120     11200000  112000  0  0
Scan: 1130     11300000  113000  0  0
Scan: 1140     11400000  114000  0  0
Scan: 1150     11500000  115000  0  0
Scan: 1160     11600000  116000  0  0
Scan: 1170     11700000  117000  0  0
Scan: 1180     11800000  118000  0  0
Scan: 1190     11900000  119000  0  0
Scan: 1200     12000000  120000  0  0
Scan: 1210     12100000  121000  0  0
Scan: 1220     12200000  122000  0  0
Scan: 1230     12300000  123000  0  0
Scan: 1240     12400000  124000  0  0
Scan: 1250     12500000  125000  0  0
Scan: 1260     12600000  126000  0  0
Scan: 1270     12700000  127000  0  0
Scan: 1280     12800000  128000  0  0
Scan: 1290     12900000  129000  0  0
Scan: 1300     13000000  130000  0  0
Scan: 1310     13100000  131000  0  0
Scan: 1320     13200000  132000  0  0
Scan: 1330     13300000  133000  0  0
Scan: 1340     13400000  134000  0  0
Scan: 1350     13500000  135000  0  0
Scan: 1360     13600000  136000  0  0
Scan: 1370     13700000  137000  0  0
Scan: 1380     13800000  138000  0  0
Scan: 1390     13900000  139000  0  0
Scan: 1400     14000000  140000  0  0
Scan: 1410     14100000  141000  0  0
Scan: 1420     14200000  142000  0  0
Scan: 1430     14300000  143000  0  0
Scan: 1440     14400000  144000  0  0
Scan: 1450     14500000  145000  0  0
Scan: 1460     14600000  146000  0  0
Scan: 1470     14700000  147000  0  0
Scan: 1480     14800000  148000  0  0
Scan: 1490     14900000  149000  0  0
Scan: 1500     15000000  150000  0  0
Scan: 1510     15100000  151000  0  0
Scan: 1520     15200000  152000  0  0
Scan: 1530     15300000  153000  0  0
Scan: 1540     15400000  154000  0  0
Scan: 1550     15500000  155000  0  0
Scan: 1560     15600000  156000  0  0
Scan: 1570     15700000  157000  0  0
Scan: 1580     15800000  158000  0  0
Scan: 1590     15900000  159000  0  0
Scan: 1600     16000000  160000  0  0
Scan: 1610     16100000  161000  0  0
Scan: 1620     16200000  162000  0  0
Scan: 1630     16300000  163000  0  0
Scan: 1640     16400000  164000  0  0
Scan: 1650     16500000  165000  0  0
Scan: 1660     16600000  166000  0  0
Scan: 1670     16700000  167000  0  0
Scan: 1680     16800000  168000  0  0
Scan: 1690     16900000  169000  0  0
Scan: 1700     17000000  170000  0  0
Scan: 1710     17100000  171000  0  0
Scan: 1720     17200000  172000  0  0
Scan: 1730     17300000  173000  0  0
Scan: 1740     17400000  174000  0  0
Scan: 1750     17500000  175000  0  0
Scan: 1760     17600000  176000  0  0
Scan: 1770     17700000  177000  0  0
Scan: 1780     17800000  178000  0  0
Scan: 1790     17900000  179000  0  0
Scan: 1800     18000000  180000  0  0
Scan: 1810     18100000  181000  0  0
Scan: 1820     18200000  182000  0  0
Scan: 1830     18300000  183000  0  0
Scan: 1840     18400000  184000  0  0
Scan: 1850     18500000  185000  0  0
Scan: 1860     18600000  186000  0  0
Scan: 1870     18700000  187000  0  0
Scan: 1880     18800000  188000  0  0
Scan: 1890     18900000  189000  0  0
Scan: 1900     19000000  190000  0  0
Scan: 1910     19100000  191000  0  0
Scan: 1920     19200000  192000  0  0
Scan: 1930     19300000  193000  0  0
Scan: 1940     19400000  194000  0  0
Scan: 1950     19500000  195000  0  0
Scan: 1960     19600000  196000  0  0
Scan: 1970     19700000  197000  0  0
Scan: 1980     19800000  198000  0  0
Scan: 1990     19900000  199000  0  0
Scan: 2000     20000000  200000  0  0
Scan: 2010     20100000  201000  0  0
Scan: 2020     20200000  202000  0  0
Scan: 2030     20300000  203000  0  0
Scan: 2040     20400000  204000  0  0

This is the output of test-usb-ctr-mcs with frequency=1000. It has 2 problems:


Note that the I did not have timer 1 connected to counter 0, I had timer 0 connected, and it was running at 1 MHz.

There is an additional issue with this mode.  It currently requires that "count" (total scan points) must be a multiple of the readout chunk size.  If it is not then the last buffer to be read will be a different size, and it will generate a read error, even if I request a read of the size that it will be.  This is a problem because the user should be able to set any number of points in the scan, including a prime number.  This is not currently compatible with reading the data in chunks.

I don't have to worry about this in the Windows driver, somehow their code will read all the points in a scan without having to worry about buffer size.  I don't know how they are doing it of course.
wjasper commented 4 years ago

Hi Mark, Please update to latest version. I've added a 'C' option to test continuous mode. There are 2 frequencies you can change, the scan frequency and the timer frequency. Right now I've set them to 1000 for the single scan and to 100 for continuous scan. Let me know if this works for your cases, or if not, change my test code so I can duplicate your errors. For continuous mode, I now set count=0 that puts it into continuous mode and read out chunks of wMaxPacketSize which should work out. At really slow speeds, there my be timing issues, but at the speeds you are testing, it should be ok. I'm also returning the number of bytes transferred, which makes more sense.

MarkRivers commented 4 years ago

Can you merge my changes for the #defines? I cannot use the ones in the distribution because they conflict with cbw.h from Measurement Computing, which I also need to include in my application. Right now I can't run your version of test-usb-ctr.c because of the different macro names.

I think it might make sense to return the negative error codes when there is an error, and the positive transferred when there is no error.

MarkRivers commented 4 years ago

In general I think all of the #defines need to have a device-specific prefix or suffix. There are not just conflicts with cbw.h, there are conflicts within your own header files. For example:

corvette:~/measComp_Linux_Drivers/USB/mcc-libusb>grep "#define DIO_PORTB" *
minilab-1008.h:#define DIO_PORTB     (0x04)
usb-1024LS.h:#define DIO_PORTB (0x04)
usb-1208FS.h:#define DIO_PORTB (0x01)
usb-1208LS.h:#define DIO_PORTB     (0x04)
usb-1408FS.h:#define DIO_PORTB (0x01)
usb-dio24.h:#define DIO_PORTB (0x04)
usb-dio32HS.h:#define DIO_PORTB  (0x01)
usb-erb.h:#define DIO_PORTB     (0x1)

This means that if I want to write a program that talks to both the usb-1208FS and the usb-1208LS I can be in trouble because they both define DIO_PORTB but differently. I would have to isolate the code for them in separate source files so that both header files are not included in the same source file.

For my current application I really need to include cbw.h and usb-ctr.h in the same source file. Definitions from both are need in the same function.

MarkRivers commented 4 years ago

Hi Warren, I have built the version of test-usb-ctr.c that is in my pull request (i.e. your version with different #defines. I made this change to work with my version of usbScanStart_USB_CTR that takes a packet_size argument.

corvette:measComp/measCompApp/Linux_Drivers>git diff test-usb-ctr.c
diff --git a/measCompApp/Linux_Drivers/test-usb-ctr.c b/measCompApp/Linux_Drivers/test-usb-ctr.c
index b1a8ac4..e4609cd 100644
--- a/measCompApp/Linux_Drivers/test-usb-ctr.c
+++ b/measCompApp/Linux_Drivers/test-usb-ctr.c
@@ -238,7 +238,7 @@ int main (int argc, char **argv)
        usbTimerDelayW_USB_CTR(udev, timer, 0);
        usbTimerControlW_USB_CTR(udev, timer, 0x1);

-       usbScanStart_USB_CTR(udev, count, 0, frequency, 0);
+       usbScanStart_USB_CTR(udev, count, 0, frequency, 0, 0);
        flag = fcntl(fileno(stdin), F_GETFL);
        fcntl(0, F_SETFL, flag | O_NONBLOCK);
         j = 0;

I then made these changes to print out the values of the first reading in each buffer.

corvette:measComp/measCompApp/Linux_Drivers>git diff test-usb-ctr.c
diff --git a/measCompApp/Linux_Drivers/test-usb-ctr.c b/measCompApp/Linux_Drivers/test-usb-ctr.c
index b1a8ac4..0d1a6d7 100644
--- a/measCompApp/Linux_Drivers/test-usb-ctr.c
+++ b/measCompApp/Linux_Drivers/test-usb-ctr.c
@@ -230,7 +230,7 @@ int main (int argc, char **argv)

        // set up the timer to generate some pulses
        timer = 1;
-       timer_frequency = 100.;
+       timer_frequency = 10000.;
        period = 96.E6/timer_frequency - 1;
        usbTimerPeriodW_USB_CTR(udev, timer, period);
        usbTimerPulseWidthW_USB_CTR(udev, timer, period / 2);
@@ -238,13 +238,27 @@ int main (int argc, char **argv)
        usbTimerDelayW_USB_CTR(udev, timer, 0);
        usbTimerControlW_USB_CTR(udev, timer, 0x1);

-       usbScanStart_USB_CTR(udev, count, 0, frequency, 0);
+       usbScanStart_USB_CTR(udev, count, 0, frequency, 0, 0);
        flag = fcntl(fileno(stdin), F_GETFL);
        fcntl(0, F_SETFL, flag | O_NONBLOCK);
         j = 0;
+  int bytesPerSample = 32;  /* 4 channels, 8 bytes per channel */
+  int currentPoint = 0;
        do {
           ret = usbScanRead_USB_CTR(udev, count, scanList.lastElement, data);
-         printf("Scan: %d  samples read: %d\n", j++, ret);
+    int numSamples = ret/bytesPerSample;
+         printf("Scan: %d  samples read: %d\n", j++, numSamples);
+
+         for (counter = 0; counter < 4; counter++) {
+           offset = counter*4;
+            counter_data[counter] =  (uint64_t) data[offset];
+           counter_data[counter] += ((uint64_t) (data[offset+1] & 0xffff)) << 16;
+           counter_data[counter] += ((uint64_t) (data[offset+2] & 0xffff)) << 32;
+           counter_data[counter] += ((uint64_t) (data[offset+3] & 0xffff)) << 48;
+         }
+         printf("Scan point: %d     %lld  %lld  %lld  %lld\n", currentPoint, (long long) counter_data[0], (long long) counter_data[1],
+                (long long) counter_data[2], (long long) counter_data[3]);
+                currentPoint += numSamples;
        } while (!isalpha(getchar()));
        fcntl(fileno(stdin), F_SETFL, flag);
        usbScanStop_USB_CTR(udev);

I tested frequency = 100, 1000, 10000, 100000 and they all worked. This is the last bit of output at frequency=100000

Scan: 26177  samples read: 16
Scan point: 418832     4188320  41884  0  0
Scan: 26178  samples read: 16
Scan point: 418848     4188480  41885  0  0
Scan: 26179  samples read: 16
Scan point: 418864     4188640  41887  0  0
Scan: 26180  samples read: 16
Scan point: 418880     4188800  41888  0  0
Scan: 26181  samples read: 16
Scan point: 418896     4188960  41890  0  0
Scan: 26182  samples read: 16
Scan point: 418912     4189120  41892  0  0
Scan: 26183  samples read: 16
Scan point: 418928     4189280  41893  0  0
Scan: 26184  samples read: 16
Scan point: 418944     4189440  41895  0  0
Scan: 26185  samples read: 16
Scan point: 418960     4189600  41896  0  0
Scan: 26186  samples read: 16
Scan point: 418976     4189760  41898  0  0
Scan: 26187  samples read: 16
Scan point: 418992     4189920  41900  0  0

I fixed your printf to be actual number of samples, you were printing number of bytes but calling it number of samples.

Note that I am using a 1 MHz timer into channel 0, so counter 1 should be scan point * 10, which it is.

This appears to be a different behavior than I was observing in my MCS application, where I am counting for a preset number of counts (not continuous). I am explictly asking for 128 readings per buffer, rather than 0, but both result in 512 bytes being returned.

There are a couple of issues with the approach taken in the test program:

MarkRivers commented 4 years ago

Hi Warren, I've made a lot of progress. I made a new version of test-usb-ctr.c. https://github.com/epics-modules/measComp/blob/master/measCompApp/Linux_Drivers/test-usb-ctr.c. It has these changes/improvements:

I have tested this with a number of combinations of numCounters and numBanks, and it works even 256 is not evenly divisible by numCounters * numBanks. In that case packet_size is somewhat less than 512.

Here is the output when numCounters=7, numBanks=4, and count=9000.

It works fine, the number of counts is exactly as expected (10 times point number), and it stops after the last point as it should.


C
Testing continuous scan input
Connect Timer 1 to Counter 1
packet_size=252
samplesPerPacket=9 count=9000
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 0  bytes read: 504 samples read: 9
Scan point: 0     0   0   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 1  bytes read: 504 samples read: 9
Scan point: 9     90   1   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 2  bytes read: 504 samples read: 9
Scan point: 18     180   2   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 3  bytes read: 504 samples read: 9
Scan point: 27     270   3   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 4  bytes read: 504 samples read: 9
Scan point: 36     360   4   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 5  bytes read: 504 samples read: 9
Scan point: 45     450   5   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 6  bytes read: 504 samples read: 9
Scan point: 54     540   6   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
...

Scan point: 8937     89370   894   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 994  bytes read: 504 samples read: 9
Scan point: 8946     89460   895   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 995  bytes read: 504 samples read: 9
Scan point: 8955     89550   896   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 996  bytes read: 504 samples read: 9
Scan point: 8964     89640   897   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 997  bytes read: 504 samples read: 9
Scan point: 8973     89730   898   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 998  bytes read: 504 samples read: 9
Scan point: 8982     89820   898   0   0   0   0   0
usbScanRead_USB_CTR: number of bytes transferred = 504, nbytes = 512
Scan: 999  bytes read: 504 samples read: 9
Scan point: 8991     89910   899   0   0   0   0   0

These are my changes:


corvette:measComp/measCompApp/Linux_Drivers>git diff -w -b  21b3e51cada279ac3755ee1b91bc1ec9ce31b531 test-usb-ctr.c
diff --git a/measCompApp/Linux_Drivers/test-usb-ctr.c b/measCompApp/Linux_Drivers/test-usb-ctr.c
index b1a8ac4..e1e2986 100644
--- a/measCompApp/Linux_Drivers/test-usb-ctr.c
+++ b/measCompApp/Linux_Drivers/test-usb-ctr.c
@@ -63,8 +63,11 @@ int main (int argc, char **argv)
   uint8_t debounce;

   int count;
+  int numCounters=4;
   int counter;
+  int numBanks=4;
   int bank;
+  int packet_size;
   int offset;
   double frequency;
   double timer_frequency;
@@ -73,7 +76,7 @@ int main (int argc, char **argv)
   CounterParams counterParameters[8];
   ScanList scanList;
   uint16_t data[32000];
-  uint64_t counter_data[4];
+  uint64_t counter_data[8];

   udev = NULL;

@@ -159,17 +162,17 @@ int main (int argc, char **argv)
         frequency = 1000;  // scan rate at 1000 Hz

         // Set up the scan list (use 4 counter 0-3)
-       for (counter = 0; counter < 4; counter++) {
-         for (bank = 0; bank < 4; bank++) {
-           scanList.scanList[4*counter + bank] = (counter & 0x7) | (bank & 0x3) << 3 | (0x2 << 5);
+        for (counter = 0; counter < numCounters; counter++) {
+          for (bank = 0; bank < numBanks; bank++) {
+            scanList.scanList[numBanks*counter + bank] = (counter & 0x7) | (bank & 0x3) << 3 | (0x2 << 5);
           }
         }
-       scanList.lastElement = 15;
+        scanList.lastElement = numCounters * numBanks -1;
         usbScanConfigW_USB_CTR(udev,scanList.lastElement, scanList);
         usbScanConfigR_USB_CTR(udev, scanList.lastElement, &scanList);

         // set up the counters
-       for (counter = 0; counter < 4; counter++) {
+        for (counter = 0; counter < numCounters; counter++) {
           usbCounterSet_USB_CTR(udev, counter, 0x0);       // set counter to 0
           usbCounterModeW_USB_CTR(udev, counter, 0x0);
           usbCounterOptionsW_USB_CTR(udev, counter, 0);    // count on rising edge
@@ -192,35 +195,40 @@ int main (int argc, char **argv)
         usbTimerControlW_USB_CTR(udev, timer, 0x0);

         for (i = 0; i < count; i++) {
-         for (counter = 0; counter < 4; counter++) {
-           offset = i*16 + counter*4;
-            counter_data[counter] =  (uint64_t) data[offset];
-           counter_data[counter] += ((uint64_t) (data[offset+1] & 0xffff)) << 16;
-           counter_data[counter] += ((uint64_t) (data[offset+2] & 0xffff)) << 32;
-           counter_data[counter] += ((uint64_t) (data[offset+3] & 0xffff)) << 48;
+          for (counter = 0; counter < numCounters; counter++) {
+            offset = i*numCounters*numBanks + counter*numBanks;
+            counter_data[counter] = 0;
+            for (bank=0; bank<numBanks; bank++) {
+              counter_data[counter] +=  (uint64_t) (data[offset+bank] & 0xffff) << (16*bank);
             }
-         printf("Scan: %d     %lld  %lld  %lld  %lld\n", i, (long long) counter_data[0], (long long) counter_data[1],
-                (long long) counter_data[2], (long long) counter_data[3]);
+          }
+          printf("Scan: %d     ", i);
+          for (counter=0; counter<numCounters; counter++) {
+            printf("%lld   ", (long long)counter_data[counter]);
+          }
+          printf("\n");
         }
         break;
       case 'C':
         printf("Testing continuous scan input\n");
         printf("Connect Timer 1 to Counter 1\n");
         count = 0;          // set to 0 for continuous scan.  Returns 256 samples per read
-       frequency = 100;    // scan rate at 100 Hz
+        frequency = 100000;    // scan rate at 100 Hz
+        numCounters = 7;
+        numBanks = 4;

         // Set up the scan list (use 4 counter 0-3)
-       for (counter = 0; counter < 4; counter++) {
-         for (bank = 0; bank < 4; bank++) {
-           scanList.scanList[4*counter + bank] = (counter & 0x7) | (bank & 0x3) << 3 | (0x2 << 5);
+        for (counter = 0; counter < numCounters; counter++) {
+          for (bank = 0; bank < numBanks; bank++) {
+            scanList.scanList[numBanks*counter + bank] = (counter & 0x7) | (bank & 0x3) << 3 | (0x2 << 5);
           }
         }
-       scanList.lastElement = 15;
+        scanList.lastElement = numCounters * numBanks - 1;
         usbScanConfigW_USB_CTR(udev,scanList.lastElement, scanList);
         usbScanConfigR_USB_CTR(udev, scanList.lastElement, &scanList);

         // set up the counters
-       for (counter = 0; counter < 4; counter++) {
+        for (counter = 0; counter < numCounters; counter++) {
           usbCounterSet_USB_CTR(udev, counter, 0x0);       // set counter to 0
           usbCounterModeW_USB_CTR(udev, counter, 0x0);
           usbCounterOptionsW_USB_CTR(udev, counter, 0);    // count on rising edge
@@ -230,7 +238,7 @@ int main (int argc, char **argv)

         // set up the timer to generate some pulses
         timer = 1;
-       timer_frequency = 100.;
+        timer_frequency = 10000.;
         period = 96.E6/timer_frequency - 1;
         usbTimerPeriodW_USB_CTR(udev, timer, period);
         usbTimerPulseWidthW_USB_CTR(udev, timer, period / 2);
@@ -238,13 +246,38 @@ int main (int argc, char **argv)
         usbTimerDelayW_USB_CTR(udev, timer, 0);
         usbTimerControlW_USB_CTR(udev, timer, 0x1);

-       usbScanStart_USB_CTR(udev, count, 0, frequency, 0);
+        int bytesPerReading = numCounters * numBanks * 2;  /* 4 channels, 8 bytes per channel */
+        packet_size = ((512 / bytesPerReading) * bytesPerReading)/2;
+        printf("packet_size=%d\n", packet_size);
+        int readingsPerPacket = packet_size*2/bytesPerReading;
+        /* Set a finite count which must be a multiple of bytes per sample */
+        count = 1000 * readingsPerPacket;
+        printf("samplesPerPacket=%d count=%d\n", readingsPerPacket, count);
+
+        usbScanStart_USB_CTR(udev, count, 0, frequency, packet_size, 0);
         flag = fcntl(fileno(stdin), F_GETFL);
         fcntl(0, F_SETFL, flag | O_NONBLOCK);
         j = 0;
+        int currentPoint = 0;
         do {
-          ret = usbScanRead_USB_CTR(udev, count, scanList.lastElement, data);
-         printf("Scan: %d  samples read: %d\n", j++, ret);
+          ret = usbScanRead_USB_CTR(udev, 0, scanList.lastElement, data);
+          int numReadings = ret/bytesPerReading;
+          printf("Scan: %d  bytes read: %d samples read: %d\n", j++, ret, numReadings);
+
+          for (counter = 0; counter < numCounters; counter++) {
+            offset = counter*numBanks;
+            counter_data[counter] = 0;
+            for (bank=0; bank<numBanks; bank++) {
+              counter_data[counter] +=  (uint64_t) (data[offset+bank] & 0xffff) << (16*bank);
+            }
+          }
+          printf("Scan point: %d     ", currentPoint);
+          for (counter=0; counter<numCounters; counter++) {
+            printf("%lld   ", (long long)counter_data[counter]);
+          }
+          printf("\n");
+          currentPoint += numReadings;
+          if (currentPoint == count) break;
         } while (!isalpha(getchar()));
         fcntl(fileno(stdin), F_SETFL, flag);
         usbScanStop_USB_CTR(udev);
MarkRivers commented 4 years ago

My EPICS application is almost done. It works in scaler mode and in MCS mode with both internal channel advance and external channel advance (CLKI input). I have tested it up to 333 kHz scan frequency reading 8 channels in 32-bit mode.

The only thing that is not working is external channel advance with prescaling. In this mode I use one of the counters as a frequency divider. That is working fine on Windows, but I can't seem to figure out how to do it in Linux.

My high-level software does it with these calls using the UL API:

  if ((channelAdvance == mcaChannelAdvance_External) && (prescale > 1) ) {
    mode = OUTPUT_ON | COUNT_DOWN_OFF | RANGE_LIMIT_ON;
    status = cbCLoad32(boardNum_, OUTPUTVAL0REG0+prescaleCounter, 0); 
    status = cbCLoad32(boardNum_, OUTPUTVAL1REG0+prescaleCounter, prescale-1); 
    status = cbCLoad32(boardNum_, MAXLIMITREG0+prescaleCounter, prescale-1); 
    status = cbCConfigScan(boardNum_, prescaleCounter, mode, CTR_DEBOUNCE_NONE, CTR_TRIGGER_BEFORE_STABLE, 
                           CTR_RISING_EDGE, CTR_TICK20PT83ns, 0);

On Linux cbLoad32 and cbConfigScan are implemented as follows:

int mcUSB_CTR::cbCLoad32(int RegNum, ULONG LoadValue)
{
    static const char *functionName = "cbCLoad32";

    uint8_t index=0;
    int reg = (RegNum/100)*100;
    int counterNum = RegNum - reg;
    switch (reg) {
      case OUTPUTVAL0REG0:
      case OUTPUTVAL1REG0:
          if (reg == OUTPUTVAL1REG0) index=1;
          asynPrint(pasynUser_, ASYN_TRACEIO_DRIVER, 
              "%s::%s calling usbCounterOutValuesW_USB_CTR counterNum=%d, index=%d, value=%d\n",
              driverName, functionName, counterNum, index, LoadValue);
          usbCounterOutValuesW_USB_CTR(deviceHandle_, counterNum, index, LoadValue);
          break;
      case MINLIMITREG0:
      case MAXLIMITREG0:
          if (reg == MAXLIMITREG0) index=1;
          asynPrint(pasynUser_, ASYN_TRACEIO_DRIVER, 
              "%s::%s calling usbCounterLimitValuesW_USB_CTR counterNum=%d, index=%d, value=%d\n",
              driverName, functionName, counterNum, index, LoadValue);
          usbCounterLimitValuesW_USB_CTR(deviceHandle_, counterNum, index, LoadValue);
          break;
      case LOADREG0:
      case LOADREG1:
          if (reg == LOADREG1) index=1;
          asynPrint(pasynUser_, ASYN_TRACE_ERROR, 
              "%s::%s got LOADREG0 or LOADREG1 command, don't know what to do counterNum=%d, index=%d, value=%ul\n",
              driverName, functionName, counterNum, index, LoadValue);
          // I don't know what function should be called for this
          //usbCounterOutValuesW_USB_CTR(deviceHandle_, counterNum, index, LoadValue);
          break;      
    }
    return NOERRORS;
}

int mcUSB_CTR::cbCConfigScan(int CounterNum, int Mode,int DebounceTime,
                             int DebounceMode, int EdgeDetection,
                             int TickSize, int MappedChannel)
{
    CounterParams params;
    static const char *functionName = "cbCConfigScan";

    params.counter = CounterNum;

    params.counterOptions = 0;
    if (Mode & CLEAR_ON_READ)  params.counterOptions |= USB_CTR_CLEAR_ON_READ;
    if (Mode & NO_RECYCLE_ON)  params.counterOptions |= USB_CTR_NO_RECYCLE;
    if (Mode & COUNT_DOWN_ON)  params.counterOptions |= USB_CTR_COUNT_DOWN;
    if (Mode & RANGE_LIMIT_ON) params.counterOptions |= USB_CTR_RANGE_LIMIT;
    if (EdgeDetection & 
        CTR_FALLING_EDGE)      params.counterOptions |= USB_CTR_FALLING_EDGE;

    params.modeOptions = 0;
    params.modeOptions |= (TickSize << 4);

    params.gateOptions = 0;
    if (Mode & GATING_ON)   params.gateOptions |= 1;
    if (Mode & INVERT_GATE) params.gateOptions |= 2; // Need to check the logic

    params.outputOptions = 0;
    if (Mode & OUTPUT_ON)                 params.outputOptions |= 1;
    if (Mode & OUTPUT_INITIAL_STATE_HIGH) params.outputOptions |= 2;

    params.debounce = 0;
    if (DebounceMode == CTR_TRIGGER_BEFORE_STABLE) params.debounce |= 0x10;
    if (DebounceTime != CTR_DEBOUNCE_NONE)         params.debounce |= (0x0f | (DebounceTime >> 1));

    asynPrint(pasynUser_, ASYN_TRACEIO_DRIVER, 
        "%s::%s calling usbCounterParamsW_USB_CTR CounterNum=%d, "
        "params.counterOptions=0x%x, params.gateOptions=0x%x, params.outputOptions=0x%x, params.debounce=0x%x\n",
        driverName, functionName, CounterNum, params.counterOptions, params.gateOptions, params.outputOptions, params.debounce);
    usbCounterParamsW_USB_CTR(deviceHandle_, CounterNum, params);

    return NOERRORS;
}

Do you see anything that looks wrong?

I can add a new "f" command to test-usb-ctr.c that implements a simple frequency counter so you can try it.

wjasper commented 4 years ago

I changed things up a little taking your comments into account. Instead of passing packet_size, which is actually a little tricky and want to leave it up to the API, I'm passing scanListSize which is lastElement +1 or the size of the scan queue. From there I can calculate packet_size or set it to 255 when count != 0. I'm setting count= 0 for continuous scan. Try out my examples now. In continuous mode, the size of the scan queue divides packet_size. I think I'm getting what you are getting in the 2 examples you posted above, and things look robust. Yes, please add a 'f' option for a simple frequency counter so I can try it out.

MarkRivers commented 4 years ago

This is from your new code in usbScanStart_USB_CTR().

  if ( count != 0 ) {
    packet_size = wMaxPacketSize/2 - 1;
  } else {
    if ((scanListSize < 1 || scanListSize > 33)) {
      perror("usbScanStart_USB_CTR: scanListSize out of range [1-33].");
      return;
    }
    packet_size = (((wMaxPacketSize/scanListSize)*scanListSize) / 2) - 1;
  }

This won't work for my application for 3 reasons: 1) I need to be able to implement SINGLEIO from the UL interface. This requires explicitly setting packet_size to a single reading, i.e. scanListSize. I need to do this for the scaler mode where count=0. 2) In MCS mode I need to set count=numPoints (non-zero) and set the packet_size it returns an integer number of readings. 3) In MCS mode at low scan rates I will want to set packet_size even smaller so I get more rapid updates for user feedback.

All 3 of these are possible with my implementation, but not with yours.

I have created a simple test program for the frequency divider using the UL API. To my surprise it works not only on Windows, but also on Linux. So now I know that it can be done on Linux, I have to figure out how my test program differs from my EPICS driver, where it is not working. So I don't think I need to add it to test-usb-ctr.c. Thanks.

MarkRivers commented 4 years ago

I implemented #3 above yesterday. It makes a dramatic improvement in the user-friendliness of the software. I am reading 8 counters in 32-bit mode, so each read is 16 16-bit words. Previously if the scan frequency was 1 Hz the plots could not update until 16 seconds had elapsed, because the packet_size was 256 words. I changed my high-level code so that if the scan frequency is less than 20 Hz it uses the SINGLEIO option, i.e. packet_size=16. Now the plot updates faster than 1 Hz in all circumstances.

wjasper commented 4 years ago

Hi Mark, I see what you mean, the way I was implementing it was incomplete. I have rewritten the code to pass a scanData structure that contains ALL the information for a data scan. Now just need to pass one data structure instead of a lot of parameters. Also added a mode variable for SINGLEIO as well as continuous modes and will adjust packet size for small frequencies below 10 Hz automatically. I think this is a lot more user friendly and still covers all the cases you outline. Much thanks for your suggestions.

MarkRivers commented 4 years ago

In my pull request I said the API was not working for me. That is not true, I misunderstood the meaning of CONTINUOUS_SCAN (renamed by me to USB_CTR_CONTINUOUS_SCAN). That sounds like one wants to scan forever, i.e. count=0. What it really means is CONTINUOUS_READOUT, which I think would be a better name to use. In my application I always want CONTINUOUS_READOUT, in both scaler and MCS mode, so I now set this flag.

It is basically working, but it is crashing under circumstances where it did not crash before so I need to figure that out.

I like the new API, it lets me have fewer class member variables in my code too. I do have a few suggestions on how to make it more flexible so I will add those to my pull request once I test them.

wjasper commented 4 years ago

I've merged the changes. I'm forcing packet_size to be an uint8_t when passing to the the device, so there could be weird behavior if the user sets it to something strange with USB_CTR_FORCE_PACKET_SIZE that could be hard to track down, but most users should not be mucking with packet_size. I also added an -f option to count frequency. Need to wait 2 seconds for things the settle down or you won't always read the correct frequency. I'm going to close out this issue as I think most of the issues have been addressed.