The issue was due to a bug in the DualDataBuffer class which has incorrectly using a lock + conditional variable mechanism, and not protecting the shared variable. Moreover, the presence of that dual buffer was preventing the upstream 100-sample Fifo to do its job. So, I remove completely the DualDataBuffer from the data path and left only the Fifo. I also added an additional 100-sample Fifo to the meta-data path as well, which didn't have any buffer.
Additionally to that fix, I also extended the SmurfPacket class and added a version which doesn't copy the data from the underlaying frame into std::vectors, and instead just reference the values directly from the frames. This improves the performance when passing the data to an external transmitter device. The previous behavior of SmurfPacket is still available, and they can be selected using policy classes; but I set the default to be the zero copy version. This change will be transparent to the user.
Another optimization was to instantiated the FIFOs only when an external transmitter class is defined. Otherwise, the FIFO are not needed, and therefore not instantiated.
In order to have more debug information in the future about drop frames (if any) I added extra variable to the rogue tree:
A command ClearCnt is available to clear the counters.
I added these two counters to the BaseTransmitter class (the previous drop counters are still present):
dataFrameCnt: Number of data frame received
metaFrameCnt: Number of metadata frame received
I also found and fixed a couple of additional bug:
a small bug in the Rogue FIFO class, which as holding one additional frame respect to the requested size. See https://github.com/slaclab/rogue/pull/777 for details.
Finally, I added a new test script validate_base_tx.py to validate the correct functionality of the base transmitter module. I added this script to CI scripts.
And, of course, the documentation was updated.
Does this PR break any interface?
[ ] Yes
[X] No
Which interface changed?
Although no interface was broken, this PR adds new variables with more debug information, as described above.
NOTE: This PR fixes the bad PR https://github.com/slaclab/pysmurf/pull/641 which was reverted as it included changes from the
developbranch.Issue
This PR resolves ESCRYODET-813.
Description
This PR fixes the bug reported by SO user in https://github.com/simonsobs/smurf-issues/issues/31.
Using the pysmurf-custom-transmitter-example I was able to reproduce the dropped frames on the transmitter.
The issue was due to a bug in the
DualDataBufferclass which has incorrectly using a lock + conditional variable mechanism, and not protecting the shared variable. Moreover, the presence of that dual buffer was preventing the upstream 100-sample Fifo to do its job. So, I remove completely theDualDataBufferfrom the data path and left only the Fifo. I also added an additional 100-sample Fifo to the meta-data path as well, which didn't have any buffer.Additionally to that fix, I also extended the
SmurfPacketclass and added a version which doesn't copy the data from the underlaying frame intostd::vectors, and instead just reference the values directly from the frames. This improves the performance when passing the data to an external transmitter device. The previous behavior ofSmurfPacketis still available, and they can be selected using policy classes; but I set the default to be the zero copy version. This change will be transparent to the user.Another optimization was to instantiated the FIFOs only when an external transmitter class is defined. Otherwise, the FIFO are not needed, and therefore not instantiated.
In order to have more debug information in the future about drop frames (if any) I added extra variable to the rogue tree:
MaxDepth: Maximum depth of the FifoSize: Number of elements in the FifoFrameDropCnt: Number of dropped framesClearCntis available to clear the counters.BaseTransmitterclass (the previous drop counters are still present):dataFrameCnt: Number of data frame receivedmetaFrameCnt: Number of metadata frame receivedI also found and fixed a couple of additional bug:
SmurfProcessor code, related to some locks being shadowed. See https://github.com/slaclab/pysmurf/commit/456b7b671491c054fc902bdacade06c838c3f107 for details.Finally, I added a new test script
validate_base_tx.pyto validate the correct functionality of the base transmitter module. I added this script to CI scripts.And, of course, the documentation was updated.
Does this PR break any interface?
Which interface changed?
Although no interface was broken, this PR adds new variables with more debug information, as described above.