From: Gerhard Sittig Date: Thu, 24 Sep 2020 14:33:20 +0000 (+0200) Subject: serial-dmm: more DMM parser callbacks (open, var length, config, start) X-Git-Url: https://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=070668a0fd0876485ea6867eb60010da0ef1c304 serial-dmm: more DMM parser callbacks (open, var length, config, start) Extend the serial-dmm driver's common infrastructure to support more per-parser (per-model) specific extensions. Add support for variable length packets (motivated by BM85x), and pass the packet length to parsers which accept it. Add callbacks which run after the COM port got opened (motivated by BM85x). Add support for additional configuration get/set/list properties (motivated by BM52x), including a hook into the acquisition start and a state container which is owned by the parser. Device specific acquisition start can check its local state which can store the result of previous config get/set requests, and can arrange for a different receive routine to execute (motivated by BM52x). The default code path will execute serial-dmm's receive routine which keeps invoking the DMM's packet parser for each registered display. Prefer double precision values in the new parser callbacks. Also fixup some data type issues: Use unsigned types for length and size specs as well as timeouts, don't promote false booleans to NULL pointers, reduce malloc() argument redundancy. Rephrase some instruction grouping and update comments to simplify future maintenance. Get the current time just once for improved consistency in the packet re-request code path. Rename identifiers in the data reception path to improve readability. --- diff --git a/src/hardware/serial-dmm/api.c b/src/hardware/serial-dmm/api.c index 6651e802..660f2080 100644 --- a/src/hardware/serial-dmm/api.c +++ b/src/hardware/serial-dmm/api.c @@ -40,8 +40,8 @@ static const uint32_t drvopts[] = { static const uint32_t devopts[] = { SR_CONF_CONTINUOUS, - SR_CONF_LIMIT_SAMPLES | SR_CONF_SET, - SR_CONF_LIMIT_MSEC | SR_CONF_SET, + SR_CONF_LIMIT_SAMPLES | SR_CONF_GET | SR_CONF_SET, + SR_CONF_LIMIT_MSEC | SR_CONF_GET | SR_CONF_SET, }; static GSList *scan(struct sr_dev_driver *di, GSList *options) @@ -53,8 +53,8 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options) struct sr_dev_inst *sdi; struct dev_context *devc; struct sr_serial_dev_inst *serial; - int dropped, ret; - size_t len; + int ret; + size_t dropped, len, packet_len; uint8_t buf[128]; size_t ch_idx; char ch_name[12]; @@ -81,16 +81,23 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options) if (serial_open(serial, SERIAL_RDWR) != SR_OK) return NULL; - sr_info("Probing serial port %s.", conn); + if (dmm->after_open) { + ret = dmm->after_open(serial); + if (ret != SR_OK) { + sr_err("Activity after port open failed: %d.", ret); + return NULL; + } + } + devices = NULL; /* Request a packet if the DMM requires this. */ if (dmm->packet_request) { if ((ret = dmm->packet_request(serial)) < 0) { sr_err("Failed to request packet: %d.", ret); - return FALSE; + return NULL; } } @@ -98,37 +105,43 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options) * There's no way to get an ID from the multimeter. It just sends data * periodically (or upon request), so the best we can do is check if * the packets match the expected format. - */ - - /* Let's get a bit of data and see if we can find a packet. */ - len = sizeof(buf); - ret = serial_stream_detect(serial, buf, &len, dmm->packet_size, - dmm->packet_valid, NULL, NULL, 3000); - if (ret != SR_OK) - goto scan_cleanup; - - /* + * * If we dropped more than two packets worth of data, something is * wrong. We shouldn't quit however, since the dropped bytes might be * just zeroes at the beginning of the stream. Those can occur as a * combination of the nonstandard cable that ships with some devices * and the serial port or USB to serial adapter. */ + len = sizeof(buf); + ret = serial_stream_detect(serial, buf, &len, dmm->packet_size, + dmm->packet_valid, dmm->packet_valid_len, &packet_len, 3000); + if (ret != SR_OK) + goto scan_cleanup; dropped = len - dmm->packet_size; - if (dropped > 2 * dmm->packet_size) - sr_warn("Had to drop too much data."); - + if (dropped > 2 * packet_len) + sr_warn("Packet search dropped a lot of data."); sr_info("Found device on port %s.", conn); - sdi = g_malloc0(sizeof(struct sr_dev_inst)); + /* + * Setup optional additional callbacks when sub device drivers + * happen to provide them. (This is a compromise to do it here, + * and not extend the DMM_CONN() et al set of macros.) + */ + if (dmm->dmm_state_init) + dmm->dmm_state = dmm->dmm_state_init(); + + /* Setup the device instance. */ + sdi = g_malloc0(sizeof(*sdi)); sdi->status = SR_ST_INACTIVE; sdi->vendor = g_strdup(dmm->vendor); sdi->model = g_strdup(dmm->device); - devc = g_malloc0(sizeof(struct dev_context)); + devc = g_malloc0(sizeof(*devc)); sr_sw_limits_init(&devc->limits); sdi->inst_type = SR_INST_SERIAL; sdi->conn = serial; sdi->priv = devc; + + /* Create (optionally device dependent) channel(s). */ dmm->channel_count = 1; if (dmm->packet_parse == sr_brymen_bm52x_parse) dmm->channel_count = BRYMEN_BM52X_DISPLAY_COUNT; @@ -154,6 +167,8 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options) snprintf(ch_name, sizeof(ch_name), fmt, ch_num); sr_channel_new(sdi, ch_idx, SR_CHANNEL_ANALOG, TRUE, ch_name); } + + /* Add found device to result set. */ devices = g_slist_append(devices, sdi); scan_cleanup: @@ -162,27 +177,85 @@ scan_cleanup: return std_scan_complete(di, devices); } -static int config_set(uint32_t key, GVariant *data, +static int config_get(uint32_t key, GVariant **data, const struct sr_dev_inst *sdi, const struct sr_channel_group *cg) { struct dev_context *devc; + struct dmm_info *dmm; - (void)cg; + if (!sdi) + return SR_ERR_ARG; + devc = sdi->priv; + + switch (key) { + case SR_CONF_LIMIT_SAMPLES: + case SR_CONF_LIMIT_FRAMES: + case SR_CONF_LIMIT_MSEC: + return sr_sw_limits_config_get(&devc->limits, key, data); + default: + dmm = (struct dmm_info *)sdi->driver; + if (!dmm || !dmm->config_get) + return SR_ERR_NA; + return dmm->config_get(dmm->dmm_state, key, data, sdi, cg); + } + /* UNREACH */ +} + +static int config_set(uint32_t key, GVariant *data, + const struct sr_dev_inst *sdi, const struct sr_channel_group *cg) +{ + struct dev_context *devc; + struct dmm_info *dmm; + if (!sdi) + return SR_ERR_ARG; devc = sdi->priv; + (void)cg; - return sr_sw_limits_config_set(&devc->limits, key, data); + switch (key) { + case SR_CONF_LIMIT_SAMPLES: + case SR_CONF_LIMIT_FRAMES: + case SR_CONF_LIMIT_MSEC: + return sr_sw_limits_config_set(&devc->limits, key, data); + default: + dmm = (struct dmm_info *)sdi->driver; + if (!dmm || !dmm->config_set) + return SR_ERR_NA; + return dmm->config_set(dmm->dmm_state, key, data, sdi, cg); + } } static int config_list(uint32_t key, GVariant **data, const struct sr_dev_inst *sdi, const struct sr_channel_group *cg) { + struct dmm_info *dmm; + int ret; + + /* Use common logic for standard keys. */ + if (!sdi) + return STD_CONFIG_LIST(key, data, sdi, cg, scanopts, drvopts, devopts); + + /* + * Check for device specific config_list handler. ERR N/A from + * that handler is non-fatal, just falls back to common logic. + */ + dmm = (struct dmm_info *)sdi->driver; + if (dmm && dmm->config_list) { + ret = dmm->config_list(dmm->dmm_state, key, data, sdi, cg); + if (ret != SR_ERR_NA) + return ret; + } + return STD_CONFIG_LIST(key, data, sdi, cg, scanopts, drvopts, devopts); } static int dev_acquisition_start(const struct sr_dev_inst *sdi) { struct dev_context *devc; + struct dmm_info *dmm; + sr_receive_data_callback cb_func; + void *cb_data; + int ret; struct sr_serial_dev_inst *serial; devc = sdi->priv; @@ -190,16 +263,28 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi) sr_sw_limits_acquisition_start(&devc->limits); std_session_send_df_header(sdi); + cb_func = receive_data; + cb_data = (void *)sdi; + dmm = (struct dmm_info *)sdi->driver; + if (dmm && dmm->acquire_start) { + ret = dmm->acquire_start(dmm->dmm_state, sdi, + &cb_func, &cb_data); + if (ret < 0) + return ret; + } + serial = sdi->conn; serial_source_add(sdi->session, serial, G_IO_IN, 50, - receive_data, (void *)sdi); + cb_func, cb_data); return SR_OK; } -#define DMM_CONN(ID, CHIPSET, VENDOR, MODEL, \ +#define DMM_ENTRY(ID, CHIPSET, VENDOR, MODEL, \ CONN, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ - REQUEST, VALID, PARSE, DETAILS) \ + OPEN, REQUEST, VALID, PARSE, DETAILS, \ + INIT_STATE, FREE_STATE, VALID_LEN, PARSE_LEN, \ + CFG_GET, CFG_SET, CFG_LIST, ACQ_START) \ &((struct dmm_info) { \ { \ .name = ID, \ @@ -210,7 +295,7 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi) .scan = scan, \ .dev_list = std_dev_list, \ .dev_clear = std_dev_clear, \ - .config_get = NULL, \ + .config_get = config_get, \ .config_set = config_set, \ .config_list = config_list, \ .dev_open = std_serial_dev_open, \ @@ -220,13 +305,34 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi) .context = NULL, \ }, \ VENDOR, MODEL, CONN, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ - REQUEST, 1, NULL, VALID, PARSE, DETAILS, sizeof(struct CHIPSET##_info) \ + REQUEST, 1, NULL, VALID, PARSE, DETAILS, \ + sizeof(struct CHIPSET##_info), \ + NULL, INIT_STATE, FREE_STATE, \ + OPEN, VALID_LEN, PARSE_LEN, \ + CFG_GET, CFG_SET, CFG_LIST, ACQ_START, \ }).di +#define DMM_CONN(ID, CHIPSET, VENDOR, MODEL, \ + CONN, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ + REQUEST, VALID, PARSE, DETAILS) \ + DMM_ENTRY(ID, CHIPSET, VENDOR, MODEL, \ + CONN, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ + NULL, REQUEST, VALID, PARSE, DETAILS, \ + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL) + #define DMM(ID, CHIPSET, VENDOR, MODEL, SERIALCOMM, PACKETSIZE, TIMEOUT, \ DELAY, REQUEST, VALID, PARSE, DETAILS) \ - DMM_CONN(ID, CHIPSET, VENDOR, MODEL, NULL, SERIALCOMM, PACKETSIZE, \ - TIMEOUT, DELAY, REQUEST, VALID, PARSE, DETAILS) + DMM_CONN(ID, CHIPSET, VENDOR, MODEL, \ + NULL, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ + REQUEST, VALID, PARSE, DETAILS) + +#define DMM_LEN(ID, CHIPSET, VENDOR, MODEL, \ + CONN, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ + INIT, FREE, OPEN, REQUEST, VALID, PARSE, DETAILS) \ + DMM_ENTRY(ID, CHIPSET, VENDOR, MODEL, \ + CONN, SERIALCOMM, PACKETSIZE, TIMEOUT, DELAY, \ + OPEN, REQUEST, NULL, NULL, DETAILS, \ + INIT, FREE, VALID, PARSE, NULL, NULL, NULL, NULL) SR_REGISTER_DEV_DRIVER_LIST(serial_dmm_drivers, /* diff --git a/src/hardware/serial-dmm/protocol.c b/src/hardware/serial-dmm/protocol.c index 71fc1a46..483bb009 100644 --- a/src/hardware/serial-dmm/protocol.c +++ b/src/hardware/serial-dmm/protocol.c @@ -39,24 +39,25 @@ static void log_dmm_packet(const uint8_t *buf, size_t len) sr_hexdump_free(text); } -static void handle_packet(const uint8_t *buf, struct sr_dev_inst *sdi, - void *info) +static void handle_packet(struct sr_dev_inst *sdi, + const uint8_t *buf, size_t len, void *info) { struct dmm_info *dmm; + struct dev_context *devc; float floatval; + double doubleval; struct sr_datafeed_packet packet; struct sr_datafeed_analog analog; struct sr_analog_encoding encoding; struct sr_analog_meaning meaning; struct sr_analog_spec spec; - struct dev_context *devc; gboolean sent_sample; struct sr_channel *channel; size_t ch_idx; dmm = (struct dmm_info *)sdi->driver; - log_dmm_packet(buf, dmm->packet_size); + log_dmm_packet(buf, len); devc = sdi->priv; sent_sample = FALSE; @@ -70,8 +71,16 @@ static void handle_packet(const uint8_t *buf, struct sr_dev_inst *sdi, analog.num_samples = 1; analog.meaning->mq = 0; - dmm->packet_parse(buf, &floatval, &analog, info); - analog.data = &floatval; + if (dmm->packet_parse) { + dmm->packet_parse(buf, &floatval, &analog, info); + analog.data = &floatval; + analog.encoding->unitsize = sizeof(floatval); + } else if (dmm->packet_parse_len) { + dmm->packet_parse_len(dmm->dmm_state, buf, len, + &doubleval, &analog, info); + analog.data = &doubleval; + analog.encoding->unitsize = sizeof(doubleval); + } /* If this DMM needs additional handling, call the resp. function. */ if (dmm->dmm_details) @@ -97,30 +106,34 @@ SR_PRIV int req_packet(struct sr_dev_inst *sdi) struct dmm_info *dmm; struct dev_context *devc; struct sr_serial_dev_inst *serial; + uint64_t now, left, next; int ret; dmm = (struct dmm_info *)sdi->driver; - if (!dmm->packet_request) return SR_OK; devc = sdi->priv; serial = sdi->conn; - if (devc->req_next_at && (devc->req_next_at > g_get_monotonic_time())) { - sr_spew("Not requesting new packet yet, %" PRIi64 " ms left.", - ((devc->req_next_at - g_get_monotonic_time()) / 1000)); + now = g_get_monotonic_time(); + if (devc->req_next_at && now < devc->req_next_at) { + left = (devc->req_next_at - now) / 1000; + sr_spew("Not re-requesting yet, %" PRIu64 "ms left.", left); return SR_OK; } + sr_spew("Requesting next packet."); ret = dmm->packet_request(serial); if (ret < 0) { sr_err("Failed to request packet: %d.", ret); return ret; } - if (dmm->req_timeout_ms) - devc->req_next_at = g_get_monotonic_time() + (dmm->req_timeout_ms * 1000); + if (dmm->req_timeout_ms) { + next = now + dmm->req_timeout_ms * 1000; + devc->req_next_at = next; + } return SR_OK; } @@ -129,48 +142,96 @@ static void handle_new_data(struct sr_dev_inst *sdi, void *info) { struct dmm_info *dmm; struct dev_context *devc; - int len, offset; struct sr_serial_dev_inst *serial; + int ret; + size_t read_len, check_pos, check_len, pkt_size, copy_len; + uint8_t *check_ptr; + uint64_t deadline; dmm = (struct dmm_info *)sdi->driver; devc = sdi->priv; serial = sdi->conn; - /* Try to get as much data as the buffer can hold. */ - len = DMM_BUFSIZE - devc->buflen; - len = serial_read_nonblocking(serial, devc->buf + devc->buflen, len); - if (len == 0) + /* Add the maximum available RX data we can get to the local buffer. */ + read_len = DMM_BUFSIZE - devc->buflen; + ret = serial_read_nonblocking(serial, &devc->buf[devc->buflen], read_len); + if (ret == 0) return; /* No new bytes, nothing to do. */ - if (len < 0) { - sr_err("Serial port read error: %d.", len); + if (ret < 0) { + sr_err("Serial port read error: %d.", ret); return; } - devc->buflen += len; - - /* Now look for packets in that data. */ - offset = 0; - while ((devc->buflen - offset) >= dmm->packet_size) { - if (dmm->packet_valid(devc->buf + offset)) { - handle_packet(devc->buf + offset, sdi, info); - offset += dmm->packet_size; - - /* Request next packet, if required. */ - if (!dmm->packet_request) + devc->buflen += ret; + + /* + * Process packets when their reception has completed, or keep + * trying to synchronize to the stream of input data. + */ + check_pos = 0; + while (check_pos < devc->buflen) { + /* Got the (minimum) amount of receive data for a packet? */ + check_len = devc->buflen - check_pos; + if (check_len < dmm->packet_size) + break; + sr_dbg("Checking: pos %zu, len %zu.", check_pos, check_len); + + /* Is it a valid packet? */ + check_ptr = &devc->buf[check_pos]; + if (dmm->packet_valid_len) { + ret = dmm->packet_valid_len(dmm->dmm_state, + check_ptr, check_len, &pkt_size); + if (ret == SR_PACKET_NEED_RX) { + sr_dbg("Need more RX data."); break; - if (dmm->req_timeout_ms || dmm->req_delay_ms) - devc->req_next_at = g_get_monotonic_time() + - dmm->req_delay_ms * 1000; - req_packet(sdi); - } else { - offset++; + } + if (ret == SR_PACKET_INVALID) { + sr_dbg("Not a valid packet, searching."); + check_pos++; + continue; + } + } else if (dmm->packet_valid) { + if (!dmm->packet_valid(check_ptr)) { + sr_dbg("Not a valid packet, searching."); + check_pos++; + continue; + } + pkt_size = dmm->packet_size; + } + + /* Process the package. */ + sr_dbg("Valid packet, size %zu, processing", pkt_size); + handle_packet(sdi, check_ptr, pkt_size, info); + check_pos += pkt_size; + + /* Arrange for the next packet request if needed. */ + if (!dmm->packet_request) + continue; + if (dmm->req_timeout_ms || dmm->req_delay_ms) { + deadline = g_get_monotonic_time(); + deadline += dmm->req_delay_ms * 1000; + devc->req_next_at = deadline; } + req_packet(sdi); + continue; } /* If we have any data left, move it to the beginning of our buffer. */ - if (devc->buflen > offset) - memmove(devc->buf, devc->buf + offset, devc->buflen - offset); - devc->buflen -= offset; + if (devc->buflen > check_pos) { + copy_len = devc->buflen - check_pos; + memmove(&devc->buf[0], &devc->buf[check_pos], copy_len); + } + devc->buflen -= check_pos; + + /* + * If the complete buffer filled up and none of it got processed, + * discard the unprocessed buffer, re-sync to the stream in later + * calls again. + */ + if (devc->buflen == sizeof(devc->buf)) { + sr_info("Drop unprocessed RX data, try to re-sync to stream."); + devc->buflen = 0; + } } int receive_data(int fd, int revents, void *cb_data) diff --git a/src/hardware/serial-dmm/protocol.h b/src/hardware/serial-dmm/protocol.h index 0713f5cf..092f506b 100644 --- a/src/hardware/serial-dmm/protocol.h +++ b/src/hardware/serial-dmm/protocol.h @@ -34,17 +34,17 @@ struct dmm_info { /** serialcomm string. */ const char *serialcomm; /** Packet size in bytes. */ - int packet_size; + size_t packet_size; /** * Request timeout [ms] before request is considered lost and a new * one is sent. Used only if device needs polling. */ - int64_t req_timeout_ms; + uint64_t req_timeout_ms; /** * Delay between reception of packet and next request. Some DMMs * need this. Used only if device needs polling. */ - int64_t req_delay_ms; + uint64_t req_delay_ms; /** Packet request function. */ int (*packet_request)(struct sr_serial_dev_inst *); /** Number of channels / displays. */ @@ -60,6 +60,24 @@ struct dmm_info { void (*dmm_details)(struct sr_datafeed_analog *, void *); /** Size of chipset info struct. */ gsize info_size; + /* Serial-dmm items "with state" and variable length packets. */ + void *dmm_state; + void *(*dmm_state_init)(void); + void (*dmm_state_free)(void *state); + int (*after_open)(struct sr_serial_dev_inst *serial); + int (*packet_valid_len)(void *state, const uint8_t *data, size_t dlen, + size_t *pkt_len); + int (*packet_parse_len)(void *state, const uint8_t *data, size_t dlen, + double *val, struct sr_datafeed_analog *analog, void *info); + int (*config_get)(void *state, uint32_t key, GVariant **data, + const struct sr_dev_inst *sdi, const struct sr_channel_group *cg); + int (*config_set)(void *state, uint32_t key, GVariant *data, + const struct sr_dev_inst *sdi, const struct sr_channel_group *cg); + int (*config_list)(void *state, uint32_t key, GVariant **data, + const struct sr_dev_inst *sdi, const struct sr_channel_group *cg); + /** Hook at acquisition start. Can re-route the receive routine. */ + int (*acquire_start)(void *state, const struct sr_dev_inst *sdi, + sr_receive_data_callback *cb, void **cb_data); }; #define DMM_BUFSIZE 256 @@ -68,14 +86,13 @@ struct dev_context { struct sr_sw_limits limits; uint8_t buf[DMM_BUFSIZE]; - int bufoffset; - int buflen; + size_t buflen; /** * The timestamp [µs] to send the next request. * Used only if device needs polling. */ - int64_t req_next_at; + uint64_t req_next_at; }; SR_PRIV int req_packet(struct sr_dev_inst *sdi);