-
Notifications
You must be signed in to change notification settings - Fork 333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for dreamsourcelab DSLogic U3Pro16 #170
base: master
Are you sure you want to change the base?
Support for dreamsourcelab DSLogic U3Pro16 #170
Conversation
deinterleave_buffer is used to convert the received samples from DSLogic device (which is a sequence of 64-bit words in round-robin for each channel, to the format sigrok is expecting (sequence of words containing the value for given channel as bitmask). The existing implementation looped over every single bit, making it inefficient assuming that a typical trace contains little signal changes compared to the sample rate. This change improves the deainterleave function by forward copying 64 words assuming there is no change. Only if there is a change, it searches the individual channels for bit-wise changes.
deinterleave_buffer function as well as sr_session_send is directly executed in the USB receive data callback and therefore blocks any re-submission of new USB transfers. This change is to introduce a worker thread which takes care of calling deinterleave_buffer and sr_session_send while next USB reception can already be processed. Assuming there are 2 or more USB transfers this finally leads to a double buffering effect where the received data is being processed while the next transfer is already triggered/ongoing.
This adds support for U3Pro16 logic analyzer from dreamsource logic. (https://sigrok.org/wiki/DreamSourceLab_DSLogic_U3Pro16) The main difference between existing dslogic devices and U3Pro16 is the support for USB3 (using a Cypress CYUSB3014-BZX) and dreamsource logic also changed the USB API itself. Still, mostly of the driver is re-used and common code and only if diversity is needed there is a runtime check using the api_version enum in the dslogic_profile structure. The device still uses buffered mode per default which doesn't allow long captures especially at high sample rates.
This removes the buffered mode for dslogic as it doesn't give any advantage. The device now operates in stream mode, which anyhow uses the internal memory of dslogic as a buffer
This reverts commit 0118f9a.
During buffered mode the internal memory of the DS device may not be able to capture the amount of samples requested. In that case, reading the samples beyond max captured point will return all random data. This commit reads the number of captures samples from the trigger header. With that information the existing limit_samples variable is overwritten. The change only applies to buffered mode as in streaming mode a overflow is automatically detected.
In case not all 16 channels are enabled, the data for disabled channels will be random. Upper layer applications (like pulseview) will receive this garbage then. (Example: In pulseview when recording with not all channels enabled but then enable the disabled channels, will show this garbage data)
Thanks for this PR! I was trying to port the device a while by attempting to copy the codebase from DSView, but that didn't work out to well. I am currently trying this fork on my U3Pro16 (unfortunately I don't have any older devices to test for legacy), and here are a couple of things I noticed:
|
Right, the U3Pro16 has some HW limits which is a combination of enabled channels and configured sample rate. That's also the reason why in DSView some combinations don't work. pulseview on the other hand is device agnostic, so it has no idea about any combinatorial device constraints. On startup it fetches the supported sample rate from libsigrok and the number of channels, but completely independent from each other. I could add some checks in the driver the disallow some combinations and return an error to the upper application. I guess in sigrok-cli this error is shown but in pulseview it would just get logged on the console/logger but not in the GUI. So at the end, pulseview would need an update as well but since pulseview is device agnostic it would mean that such change has a generic impact on all devices. But let me look into this to see which combinations are allowed.
This is exactly the problem I mentioned above. At 1GHz you can't use all channels (DSView for instance limits it to 8 channels in buffered mode and i guess 3 channels in streaming mode).
Indeed, in this case I stop the ongoing acquisition but don't start the USB transfer to fetch the data. I will change this.
Out of legacy reasons (existing dslogic driver), the default behavior is buffered mode and unfortunately there is no way in pulseview to change it. The option to change it in the driver is there but pulseview is not showing this option, it is filtered in pulseview. If you use sigrok-cli you can switch between the two modes. Actually, i'm currently working on some improvements in pulseview whereas I don't think they can be upstreamed as some of them are just quick hacks. The changes are:
Latest development build is here: https://github.com/christianeisendle/pulseview/releases/tag/pulseview-0.5.4 If i find a bit more time, i might consider upstreaming few of those features once I cleaned up the implementation |
- In case of an incomplete USB transfer, process received data but stop acquisition since upcoming data would lead to an inconsistent capture stream - Call finish_acquisition in case no transfer could be scheduled due to an error
Instead of only using one buffer for deinterleaving in worker thread, this commit uses now a queue: For every finished USB transfer, the data is put into a queue and a new transfer is immediately scheduled. This completely decouples USB transfers from data processing activities which could be quite time consuming, especially the deinterlaving of data with high number of signal changes. The additional buffers needed are dynamically allocated, i.e. as long as memory is available it will allocate new buffers for the data processing or stops the acquisition if no more memory is available. Once a buffer has finished prcessing, the memory will be released immediately.
1f8059e
to
4464954
Compare
Since buffering is now handled in driver, we can always go for continous/streaming mode. Non-continous mode/buffered mode makes only sense in exceptional cases where extremely high sample rates are needed and USB speed is not able to fetch data in time. In any case the max. capture size is limited to internal memory of dslogic (+RLE compression)
… version dynamically
Updated the PR to avoid hard coupling between device type and API versions. Seems that all newly ordered devices come with API version 2, so this change now tries the determine the API version dynamically over a hard coded relationship between model and API Version |
Thanks for your efforts on this. I couldn't open an issue on your repo, so commenting here.. I'm seeing the following issue on my DSLogic Plus, it just hangs at getting trigger. (The FPGA bitstream is the latest one from the DSView github).
|
Ok, my guess is that FPGA bitstream is still the old version, in fact, it seems that FW version (API version 1 and 2) and FPGA bitstream version are completely independent. So it seems that FPGA API is coupled to device while FW API is coupled to FW version. If this assumption is correct, the fix would be straight-forward since the FPGA related functionality is encapsulated in fpga_configure. |
Actually, i just reviewed the code of DSView and i don't see any diversification in dsl_fpga_arm. (well, there is one but it seems to be unrelated) I would propose to dump the content of "setting struct" in dsl_fpga_arm() while running DSView to a file and then load the content of the dump file in libsigrok dsl.c/fpga_configure() and send it there to the FPGA. Assuming this works, we just need to compare the content of setting struct between libsigrok and DSView. Are you able to compile DSView under Linux? If so, i can give you a patch for DSView to dump the settings. |
Thanks, yes please - I'm compiling DSView locally. (I think you should have my email from the sigrok-devel list, so feel free to email over a patch). I also started looking at the USB data in Wireshark, there are some differences between the two, but is taking me a while to reference that back to the code.. Cheers, Jon. |
Hi @dresco, Christian. |
Hi @christianeisendle, can confirm is working well for me now with that last update. I've built your |
This also works for me, using the newest firmware from DSView. I suggest adding new entries to contrib/60-libsigrok.rules:
Thanks for implementing support! |
Thanks, wasn't aware about presence of the rules file - just added U3Pro16 and updated the PR |
Hi, just a quick observation. Have recently noticed that the pre-trigger capture ratio in pulseview isn't working as I would have expected.. For example, when set to 50% pre-trigger with 50M samples @ 10MHz, it only captures ~670ms of pre trigger data (instead of the expected 2.5s). The ratio seems dependent on the amount of data, 5M samples @ 1Mhz is okay, but at higher speeds (or longer duration) there is even less pre-trigger data stored. I think the distinction is that it works fine when the selected combination of data rate & duration is not using RLE. Just wondering if this is known / expected behaviour for these devices? |
Hi @dresco and @christianeisendle what happened? Is this PR stalled because this issue with pre-trigger? I got a DSLogic U3Pro16 and want to use it with PulseView instead of DSView. Thank you very much! |
Hi @dresco, @acassis, |
Hi @christianeisendle very nice! Sorry to hurry up you :-) At least it is working fine for up to 8 inputs. |
Hi @christianeisendle Thanks for picking this back up again. Just to confirm I can recreate this issue with fewer channels though (perhaps is more apparent with the lower hw spec of my DSLogic Plus)? Here are some example images (triggering off channel 2, with 50% pre-trigger); 10M samples @ 10MHz on 4 channels, 500ms pre-trigger data as expected 50M samples @ 50MHz on 4 channels, ~134ms pre-trigger data 50M samples @ 50MHz on 8 channels, ~67ms pre-trigger data And this pattern remains consistent, moving from 8 to 16 channels I get ~33ms of pre-trigger data. Changing the sampling rate also changes the pre-trigger duration (half the sample rate = double the pre-trigger time) etc. Has been a while since I first looked at this, but my suspicion at the time was that it happened if the selected sample rate and duration required RLE.. |
Hi @dresco, thanks for sharing your findings. To me it really looks like a hardware limitation. I did a comparison with DSView to see how they handle it there. Interestingly, they don't allow changing pre-trigger in streaming mode at all, it's only supported in buffered mode. Streaming mode (SR_CONF_CONTINUOUS in the driver) is enabled by default but back in January it wasn't possible to change that in pulseview. Coincidentally, @gsigh fixed that in Feb: sigrokproject/pulseview@a00c1b6 So i built pulseview from latest master, disabled continous mode in device settings and after that pre-buffering works as expected: Probably there are ways to fix it also for continous/streaming mode but since DSView also doesn't offer this option, i would be difficult to find out how to do it without reverse engineering the device. On the other hand, the guys from dreamsourcelab might have had a good reason to allow it only for buffered mode... |
Thanks for looking at this @christianeisendle. I rebuilt with the latest upstream pulseview, and can confirm that it works somewhat better with continuous mode disabled. It does still reduce the amount of pre-trigger data as I increase the frequency or number of channels even further, but agree that it looks like a device limitation. This prompted me to take another look at DSView - and it shows the same behaviour of changing the amount of pre-trigger data (in buffer mode) as these settings are changed. Also, their UI logic seems a bit buggy, but DSView does actually attempt to adjust the maximum selectable trigger position as the frequency etc increases. |
i can think of implementing this pre-buffering feature in the driver itself. For that we would need to reserve significant memory in the driver and store the pre-buffered data there (using a ring-buffer) and only if the driver detects an edge change it pushes the pre-buffer + the streaming data which comes after the trigger to the upper layer (pulseview/sigrok-cli). Currently pre-buffering is done in the device and since it has limited memory it will obviously start to limit the max. pre-buffer length depending on number of channels and sampling frequency. I was just thinking about one option which could make sense: In case upper layer sets a prebuffer > 0, the driver implicitly enables buffered mode (with the drawback of having no live capturing after the has triggered). Let me know what you think about this option. |
I'd worry that this might not be very intuitive? Honestly, now knowing it's a device limitation (with similar results in vendor software), perhaps it just needs documenting somewhere as expected behaviour? |
i just tried the branch with my U3Pro16, it works beautifully! besides the documentation issue for the pre-trigger, is there something keeping this from being mainlined? |
Bought my self a U3Pro16 as xmas pressent a year or two ago and I have always wanted to use it with sigrok-cli, PulsView etc... and now I soon will!.. |
For whatever reason, many PR's appear stalled without upstream review, so you might do well to build from source (if you aren't already). The sigrok-util repo makes building fairly straightforward, just needs a small update to pull libsigrok from the right place. Fwiw, these are my changes to build for Linux..
|
Still not merged with main, I guess? About building from source; I will need to build libsigrok to get the device to work in pulseview or do I also need to build pulseview? Side note; someone should update the list of supported hardware.. this device still says planned, but that's not true... :D |
Is there a guide how to build full sigrok for windows? Looked at the existing, but I get the impression that's it's for Linux only? Anyone knows? |
Build for windows is supported via cross compile toolchain under Linux.
This worked fine so far for me, I didn't manage to compile natively.
Just follow the instructions given in the README i
http://sigrok.org/gitweb/?p=sigrok-util.git;a=tree;f=cross-compile/mingw
You may need to adjust the repository and/or branch in sigrok-cross-mingw
Am 23. November 2022 23:09:31 schrieb carlberg74 ***@***.***>:
… Still not merged with main, I guess?
About building from source; I will need to build libsigrok to get the
device to work in pulseview or do I also need to build pulseview? (+ want
to run pulseview on Windows)
Side note; someone should update the list of supported hardware.. this
device still says planned, but that's not true... :D
Is there a guide how to build full sigrok for windows? Looked at the
existing, but I get the impression that's it's for Linux only? Anyone knows?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
|
Has there been any movement getting this reviewed? Seems like a shame to leave it here to fester. |
fwiw I believe there is some effort going into triage of outstanding issues and PR's. There is an ongoing desire for more review input on code submissions. As a random nobody, my thoughts on this PR are that it obviously works well for multiple people (myself included), but @christianeisendle don't know whether you still have the desire/time to pursue this? I don't mind having a go at it if not? |
I feel we need to bring it to the official mailing list, not sure if the maintainers follow all pull-requests on github. Let me reach out to others via the mailing list |
Cool thanks. There seems to be more developer activity on the IRC channel, don't know whether that's another option for you? |
Is anyone still working on this? Sigrok is also missing U3Pro32 support. I will have some time in the next few weeks to do some work on this. |
Ok, I did a bit of looking at protocol and there seems to be missing only mass write DSView:
Sigrok:
Note, I am working on U3Pro32, but it seems to be pretty similar. Also, I am using fpga code from commit |
It seems to be outputing data, but, only after 4min, 26sec, for some reason |
@nemanjan00 any update? |
Never managed to get it to work properly. Gave up. Just used slower LA I own with sigrok |
@christianeisendle Thanks for your work on this! As an owner of the V2 firmware DLogic Plus I'm excited to see that sigrok + V2 firmware is possible. Does this PR support other V2 devices as it stands, or would some modifications need to be made? |
Is this PR still being worked on? I'm in the same situation as @MabezDev , I have a DSLogic Plus "Pango Variant" (which I believe is what everyone is referring to as "V2", please correct me if I'm wrong). @christianeisendle Would this PR introduce support for that version? I see that there's the addition of a new firmware for the U3Pro16, but looking into the DSView repository, I see that there're at least two new versions of firmware for the DSLogic Plus "Pango" (I think):
Would it be sufficient to arrange the code so that the correct firmware is referenced and uploaded, or is there something else to consider? |
This PR adds support for DSLogic U3Pro16 to libsigrok.
It is a logic analyzer with up to 1GHz sample rate 16 channels, buffered and streaming mode support and USB3 interface.
The support has been added to existing dreamsourcelab-dslogic driver with some runtime diversity since the U3Pro16 has a new API which is not fully backward-compatible with the legacy dreamsource logic devices.
In addition, the generic part of the driver got some performance updates in the deinterleaving function and a dedicated worker thread to process/deinterlave samples while processing already the next USB transfer.