Check some settings are what we expect, and minor reorg

Author: byteit101

src/hardware/tektronix-tds2000b/api.c

@@ -25,24 +25,36 @@
 #include "protocol.h"
 #include "scpi.h"
 
-static struct sr_dev_driver tektronix_ocp2k5_driver_info;
-
 /**
  * Documentation for the SCPI commands can be found in
  * https://download.tek.com/manual/TBS1000-B-EDU-TDS2000-B-C-TDS1000-B-C-EDU-TDS200-TPS2000-B-Programmer-077044403_RevB.pdf
  * and is referred to as "doc page $PDF_PAGE/$PRINTED_PAGE"
  */
 
-static const uint32_t scanopts[] = {SR_CONF_CONN, SR_CONF_SERIALCOMM};
 
-static const uint32_t drvopts[] = {SR_CONF_OSCILLOSCOPE};
 /**
- * TODOS
- *
- * General cleanup
- * current options?
+ * Missing semi-important features:
+ *   bandwidth limiting ch<x>:bandwidth
+ *   chanel invert ch<x>:invert
+ *   volt/amp configuration ch:<x>:yunit
+ *   pulse triggering
+ *   ext trigger coupling
+ *   peak-detect mode (data retreival)
+ * 
+ * Missing less important features:
+ *   capture/savefiles
+ *   screenshots
+ *   fine adjust of vdivs
+ *   video triggering
  */
 
+
+static struct sr_dev_driver tektronix_ocp2k5_driver_info;
+
+static const uint32_t scanopts[] = {SR_CONF_CONN, SR_CONF_SERIALCOMM};
+
+static const uint32_t drvopts[] = {SR_CONF_OSCILLOSCOPE};
+
 static const uint32_t devopts[] = {
 	SR_CONF_TIMEBASE | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
 	SR_CONF_TRIGGER_SOURCE | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
@@ -58,37 +70,20 @@ static const uint32_t devopts[] = {
 	SR_CONF_BUFFERSIZE | SR_CONF_GET,
 	SR_CONF_DATA_SOURCE | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
 	SR_CONF_PEAK_DETECTION | SR_CONF_GET | SR_CONF_SET,
-
-	// capturefile or session file?
-
-	// SR_CONF_LIMIT_SAMPLES? (could be here or per-channel, but
-        // realistically, 2.5k samples isn't going to overwhelm anyone)
-
-		// TODO: voltage threshold?
 };
 
-/**
- * Missing semi-important features:
- * bandwidth limiting ch<x>:bandwidth
- * chanel invert ch<x>:invert
- * volt/amp configuration ch:<x>:yunit
- */
-
 static const uint32_t devopts_cg_analog[] = {
 	SR_CONF_NUM_VDIV | SR_CONF_GET,
 	SR_CONF_VDIV | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
 	SR_CONF_COUPLING | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
 	SR_CONF_PROBE_FACTOR | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
 	SR_CONF_ENABLED | SR_CONF_GET | SR_CONF_SET,
-
-	// Is SR_CONF_CHANNEL_CONFIG how "advanced" features are supported?
 };
 
-// TODO: Compensation adjusts the vdivs, and I implemented a semi-proper
-// configuration of that, but it  wasn't ideal, so for now
-// all possible compensation vdivs are present
+// TODO: Compensation adjusts the vdivs, but PulseView doesn't pick up
+// any such dynamic changes, nor does it seem to support fine-adjust,
+// so for now all possible compensation vdivs are present
 
-// TODO: allow fine adjust
 // validated in doc page 75/2-57
 static const uint64_t vdivs[][2] = {
 	/* millivolts */
@@ -118,7 +113,7 @@ static const uint64_t vdivs[][2] = {
 	{5000, 1},
 };
 
-// everyone uses the same voltrange
+// everyone uses the same voltrange, only modified by compensation ranges (also the same)
 #define VOLTRANGE_2m_5V 0, 0
 
 static const uint64_t timebases[][2] = {
@@ -165,7 +160,6 @@ static const uint64_t timebases[][2] = {
 #define TIMEBASE_10n_50s 2, 0
 #define TIMEBASE_5ns_5s  1, 3
 
-// TODO: edge trigger has other options
 // validated in doc page 71/2-53
 static const char *coupling[] = {
 	"AC",
@@ -175,25 +169,9 @@ static const char *coupling[] = {
 
 // validated in doc page 74/2-53
 static const uint64_t probe_factor_new[] = {1, 10, 20, 50, 100, 500, 1000};
-// tds200, tds2000, tds1000
+// Only for tds200, tds2000, tds1000
 static const uint64_t probe_factor_old[] = {1, 10, 100, 1000};
 
-
-// TODO: current probe factor FIXME: 0.2x current probes are supported but this
-// is an int setting
-
-// {5,1,.5,.2,.1,.02,.01,.001} tbs1000B/EDU,
-// {5,1,.5,.2,.1,.02,.01,.001} tbs1000,
-// {5,1,.5,.2,.1,.02,.01,.001} tds2000c,
-// {5,1,.5,.2,.1,.02,.01,.001} tds1000C-edu,
-// {5,1,.5,.2,.1,.02,.01,.001} tds2000b,
-// {5,1,.5,.2,.1,.02,.01,.001} tds1000B,
-
-// {5,1,.2,.1,.05.02,.01,.001} tps2000/B
-// N/A tds200
-// tds2000
-// tds1000 ?
-
 static const char *trigger_slopes[] = {
 	"r",
 	"f",
@@ -247,18 +225,19 @@ static const char *trigger_sources_models_TPS_2k[] = {
 			4 + channels                                           \
 	}
 
-/* The doc is for:
+/* This table was generated from the documentation:
  *
- * [x] TBS1000B/EDU: https://download.tek.com/manual/TBS1000B-User-Manual-077088602-RevA.pdf
- * [x] TBS1000: https://download.tek.com/manual/TBS1000-Oscilloscope-User-Manual_077076001.pdf
- * [x] TDS2000C/TDS1000C-EDU: https://download.tek.com/manual/TDS2000C-and-TDS1000C-EDU-Oscilloscope-User-Manual-EN_077082600.pdf
- * [x] TDS2000B/TDS1000B: https://download.tek.com/manual/071181702web.pdf
- * [x] TDS2000/TDS1000: https://download.tek.com/manual/TDS2000_TDS1000_User_071106400_Revision_A.pdf
- * [x] TDS200: https://download.tek.com/manual/071039803.pdf
- * [x] TPS2000B:https://download.tek.com/manual/TPS2000B-Digital-Oscilloscope-User-Manual-077137901.pdf
- * [x] TPS2000: https://download.tek.com/manual/071144105web.pdf
+ * TBS1000B/EDU: https://download.tek.com/manual/TBS1000B-User-Manual-077088602-RevA.pdf
+ * TBS1000: https://download.tek.com/manual/TBS1000-Oscilloscope-User-Manual_077076001.pdf
+ * TDS2000C/TDS1000C-EDU: https://download.tek.com/manual/TDS2000C-and-TDS1000C-EDU-Oscilloscope-User-Manual-EN_077082600.pdf
+ * TDS2000B/TDS1000B: https://download.tek.com/manual/071181702web.pdf
+ * TDS2000/TDS1000: https://download.tek.com/manual/TDS2000_TDS1000_User_071106400_Revision_A.pdf
+ * TDS200: https://download.tek.com/manual/071039803.pdf
+ * TPS2000B:https://download.tek.com/manual/TPS2000B-Digital-Oscilloscope-User-Manual-077137901.pdf
+ * TPS2000: https://download.tek.com/manual/071144105web.pdf
  *
  * All specs can be found in Appendix A's of the linked pdfs
+ * EDU series are badge-only, and respond as if they are non-EDU products
  */
 static const struct device_spec device_models[] = {
 
@@ -690,7 +669,7 @@ static int config_set(uint32_t key, GVariant *data,
 			tmp_str = "EXT10";
 		else
 			tmp_str = (char *)devc->trigger_source;
-		// TODO: pulse and video
+		// Note: pulse and video triggering isn't set here
 		return tektronix_ocp2k5_config_set(
 			sdi, "TRIG:mai:edge:sou %s", tmp_str);
 	case SR_CONF_VDIV:

src/hardware/tektronix-tds2000b/protocol.c

@@ -425,6 +425,27 @@ static int parse_scpi_blockstart(const char *data, int *out_err)
 	return -1;
 }
 
+
+static void check_expected_value(
+	const char* name, int actual, int expected, int* out_err, 
+	const struct tek_enum_parser *parser_table)
+{
+	if (actual != expected) {
+		*out_err = SR_ERR_DATA;
+		if (parser_table == NULL)
+			sr_err(
+				"Error validating data header. Field '%s' expected %d, but found %d",
+				name, expected, actual);
+		else {
+			sr_err(
+				"Error validating data header. Field '%s' expected %s, but found %s",
+				name,
+				render_scpi_enum(expected, parser_table, out_err),
+				render_scpi_enum(actual, parser_table, out_err));
+		}
+	}
+}
+
 static int tektronix_ocp2k5_parse_header(struct sr_dev_inst *sdi, char *end_buf)
 {
 	struct sr_scpi_dev_inst *scpi = sdi->conn;
@@ -481,7 +502,6 @@ static int tektronix_ocp2k5_parse_header(struct sr_dev_inst *sdi, char *end_buf)
 	*/
 
 	i = 0;
-	// TODO: assert a few of these values
 	byte_width = parse_scpi_int(fields[i++], &ret, 1);
 	bit_width = parse_scpi_int(fields[i++], &ret, 8);
 	encoding = parse_scpi_enum(
@@ -495,8 +515,7 @@ static int tektronix_ocp2k5_parse_header(struct sr_dev_inst *sdi, char *end_buf)
 	pt_format = parse_scpi_enum(
 		fields[i++], parse_table_point_format, &ret, PT_FMT_Y);
 	devc->wavepre.x_incr = parse_scpi_float(fields[i++], &ret, 1);
-	pt_off = parse_scpi_int(fields[i++], &ret, 0); // always zero, parsed
-	                                               // only for completeness
+	pt_off = parse_scpi_int(fields[i++], &ret, 0);
 	devc->wavepre.x_zero = parse_scpi_float(fields[i++], &ret, 0);
 	devc->wavepre.x_unit = parse_scpi_enum(sr_scpi_unquote_string(fields[i++]),
 		parse_table_xunits, &ret, XU_SECOND);
@@ -528,6 +547,19 @@ static int tektronix_ocp2k5_parse_header(struct sr_dev_inst *sdi, char *end_buf)
 			render_scpi_enum(devc->wavepre.y_unit,
 				parse_table_yunits, &ret),
 			blocklength);
+	
+	// check that settings weren't tampered with
+	check_expected_value("byte width", byte_width, 1, &ret, NULL);
+	check_expected_value("bit size", bit_width, 8, &ret, NULL);
+	check_expected_value("data encoding", encoding, ENC_BINARY, &ret, parse_table_data_encoding);
+	check_expected_value("data format", format, FMT_RI, &ret, parse_table_data_format);
+	check_expected_value("data encoding", ordering, ORDER_MSB, &ret, parse_table_data_ordering);
+	check_expected_value("number of points", devc->wavepre.num_pts, TEK_BUFFER_SIZE, &ret, NULL);
+	// this value is ENV when in peak detect mode
+	check_expected_value("point format", pt_format, PT_FMT_Y, &ret, parse_table_point_format);
+
+	check_expected_value("point offset", pt_off, 0, &ret, NULL);
+	check_expected_value("block length", blocklength, TEK_BUFFER_SIZE, &ret, NULL);
 	return ret;
 }
 
@@ -782,6 +814,7 @@ SR_PRIV int tektronix_ocp2k5_get_dev_cfg_horizontal(const struct sr_dev_inst *sd
 			devc->model->sample_rate);
 	else
 		sr_dbg("Current samplerate: %ld Sa/s.", (long)fvalue);
+
 	// TODO: peak detect mode is half of this
 	sr_dbg("Current memory depth: %d.", TEK_BUFFER_SIZE);
 	return SR_OK;