X-Git-Url: https://sigrok.org/gitweb/?a=blobdiff_plain;f=src%2Fhardware%2Fjuntek-jds6600%2Fprotocol.c;h=5759c6a07722456b4d7eb78112b7d56aa8868348;hb=1ebdf6406682edb25cd4929638c24391bec8e5f6;hp=c6a19f85ec74a041cf958f92002f6afe18a7a86b;hpb=18baeeed7bb99f5470f04caa60e1b624207728b7;p=libsigrok.git diff --git a/src/hardware/juntek-jds6600/protocol.c b/src/hardware/juntek-jds6600/protocol.c index c6a19f85..5759c6a0 100644 --- a/src/hardware/juntek-jds6600/protocol.c +++ b/src/hardware/juntek-jds6600/protocol.c @@ -38,21 +38,25 @@ * - Add support for the frequency measurement and/or the counter. This * feature's availability may depend on or interact with the state of * other generator channels. Needs consideration of constraints. - * - Add support for modulation (sweep, pulse, burst). Add support for - * other "modes"? + * - Add support for "modes" (sweep, pulse, burst; modulation if the + * device supports it). * - Add support for download/upload of arbitrary waveforms. This needs * infrastructure in common libsigrok code as well as in applications. * At the moment "blob transfer" (waveform upload/download) appears to * not be supported. * - Re-consider parameter value ranges. Frequency depends on the model. - * Amplitude range depends on the model and frequencies and interact - * with the offset offset configuration. Can be -20..+20, or -10..+10, - * or -5..+5 ranges. This implementation caps to the -20..+20 range. - * Many values are passed to the device and may result in capping or - * transformation there in the firwmare. + * Amplitude depends on the model and frequencies. Can be -20..+20, + * or -10..+10, or -5..+5. Could be affected by offsets and further + * get clipped. This implementation caps application's input to the + * -20..+20 range, and sends the set request to the device. If any + * further transformation happens in the device then applications + * need to read back, this library driver doesn't. * * Implementation details: - * - Communicates via USB CDC at 115200/8n1 (virtual COM port). + * - Communicates via USB CDC at 115200/8n1 (virtual COM port). The user + * perceives a USB attached device (full speed, CDC/ACM class). The + * implementation needs to remember that a WCH CH340G forwards data + * to a microcontroller. Maximum throughput is in the 10KiB/s range. * - Requests are in text format. Start with a ':' colon, followed by a * single letter instruction opcode, followed by a number which either * addresses a parameter (think hardware register) or storage slot for @@ -64,12 +68,11 @@ * by an instruction letter, followed by a number (a parameter index, * or a waveform index), followed by '=' equal sign and one or more * values. Optionally ending in a '.' period. And ending in the - * firmware's end-of-line. Read requests will have this format. - * Alternatively write requests may just respond with the ":ok" - * text phrase. + * firmware's end-of-line. Read responses will have this format. + * Responses to write requests might just have the ":ok." literal. * - There are four instructions: 'r' to read and 'w' to write parameters - * (think hardware registers, optionaly multi-valued), 'a' to write and - * 'b' to read arbitrary waveform data (sequence of sample values). + * (think "hardware registers", optionaly multi-valued), 'a' to write + * and 'b' to read arbitrary waveform data (sequence of sample values). * - Am not aware of a vendor's documentation for the protocol. Joy-IT * provides the JT-JDS6600-Communication-protocol.pdf document which * leaves a lot of questions. This sigrok driver implementation used @@ -90,6 +93,39 @@ * of responses and phrasing of values in requests is arbitrary, this * "black magic" was found by local experimentation (reading back the * values which were configured by local UI interaction). + * - Communication is more reliable when the host unconditionally sends + * "function codes" (register and waveform indices) in two-digit form. + * Device firmware might implement rather specific assumptions. + * - Semantics of the right hand side in :rNN= and :bNN= read requests + * is uncertain. Just passing 0 in all situations worked in a local + * setup. As did omitting the value during interactive exploration. + * + * Example requests and responses. + * - Get model identification (max output frequency) + * TX text: --> :r00=0. + * TX bytes: --> 3a 72 30 30 3d 30 2e 0d 0a + * RX bytes: <-- 3a 72 30 30 3d 36 30 2e 0d 0a + * RX text: <-- :r00=60. + * - Get all channels' enabled state + * TX text: --> :r20=0. + * TX bytes: --> 3a 72 32 30 3d 30 2e 0d 0a + * RX bytes: <-- 3a 72 32 30 3d 31 2c 31 2e 0d 0a + * RX text: <-- :r20=1,1. + * - Get first channel's waveform selection + * TX text: --> :r21=0. + * TX bytes: --> 3a 72 32 31 3d 30 2e 0d 0a + * RX bytes: <-- 3a 72 32 31 3d 31 30 33 2e 0d 0a + * RX text: <-- :r21=103. + * - Set second channel's output frequency + * TX text: --> :w24=1234500,0. + * TX bytes: --> 3a 77 32 34 3d 31 32 33 34 35 30 30 2c 30 2e 0d 0a + * RX bytes: <-- 3a 6f 6b 0d 0a + * RX text: <-- :ok + * - Read arbitrary waveform number 13 + * TX text: --> :b13=0. + * TX bytes: --> 3a 62 31 33 3d 30 2e 0d 0a + * RX bytes: <-- 3a 62 31 33 3d 34 30 39 35 2c 34 30 39 35 2c ... 2c 34 30 39 35 2c 34 30 39 35 2c 0d 0a + * RX text: <-- :b13=4095,4095,...,4095,4095, */ #include "config.h" @@ -100,6 +136,7 @@ #include "protocol.h" +#define WITH_SERIAL_RAW_DUMP 0 /* Includes EOL and non-printables. */ #define WITH_ARBWAVE_DOWNLOAD 0 /* Development HACK */ /* @@ -110,11 +147,36 @@ */ #define MAX_RSP_LENGTH (8 + 2048 * 5) -/* Times are in milliseconds. */ -#define DELAY_AFTER_WRITE 10 +/* + * Times are in milliseconds. + * - Delay after transmission was an option during initial development. + * Has become obsolete. Support remains because it doesn't harm. + * - Delay after flash is essential when writing multiple waveforms to + * the device. Not letting more idle time pass after successful write + * and reception of the "ok" response, and before the next write, will + * result in corrupted waveform storage in the device. The next wave + * that is written waveform will start with several hundred samples + * of all-one bits. + * - Timeout per receive attempt at the physical layer can be short. + * Experience suggests that 2ms are a good value. Reception ends when + * the response termination was seen. Or when no receive data became + * available within that per-attemt timeout, and no higher level total + * timeout was specified. Allow some slack for USB FS frame intervals. + * - Timeout for identify attempts at the logical level can be short. + * Captures of the microcontroller communication suggest that firmware + * responds immediately (within 2ms). So 10ms per identify attempt + * are plenty for successful communication, yet quick enough to not + * stall on missing peripherals. + * - Timeout for waveform upload/download needs to be huge. Textual + * presentation of 2k samples with 12 significant bits (0..4095 range) + * combined with 115200bps UART communication result in a 1s maximum + * transfer time per waveform. So 1.2s is a good value. + */ +#define DELAY_AFTER_SEND 0 #define DELAY_AFTER_FLASH 100 -#define TIMEOUT_READ_CHUNK 20 -#define TIMEOUT_IDENTIFY 200 +#define TIMEOUT_READ_CHUNK 2 +#define TIMEOUT_IDENTIFY 10 +#define TIMEOUT_WAVEFORM 1200 /* Instruction codes. Read/write parameters/waveforms. */ #define INSN_WRITE_PARA 'w' @@ -226,9 +288,33 @@ static const char *waveform_names[] = { }; #define WAVEFORM_ARB_NAME_FMT "arb-%02zu" +static void log_raw_bytes(const char *caption, GString *buff) +{ + GString *text; + + if (!WITH_SERIAL_RAW_DUMP) + return; + if (sr_log_loglevel_get() < SR_LOG_SPEW) + return; + + if (!caption) + caption = ""; + text = sr_hexdump_new((const uint8_t *)buff->str, buff->len); + sr_spew("%s%s", caption, text->str); + sr_hexdump_free(text); +} + /* * Writes a text line to the serial port. Normalizes end-of-line * including trailing period. + * + * Accepts: + * ":r01=0." + * ":r01=0." + * ":r01=0" + * ":r01=0" + * Normalizes to: + * ":r01=0." */ static int serial_send_textline(const struct sr_dev_inst *sdi, GString *s, unsigned int delay_ms) @@ -246,7 +332,12 @@ static int serial_send_textline(const struct sr_dev_inst *sdi, if (!s) return SR_ERR_ARG; - /* Trim surrounding whitespace. Normalize end-of-line. */ + /* + * Trim surrounding whitespace. Normalize to canonical format. + * Make sure there is enough room for the period and CR/LF + * (and NUL termination). Use a glib API that's easy to adjust + * the padded length of. Performance is not a priority here. + */ padlen = 4; while (padlen--) g_string_append_c(s, '\0'); @@ -256,10 +347,11 @@ static int serial_send_textline(const struct sr_dev_inst *sdi, rdlen--; g_string_set_size(s, rdlen); g_string_append_c(s, '.'); - sr_spew("serial TX data: --> %s", rdptr); + sr_spew("serial TX text: --> %s", rdptr); g_string_append_c(s, '\r'); g_string_append_c(s, '\n'); rdlen = strlen(rdptr); + log_raw_bytes("serial TX bytes: --> ", s); /* Handle chunked writes, check for transmission errors. */ while (rdlen) { @@ -316,6 +408,8 @@ static int serial_recv_textline(const struct sr_dev_inst *sdi, ser = sdi->conn; if (!ser) return SR_ERR_ARG; + if (!s) + return SR_ERR_ARG; g_string_set_size(s, MAX_RSP_LENGTH); g_string_truncate(s, 0); @@ -342,6 +436,7 @@ static int serial_recv_textline(const struct sr_dev_inst *sdi, eol_pos = strchr(rdptr, '\n'); rdptr += got; rdlen -= got; + g_string_set_size(s, s->len + got); /* Check timeout expiration upon empty reception. */ has_timedout = FALSE; if (timeout_ms && !got) { @@ -354,13 +449,14 @@ static int serial_recv_textline(const struct sr_dev_inst *sdi, break; continue; } + log_raw_bytes("serial RX bytes: <-- ", s); /* Normalize the received text line. */ *eol_pos++ = '\0'; rdptr = s->str; (void)sr_text_trim_spaces(rdptr); rdlen = strlen(rdptr); - sr_spew("serial RX data: <-- %s", rdptr); + sr_spew("serial RX text: <-- %s", rdptr); if (rdlen && rdptr[rdlen - 1] == '.') rdptr[--rdlen] = '\0'; @@ -407,7 +503,7 @@ static int serial_recv_textline(const struct sr_dev_inst *sdi, if (ret != SR_OK || !endptr) return SR_ERR_DATA; if (wants_index && got_index != wants_index) { - sr_dbg("serial read, unexpected index %zu", got_index); + sr_dbg("serial read, unexpected index %lu", got_index); return SR_ERR_DATA; } rdptr = endptr; @@ -427,8 +523,9 @@ static int serial_recv_textline(const struct sr_dev_inst *sdi, *rhs_length = strlen(rdptr); return SR_OK; } + log_raw_bytes("serial RX bytes: <-- ", s); + sr_dbg("serial read, unterminated response, discarded"); - sr_dbg("serial read, no EOL seen"); return SR_ERR_DATA; } @@ -436,7 +533,7 @@ static int serial_recv_textline(const struct sr_dev_inst *sdi, static void append_insn_read_para(GString *s, char insn, size_t idx) { - g_string_append_printf(s, ":%c%02zu=0", insn, idx); + g_string_append_printf(s, ":%c%02zu=0", insn, idx & 0xff); } static void append_insn_write_para_va(GString *s, char insn, size_t idx, @@ -444,7 +541,7 @@ static void append_insn_write_para_va(GString *s, char insn, size_t idx, static void append_insn_write_para_va(GString *s, char insn, size_t idx, const char *fmt, va_list args) { - g_string_append_printf(s, ":%c%02zu=", insn, idx); + g_string_append_printf(s, ":%c%02zu=", insn, idx & 0xff); g_string_append_vprintf(s, fmt, args); } @@ -512,14 +609,14 @@ static int parse_freq_text(char *s, double *value) ret = sr_atoul_base(word, &scale, NULL, 10); if (ret != SR_OK) return ret; - sr_spew("parse freq, mant %lf, scale %ld", dvalue, scale); + sr_spew("parse freq, mant %f, scale %lu", dvalue, scale); if (scale >= ARRAY_SIZE(scales_freq)) return SR_ERR_DATA; /* Do scale the mantissa's value. */ dvalue /= 100.0; dvalue /= scales_freq[scale]; - sr_spew("parse freq, value %lf", dvalue); + sr_spew("parse freq, value %f", dvalue); if (value) *value = dvalue; @@ -535,9 +632,9 @@ static int parse_volt_text(char *s, double *value) ret = sr_atod(s, &dvalue); if (ret != SR_OK) return ret; - sr_spew("parse volt, mant %lf", dvalue); + sr_spew("parse volt, mant %f", dvalue); dvalue /= 1000.0; - sr_spew("parse volt, value %lf", dvalue); + sr_spew("parse volt, value %f", dvalue); if (value) *value = dvalue; @@ -557,14 +654,14 @@ static int parse_bias_text(char *s, double *value) ret = sr_atod(s, &dvalue); if (ret != SR_OK) return ret; - sr_spew("parse bias, mant %lf", dvalue); + sr_spew("parse bias, mant %f", dvalue); dvalue /= 100.0; dvalue -= 10.0; if (dvalue >= 9.99) dvalue = 9.99; if (dvalue <= -9.99) dvalue = -9.99; - sr_spew("parse bias, value %lf", dvalue); + sr_spew("parse bias, value %f", dvalue); if (value) *value = dvalue; @@ -583,9 +680,9 @@ static int parse_duty_text(char *s, double *value) ret = sr_atod(s, &dvalue); if (ret != SR_OK) return ret; - sr_spew("parse duty, mant %lf", dvalue); + sr_spew("parse duty, mant %f", dvalue); dvalue /= 1000.0; - sr_spew("parse duty, value %lf", dvalue); + sr_spew("parse duty, value %f", dvalue); if (value) *value = dvalue; @@ -601,9 +698,9 @@ static int parse_phase_text(char *s, double *value) ret = sr_atod(s, &dvalue); if (ret != SR_OK) return ret; - sr_spew("parse phase, mant %lf", dvalue); + sr_spew("parse phase, mant %f", dvalue); dvalue /= 10.0; - sr_spew("parse phase, value %lf", dvalue); + sr_spew("parse phase, value %f", dvalue); if (value) *value = dvalue; @@ -622,7 +719,7 @@ static void write_freq_text(GString *s, double freq) unsigned long scale_idx; const char *text_pos; - sr_spew("write freq, value %lf", freq); + sr_spew("write freq, value %f", freq); text_pos = &s->str[s->len]; /* @@ -634,7 +731,7 @@ static void write_freq_text(GString *s, double freq) freq *= scales_freq[scale_idx]; freq *= 100.0; - g_string_append_printf(s, "%.0lf,%lu", freq, scale_idx); + g_string_append_printf(s, "%.0f,%lu", freq, scale_idx); sr_spew("write freq, text %s", text_pos); } @@ -642,7 +739,7 @@ static void write_volt_text(GString *s, double volt) { const char *text_pos; - sr_spew("write volt, value %lf", volt); + sr_spew("write volt, value %f", volt); text_pos = &s->str[s->len]; /* @@ -655,7 +752,7 @@ static void write_volt_text(GString *s, double volt) if (volt < 0.0) volt = 0.0; volt *= 1000.0; - g_string_append_printf(s, "%.0lf", volt); + g_string_append_printf(s, "%.0f", volt); sr_spew("write volt, text %s", text_pos); } @@ -663,7 +760,7 @@ static void write_bias_text(GString *s, double volt) { const char *text_pos; - sr_spew("write bias, value %lf", volt); + sr_spew("write bias, value %f", volt); text_pos = &s->str[s->len]; /* @@ -677,7 +774,7 @@ static void write_bias_text(GString *s, double volt) volt += 10.0; volt *= 100.0; - g_string_append_printf(s, "%.0lf", volt); + g_string_append_printf(s, "%.0f", volt); sr_spew("write bias, text %s", text_pos); } @@ -685,7 +782,7 @@ static void write_duty_text(GString *s, double duty) { const char *text_pos; - sr_spew("write duty, value %lf", duty); + sr_spew("write duty, value %f", duty); text_pos = &s->str[s->len]; /* @@ -698,7 +795,7 @@ static void write_duty_text(GString *s, double duty) duty = 1.0; duty *= 1000.0; - g_string_append_printf(s, "%.0lf", duty); + g_string_append_printf(s, "%.0f", duty); sr_spew("write duty, text %s", text_pos); } @@ -706,7 +803,7 @@ static void write_phase_text(GString *s, double phase) { const char *text_pos; - sr_spew("write phase, value %lf", phase); + sr_spew("write phase, value %f", phase); text_pos = &s->str[s->len]; /* @@ -716,7 +813,7 @@ static void write_phase_text(GString *s, double phase) phase = fmod(phase, 360.0); phase *= 10.0; - g_string_append_printf(s, "%.0lf", phase); + g_string_append_printf(s, "%.0f", phase); sr_spew("write phase, text %s", text_pos); } @@ -745,7 +842,7 @@ static int quick_send_read_then_recv(const struct sr_dev_inst *sdi, g_string_truncate(s, 0); append_insn_read_para(s, insn, idx); - ret = serial_send_textline(sdi, s, DELAY_AFTER_WRITE); + ret = serial_send_textline(sdi, s, DELAY_AFTER_SEND); if (ret != SR_OK) return ret; @@ -790,7 +887,7 @@ static int quick_send_write_then_recv_ok(const struct sr_dev_inst *sdi, va_start(args, fmt); append_insn_write_para_va(s, insn, idx, fmt, args); va_end(args); - ret = serial_send_textline(sdi, s, DELAY_AFTER_WRITE); + ret = serial_send_textline(sdi, s, DELAY_AFTER_SEND); if (ret != SR_OK) return ret; @@ -980,7 +1077,7 @@ SR_PRIV int jds6600_get_frequency(const struct sr_dev_inst *sdi, size_t ch_idx) ret = parse_freq_text(rdptr, &freq); if (ret != SR_OK) return SR_ERR_DATA; - sr_dbg("get frequency, value %lf", freq); + sr_dbg("get frequency, value %f", freq); chan->output_frequency = freq; return SR_OK; } @@ -1012,7 +1109,7 @@ SR_PRIV int jds6600_get_amplitude(const struct sr_dev_inst *sdi, size_t ch_idx) ret = parse_volt_text(rdptr, &); if (ret != SR_OK) return SR_ERR_DATA; - sr_dbg("get amplitude, value %lf", amp); + sr_dbg("get amplitude, value %f", amp); chan->amplitude = amp; return SR_OK; } @@ -1044,7 +1141,7 @@ SR_PRIV int jds6600_get_offset(const struct sr_dev_inst *sdi, size_t ch_idx) ret = parse_bias_text(rdptr, &off); if (ret != SR_OK) return SR_ERR_DATA; - sr_dbg("get offset, value %lf", off); + sr_dbg("get offset, value %f", off); chan->offset = off; return SR_OK; } @@ -1076,7 +1173,7 @@ SR_PRIV int jds6600_get_dutycycle(const struct sr_dev_inst *sdi, size_t ch_idx) ret = parse_duty_text(rdptr, &duty); if (ret != SR_OK) return SR_ERR_DATA; - sr_dbg("get duty cycle, value %lf", duty); + sr_dbg("get duty cycle, value %f", duty); chan->dutycycle = duty; return SR_OK; } @@ -1104,7 +1201,7 @@ SR_PRIV int jds6600_get_phase_chans(const struct sr_dev_inst *sdi) ret = parse_phase_text(rdptr, &phase); if (ret != SR_OK) return SR_ERR_DATA; - sr_dbg("get phase, value %lf", phase); + sr_dbg("get phase, value %f", phase); devc->channels_phase = phase; return SR_OK; } @@ -1485,8 +1582,11 @@ SR_PRIV int jds6600_setup_devc(struct sr_dev_inst *sdi) waves->fw_codes = g_malloc0(alloc_count * sizeof(waves->fw_codes[0])); alloc_count++; waves->names = g_malloc0(alloc_count * sizeof(waves->names[0])); - if (!waves->names || !waves->fw_codes) + if (!waves->names || !waves->fw_codes) { + g_free(waves->names); + g_free(waves->fw_codes); return SR_ERR_MALLOC; + } assign_idx = 0; for (idx = 0; idx < waves->builtin_count; idx++) { code = idx; @@ -1508,6 +1608,13 @@ SR_PRIV int jds6600_setup_devc(struct sr_dev_inst *sdi) * Populate internal channel configuration details from the * device's current state. Emit a series of queries which * update internal knowledge. + * + * Implementation detail: Channel count is low, all parameters + * are simple scalars. Communication cycles are few, while we + * still are in the scan/probe phase and successfully verified + * the device to respond. Disconnects and other exceptional + * conditions are extremely unlikely. Not checking every getter + * call's return value is acceptable here. */ ret = SR_OK; ret |= jds6600_get_chans_enable(sdi); @@ -1517,9 +1624,10 @@ SR_PRIV int jds6600_setup_devc(struct sr_dev_inst *sdi) ret |= jds6600_get_amplitude(sdi, idx); ret |= jds6600_get_offset(sdi, idx); ret |= jds6600_get_dutycycle(sdi, idx); + if (ret != SR_OK) + break; } ret |= jds6600_get_phase_chans(sdi); - ret |= jds6600_get_chans_enable(sdi); if (ret != SR_OK) return SR_ERR_DATA;