X-Git-Url: https://sigrok.org/gitweb/?a=blobdiff_plain;f=src%2Fhardware%2Fkorad-kaxxxxp%2Fprotocol.c;h=bac8c8ef705fe06b8d5428fd66436431109e27be;hb=HEAD;hp=9bd548e4900936bfba901181c343781c63444f0f;hpb=2e129b8b25dc18d6fff17e7f0b572dec3a7c0c3c;p=libsigrok.git diff --git a/src/hardware/korad-kaxxxxp/protocol.c b/src/hardware/korad-kaxxxxp/protocol.c index 9bd548e4..bac8c8ef 100644 --- a/src/hardware/korad-kaxxxxp/protocol.c +++ b/src/hardware/korad-kaxxxxp/protocol.c @@ -21,11 +21,11 @@ #include #include "protocol.h" -#define REQ_TIMEOUT_MS 500 #define DEVICE_PROCESSING_TIME_MS 80 +#define EXTRA_PROCESSING_TIME_MS 450 SR_PRIV int korad_kaxxxxp_send_cmd(struct sr_serial_dev_inst *serial, - const char *cmd) + const char *cmd) { int ret; @@ -38,127 +38,230 @@ SR_PRIV int korad_kaxxxxp_send_cmd(struct sr_serial_dev_inst *serial, return ret; } +/** + * Read a variable length non-terminated string (caller specified maximum size). + * + * @param[in] serial The serial port to read from. + * @param[in] count The maximum amount of data to read. + * @param[out] buf The buffer to read data into. Must be larger than @a count. + * + * @return The amount of received data, or negative in case of error. + * See @ref SR_ERR and other error codes. + * + * @internal + * + * The protocol has no concept of request/response termination. The only + * terminating conditions are either the caller's expected maxmimum byte + * count, or a period of time without receive data. It's essential to + * accept a longer initial period of time before the first receive data + * is seen. The supported devices can be very slow to respond. + * + * The protocol is text based. That's why the 'count' parameter specifies + * the expected number of text characters, and does not include the NUL + * termination which is not part of the wire protocol but gets added by + * the receive routine. The caller provided buffer is expected to have + * enough space for the text data and the NUL termination. + * + * Implementation detail: It's assumed that once receive data was seen, + * remaining response data will follow at wire speed. No further delays + * are expected beyond bitrate expectations. All normal commands in the + * acquisition phase are of fixed length which is known to the caller. + * Identification during device scan needs to deal with variable length + * data. Quick termination after reception is important there, as is the + * larger initial timeout period before receive data is seen. + */ SR_PRIV int korad_kaxxxxp_read_chars(struct sr_serial_dev_inst *serial, - int count, char *buf) + size_t count, char *buf) { - int ret, received, turns; + int timeout_first, timeout_later, timeout; + size_t retries_first, retries_later, retries; + size_t received; + int ret; + /* Clear the buffer early, to simplify the receive code path. */ + memset(buf, 0, count + 1); + + /* + * This calculation is aiming for backwards compatibility with + * an earlier implementation. An initial timeout is used which + * depends on the expected response byte count, and a maximum + * iteration count is used for read attempts. + * + * TODO Consider an absolute initial timeout instead, to reduce + * accumulated rounding errors for serial timeout results. The + * iteration with a short period is still required for variable + * length responses, because otherwise the serial communication + * layer would spend the total amount of time waiting for the + * remaining bytes, while the device probe code path by design + * passes a larger acceptable count than the typical and legal + * response would occupy. + * + * After initial receive data was seen, a shorter timeout is + * used which corresponds to a few bytes at wire speed. Idle + * periods without receive data longer than this threshold are + * taken as the end of the response. This is not compatible to + * the previous implementation, but was found to work as well. + * And severely reduces the time spent scanning for devices. + */ + timeout_first = serial_timeout(serial, count); + retries_first = 100; + timeout_later = serial_timeout(serial, 3); + retries_later = 1; + + sr_spew("want %zu bytes, timeout/retry: init %d/%zu, later %d/%zu.", + count, timeout_first, retries_first, + timeout_later, retries_later); + + /* + * Run a sequence of read attempts. Try with the larger timeout + * and a high retry count until the first receive data became + * available. Then continue with a short timeout and small retry + * count. + * + * Failed read is fatal, immediately terminates the read sequence. + * A timeout in the initial phase just keeps repeating. A timeout + * after receive data was seen regularly terminates the sequence. + * Successful reads of non-empty responses keep extending the + * read sequence until no more receive data is available. + */ received = 0; - turns = 0; - - do { - if ((ret = serial_read_blocking(serial, buf + received, - count - received, - serial_timeout(serial, count))) < 0) { - sr_err("Error %d reading %d bytes from device.", + timeout = timeout_first; + retries = retries_first; + while (received < count && retries--) { + ret = serial_read_blocking(serial, + &buf[received], count - received, timeout); + if (ret < 0) { + sr_err("Error %d reading %zu bytes from device.", ret, count); return ret; } + if (ret == 0 && !received) + continue; + if (ret == 0 && received) { + sr_spew("receive timed out, want %zu, received %zu.", + count, received); + break; + } received += ret; - turns++; - } while ((received < count) && (turns < 100)); - - buf[count] = 0; - - sr_spew("Received: '%s'.", buf); + timeout = timeout_later; + retries = retries_later; + } + /* TODO Escape non-printables? Seen those with status queries. */ + sr_dbg("got %zu bytes, received: '%s'.", received, buf); - return ret; + return received; } static void give_device_time_to_process(struct dev_context *devc) { int64_t sleeping_time; - sleeping_time = devc->req_sent_at + (DEVICE_PROCESSING_TIME_MS * 1000); - sleeping_time -= g_get_monotonic_time(); + if (!devc->next_req_time) + return; + sleeping_time = devc->next_req_time - g_get_monotonic_time(); if (sleeping_time > 0) { g_usleep(sleeping_time); sr_spew("Sleeping for processing %" PRIi64 " usec", sleeping_time); } } +static int64_t next_req_time(struct dev_context *devc, + gboolean is_set, int target) +{ + gboolean is_slow_device, is_long_command; + int64_t processing_time_us; + + is_slow_device = devc->model->quirks & KORAD_QUIRK_SLOW_PROCESSING; + is_long_command = is_set; + is_long_command |= target == KAXXXXP_STATUS; + + processing_time_us = DEVICE_PROCESSING_TIME_MS; + if (is_slow_device && is_long_command) + processing_time_us += EXTRA_PROCESSING_TIME_MS; + processing_time_us *= 1000; + + return g_get_monotonic_time() + processing_time_us; +} + SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial, - int target, struct dev_context *devc) + int target, struct dev_context *devc) { - char *msg; - const char *cmd; - float value; + char msg[20]; int ret; g_mutex_lock(&devc->rw_mutex); give_device_time_to_process(devc); + msg[0] = '\0'; + ret = SR_OK; switch (target) { case KAXXXXP_CURRENT: case KAXXXXP_VOLTAGE: case KAXXXXP_STATUS: - sr_err("Can't set measurable parameter %d.", target); - g_mutex_unlock(&devc->rw_mutex); - return SR_ERR; + sr_err("Can't set measured value %d.", target); + ret = SR_ERR; + break; case KAXXXXP_CURRENT_LIMIT: - cmd = "ISET1:%05.3f"; - value = devc->set_current_limit; + sr_snprintf_ascii(msg, sizeof(msg), + "ISET1:%05.3f", devc->set_current_limit); break; case KAXXXXP_VOLTAGE_TARGET: - cmd = "VSET1:%05.2f"; - value = devc->set_voltage_target; + sr_snprintf_ascii(msg, sizeof(msg), + "VSET1:%05.2f", devc->set_voltage_target); break; case KAXXXXP_OUTPUT: - cmd = "OUT%01.0f"; - value = (devc->set_output_enabled) ? 1 : 0; + sr_snprintf_ascii(msg, sizeof(msg), + "OUT%1d", (devc->set_output_enabled) ? 1 : 0); /* Set value back to recognize changes */ devc->output_enabled = devc->set_output_enabled; break; case KAXXXXP_BEEP: - cmd = "BEEP%01.0f"; - value = (devc->set_beep_enabled) ? 1 : 0; + sr_snprintf_ascii(msg, sizeof(msg), + "BEEP%1d", (devc->set_beep_enabled) ? 1 : 0); break; case KAXXXXP_OCP: - cmd = "OCP%01.0f"; - value = (devc->set_ocp_enabled) ? 1 : 0; + sr_snprintf_ascii(msg, sizeof(msg), + "OCP%1d", (devc->set_ocp_enabled) ? 1 : 0); /* Set value back to recognize changes */ devc->ocp_enabled = devc->set_ocp_enabled; break; case KAXXXXP_OVP: - cmd = "OVP%01.0f"; - value = (devc->set_ovp_enabled) ? 1 : 0; + sr_snprintf_ascii(msg, sizeof(msg), + "OVP%1d", (devc->set_ovp_enabled) ? 1 : 0); /* Set value back to recognize changes */ devc->ovp_enabled = devc->set_ovp_enabled; break; case KAXXXXP_SAVE: - cmd = "SAV%01.0f"; if (devc->program < 1 || devc->program > 5) { - sr_err("Only programs 1-5 supported and %d isn't " - "between them.", devc->program); - g_mutex_unlock(&devc->rw_mutex); - return SR_ERR; + sr_err("Program %d is not in the supported 1-5 range.", + devc->program); + ret = SR_ERR; + break; } - value = devc->program; + sr_snprintf_ascii(msg, sizeof(msg), + "SAV%1d", devc->program); break; case KAXXXXP_RECALL: - cmd = "RCL%01.0f"; if (devc->program < 1 || devc->program > 5) { - sr_err("Only programs 1-5 supported and %d isn't " - "between them.", devc->program); - g_mutex_unlock(&devc->rw_mutex); - return SR_ERR; + sr_err("Program %d is not in the supported 1-5 range.", + devc->program); + ret = SR_ERR; + break; } - value = devc->program; + sr_snprintf_ascii(msg, sizeof(msg), + "RCL%1d", devc->program); break; default: - sr_err("Don't know how to set %d.", target); - g_mutex_unlock(&devc->rw_mutex); - return SR_ERR; + sr_err("Don't know how to set target %d.", target); + ret = SR_ERR; + break; } - msg = g_malloc0(20 + 1); - if (cmd) - sr_snprintf_ascii(msg, 20, cmd, value); - - ret = korad_kaxxxxp_send_cmd(serial, msg); - devc->req_sent_at = g_get_monotonic_time(); - g_free(msg); + if (ret == SR_OK && msg[0]) { + ret = korad_kaxxxxp_send_cmd(serial, msg); + devc->next_req_time = next_req_time(devc, TRUE, target); + } g_mutex_unlock(&devc->rw_mutex); @@ -166,12 +269,13 @@ SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial, } SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial, - int target, struct dev_context *devc) + int target, struct dev_context *devc) { int ret, count; char reply[6]; float *value; char status_byte; + gboolean needs_ovp_quirk; gboolean prev_status; g_mutex_lock(&devc->rw_mutex); @@ -211,19 +315,20 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial, break; default: sr_err("Don't know how to query %d.", target); + ret = SR_ERR; + } + if (ret < 0) { g_mutex_unlock(&devc->rw_mutex); - return SR_ERR; + return ret; } - devc->req_sent_at = g_get_monotonic_time(); + devc->next_req_time = next_req_time(devc, FALSE, target); if ((ret = korad_kaxxxxp_read_chars(serial, count, reply)) < 0) { g_mutex_unlock(&devc->rw_mutex); return ret; } - reply[count] = 0; - if (value) { sr_atof_ascii((const char *)&reply, value); sr_dbg("value: %f", *value); @@ -258,9 +363,8 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial, devc->output_enabled_changed = devc->output_enabled != prev_status; /* OVP enabled, special handling for Velleman LABPS3005 quirk. */ - if ((devc->model->model_id == VELLEMAN_LABPS3005D && devc->output_enabled) || - devc->model->model_id != VELLEMAN_LABPS3005D) { - + needs_ovp_quirk = devc->model->quirks & KORAD_QUIRK_LABPS_OVP_EN; + if (!needs_ovp_quirk || devc->output_enabled) { prev_status = devc->ovp_enabled; devc->ovp_enabled = status_byte & (1 << 7); devc->ovp_enabled_changed = devc->ovp_enabled != prev_status; @@ -268,17 +372,16 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial, sr_dbg("Status: 0x%02x", status_byte); sr_spew("Status: CH1: constant %s CH2: constant %s. " - "Tracking would be %s. Device is " - "%s and %s. Buttons are %s. Output is %s " - "and extra bit is %s.", + "Tracking would be %s and %s. Output is %s. " + "OCP is %s, OVP is %s. Device is %s.", (status_byte & (1 << 0)) ? "voltage" : "current", (status_byte & (1 << 1)) ? "voltage" : "current", (status_byte & (1 << 2)) ? "parallel" : "series", (status_byte & (1 << 3)) ? "tracking" : "independent", - (status_byte & (1 << 4)) ? "beeping" : "silent", - (status_byte & (1 << 5)) ? "locked" : "unlocked", (status_byte & (1 << 6)) ? "enabled" : "disabled", - (status_byte & (1 << 7)) ? "true" : "false"); + (status_byte & (1 << 5)) ? "enabled" : "disabled", + (status_byte & (1 << 7)) ? "enabled" : "disabled", + (status_byte & (1 << 4)) ? "beeping" : "silent"); } /* Read the sixth byte from ISET? BUG workaround. */ @@ -291,7 +394,7 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial, } SR_PRIV int korad_kaxxxxp_get_all_values(struct sr_serial_dev_inst *serial, - struct dev_context *devc) + struct dev_context *devc) { int ret, target;