Driver for select Tektronix TDS, TPS, and TBS low-end models #213
Conversation
The vendor got back to me and was unable to provide such a list |
|
Hello @byteit101 , |
|
Ah, I had assumed it would be when it was merged, but in case, not, I squashed everything down. If you are looking for the previous history, it kept that on my tek_ocp2k5 branch |
|
Thats a bit too much squash x) Good practice here it to have at least a first "Initial driver skeleton" commit, and then only the actual code. You can of course have more commits, just ensure that hey are individually build-able to allow bisection. Also check that your code matches the project code styling. Cannot be to much more specific, I'm also a new guy around here. |
|
Ah, well, I added it in a different folder and renamed it late in the game, and attempting to re-create a fake history sounds tedious. I did format it (except the table) with @knarfS 's clang-format patch/branch, let me know if something isn't quite right |
|
I have pulled it slightly apart again, though also renamed it to |
|
After review from |
Rename the actual_channel_name identifier to curr_channel_name. This was introduced in commit 17a82e8.
Whitespace change exclusively, no change in behaviour. Reduce the level of indentation of continuation lines in common SCPI code's header and source files. See a whitespace ignoring diff for review. A TAB is sufficient to represent a level of indentation. More than this, up to four of them, or additional SPACE after TAB is excessive. Drop it.
Rename retry count and delay parameters which exclusively were used in the "*OPC?" result getter. Prefer while-not-done over counted for loop. Only return success when SCPI communication was successful and the value is true-ish (communication was not checked explicitly before, instead relied on internal implementation details of the bool getter).
Rephrase the scpi_dev_inst_new() routine for code style and robustness. Don't hide assignments in declaration blocks. Unobfuscate string compare and reduce indentation in the prefix match loop. Don't mix function call and result check and flow control in a single statement. Don't access uninitialized data in error paths (resource release call).
Avoid tight read loops when a SCPI device is busy creating the very response that the host is trying to receive. Add a new 'read_pause_us' member next to the existing 'read_timeout_us'. Default to zero for maximum backwards compatibility. Pause between read calls when an attempt sees zero receive data. Adjust the internal scpi_get_data() and the public sr_scpi_get_block() routines, which other SCPI routines are built upon.
Adjust the internal scpi_get_data() routine which collects a SCPI response message. Add in/out decorations to Doxygen comments. Group instructions that execute a logical sequence which belongs together (eliminate clutter). Use size types for string buffer lengths.
Move code which keeps growing a string buffer before accumulating more receive data out of the scpi_read_data() routine and into a separate helper. The block reader wants to share this logic.
SCPI transports construct connection ID strings for display purposes. The SCPI over TCP implementation did present the essential details (IP address, TCP port), but yielded an output format which does not work as an SR_CONF_CONN value. The colon separator even breaks CLI option parsing. Separate IP address and TCP port by means of a slash. Generate a connection ID text which matches all other transports, and also works for the creation of new device instances.
The SCPI protocol usually communicates requests (commands, queries) and responses in ASCII format. But also supports the concept of data blocks which carry raw byte sequences. Checking for CR or LF characters in response text does not combine well with transmission of raw bytes. Introduce a "block ends" method for SCPI transports. Common code is aware when a data block is received, and where it ends. That's when transports can re-consider their end-of-message condition for the remaining reception of that very response. The transport method sees all previously accumulated receive data that follows the data block. Implement that "block ends" method in the serial transport, which motivated the introduction of the method. Void falsely detected "is complete" conditions that originate from seeing CR or LF in byte data. Other transports derive their respective end-of-message condition from other information which is not affected (EOI signals, VISA status bits, TCP with length specs in headers before data, USB transfer flags, etc).
Fix typos in comments, and update comments which went stale in earlier commits. Mention that "indefinite length block" is not supported in the routine's Doxygen comment for callers' awareness. Address tiny style nits while we are here: Asterisk placement in pointer declarations. Braces around complex multi-line statements. Drop empty lines in groups of statements that belong together. Un-clutter their comments, discuss the whole group in one text.
Rephrase the sr_scpi_get_block() routine to increase readability and
robustness. Address data type issues (signedness). Rename variables to
better reflect their purpose. Concentrate more comments before larger
instruction sequences. Mention instructions' purpose first before more
detailled discussion, to speedup reader's navigation in the complex
logic. Reword the discussion of definite and indefinite length blocks.
Add more diagnostics. Although the text diff is huge, the spirit of
the previous implementation is kept.
Rephrase loops when the motivation for their phrasing was uncertain:
Replace "if (cond) do { ... } while (cond)" with just "while (cond)".
Which reduces nesting and indentation. Use "while" instead of "do while"
when it's obvious that the body does initially execute. It's believed
that behaviour remains identical.
This commit also changes behaviour: Re-arm timeouts more often after
receive data was seen. Which helps slow transports, yet does not harm
fast transports which already accumulated the input data for the next
stage of processing. Check for zero length in more locations (accept
phrases like "sigrokproject#10" as well).
There is excessive diagnostics. Needs more adjustment (wording, levels).
The previous sr_scpi_get_block() implementation immediately returned after grabbing the data block. Which could have worked by coincidence when fast transports queue large chunks of receive data early, or could have gone unnoticed because there are few callers (hameg, lecroy only). Keep reading after the data block until the underlying transport finds the response message's end. Skipping this step could result in seeing "empty responses" for the next request, and synchronization loss for the remaining session when responses no longer correspond to callers' requests. Probability to reproduce depends on the type and speed of the involved transport, and how the device firmware chunks responses.
SCPI responses are expected to either come in pure text format (ASCII), _or_ raw bytes (data blocks). Tektronix TDS2000 was observed to respond with the unusual combination of a data block that is preceeded(!) by variable length(!) text fields which are separated by semicolon. This is the consequence of the "WAVF?" request being an aggregate, sending several properties including the "CURVE" response in the same message. Introduce a rather specific "get (an optional) text then data block" reader routine, which uses caller provided logic to identify the position of the data block in the response. Tested with TDS 220. The implementation re-uses the previously existing "get block" routine and extends it to optionally get a leading text. While lots of more diagnostics are added. Two very thin public wrapper routines provide the "get block" and "get (text and) block" semantics. There is excessive diagnostics. Levels and wording needs adjustment.
|
sorry for butting in, but i'm interested in using your branch with my TDS2022 (via USB/RS-232 converter) and while it seems to talk to it to some extent, there seems to be an issue: interestingly, the scope seems to reply when i try to reproduce that with picocom: i tried increasing the time-outs, adding line delimiters on the libsigrok end and changing them on the scope (i think any suggestions? thanks! |
|
@v0lker That's interesting. I know Have you tried wireshark/usb/serial captures of failures vs picocom? |
|
This is not a timeout issue: the log clearly shows it waiting for 30 seconds before timing out. I think it's a line termination issue. libsigrok's SCPI layer just adds If you add |
|
thanks everybody for your input! @biot suggested
as stated above, i tried that already. just went through the logs that i captured and unfortunately i didn't keep those experiments. i'll run them again, just to make certain that i didn't do something stupid then, but i think i tried at least i also tried changing the (outgoing, presumably, as it didn't seem to make a difference) line terminator in the TDS' settings. @byteit101 suggested to capture the USB packets which sounds like a good idea, but maybe i'll try with a protocol analyser on the serial immediately. either of them will hopefully reveal the naked truth. i think a big difference between how @byteit101 and i are connecting is that he's using USBTMC on the TDS2000B, whereas i'm using a TDS2022 + comms module with an RS-232 interface (and connecting to it via UBS/RS-232 converter), thus it's connecting via serial. will be back with the results in maybe a week or thereabouts (hopefully...) |
This is likely the difference, IIRC |
I added the
tektronix-ocp2k5driver for TBS1000B/EDU, TBS1000, TDS2000C/TDS1000C-EDU, TDS2000B/TDS1000B, TDS2000/TDS1000, TDS200 and TPS2000B/TPS2000 Series Digital Oscilloscopes. I tried to find a proper name for this protocol, but Tektronix support was being squirrely about a name, so I just arbitrarily came up with the blandly-generic "oscilloscope control protocol 2k5" as that is the only words somewhat used to describe it in the programming manual, plus the size of memory of all of these devices. I also thought about calling it the "lem2k5" for "Low-end Millennium" as this protocol seems to emerge around the turn of the millennium in low-end scopes with the TDS200 series. I'm happy to oblige a name change to something else.I have verified it using a TDS2000B series scope. A lot of features aren't supported, but I think the critical ones are taken care of. Luckily, since it appears that local control isn't locked out, more advanced features can still be configured via the front panel. The most glaring example of this is the trigger voltage, which PulseView doesn't seem to support.
I didn't update the udev rules, but I am querying the vendor for all the relevant PIDs. This branch is based on the sigrok.org master, not the out-of-date github master.
Here are three relevant screenshots of pulseview with this driver:
Finally, the timescale is 1-2.5-5, so the rational is not always power of 1000, leading to this slight weirdness in PulseView, but I think it should be fine?