From: François Revol Date: Mon, 25 Aug 2025 15:12:01 +0000 (+0200) Subject: maynuo-m97: Synchronize read and write operations X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=524ec15486ce7a7340edbba228e91dbecc2a68c6;p=libsigrok.git maynuo-m97: Synchronize read and write operations This patch is for thread safety when the library is used by SmuView and should fix https://github.com/knarfS/smuview/issues/27 --- diff --git a/src/hardware/maynuo-m97/api.c b/src/hardware/maynuo-m97/api.c index 2dc8f09b..1b3daa93 100644 --- a/src/hardware/maynuo-m97/api.c +++ b/src/hardware/maynuo-m97/api.c @@ -201,14 +201,13 @@ static int dev_open(struct sr_dev_inst *sdi) if (sr_modbus_open(modbus) < 0) return SR_ERR; - maynuo_m97_set_bit(modbus, PC1, 1); + maynuo_m97_set_bit(sdi, PC1, 1); return SR_OK; } static int dev_close(struct sr_dev_inst *sdi) { - struct dev_context *devc; struct sr_modbus_dev_inst *modbus; modbus = sdi->conn; @@ -216,16 +215,7 @@ static int dev_close(struct sr_dev_inst *sdi) if (!modbus) return SR_ERR_BUG; - devc = sdi->priv; - - if (devc->expecting_registers) { - /* Wait for the last data that was requested from the device. */ - uint16_t registers[devc->expecting_registers]; - sr_modbus_read_holding_registers(modbus, -1, - devc->expecting_registers, registers); - } - - maynuo_m97_set_bit(modbus, PC1, 0); + maynuo_m97_set_bit(sdi, PC1, 0); return sr_modbus_close(modbus); } @@ -234,14 +224,12 @@ 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 sr_modbus_dev_inst *modbus; enum maynuo_m97_mode mode; int ret, ivalue; float fvalue; (void)cg; - modbus = sdi->conn; devc = sdi->priv; ret = SR_OK; @@ -251,60 +239,60 @@ static int config_get(uint32_t key, GVariant **data, ret = sr_sw_limits_config_get(&devc->limits, key, data); break; case SR_CONF_ENABLED: - if ((ret = maynuo_m97_get_bit(modbus, ISTATE, &ivalue)) == SR_OK) + if ((ret = maynuo_m97_get_bit(sdi, ISTATE, &ivalue)) == SR_OK) *data = g_variant_new_boolean(ivalue); break; case SR_CONF_REGULATION: - if ((ret = maynuo_m97_get_bit(modbus, UNREG, &ivalue)) != SR_OK) + if ((ret = maynuo_m97_get_bit(sdi, UNREG, &ivalue)) != SR_OK) break; if (ivalue) *data = g_variant_new_string("UR"); - else if ((ret = maynuo_m97_get_mode(modbus, &mode)) == SR_OK) + else if ((ret = maynuo_m97_get_mode(sdi, &mode)) == SR_OK) *data = g_variant_new_string(maynuo_m97_mode_to_str(mode)); break; case SR_CONF_VOLTAGE: - if ((ret = maynuo_m97_get_float(modbus, U, &fvalue)) == SR_OK) + if ((ret = maynuo_m97_get_float(sdi, U, &fvalue)) == SR_OK) *data = g_variant_new_double(fvalue); break; case SR_CONF_VOLTAGE_TARGET: - if ((ret = maynuo_m97_get_float(modbus, UFIX, &fvalue)) == SR_OK) + if ((ret = maynuo_m97_get_float(sdi, UFIX, &fvalue)) == SR_OK) *data = g_variant_new_double(fvalue); break; case SR_CONF_CURRENT: - if ((ret = maynuo_m97_get_float(modbus, I, &fvalue)) == SR_OK) + if ((ret = maynuo_m97_get_float(sdi, I, &fvalue)) == SR_OK) *data = g_variant_new_double(fvalue); break; case SR_CONF_CURRENT_LIMIT: - if ((ret = maynuo_m97_get_float(modbus, IFIX, &fvalue)) == SR_OK) + if ((ret = maynuo_m97_get_float(sdi, IFIX, &fvalue)) == SR_OK) *data = g_variant_new_double(fvalue); break; case SR_CONF_OVER_VOLTAGE_PROTECTION_ENABLED: *data = g_variant_new_boolean(TRUE); break; case SR_CONF_OVER_VOLTAGE_PROTECTION_ACTIVE: - if ((ret = maynuo_m97_get_bit(modbus, UOVER, &ivalue)) == SR_OK) + if ((ret = maynuo_m97_get_bit(sdi, UOVER, &ivalue)) == SR_OK) *data = g_variant_new_boolean(ivalue); break; case SR_CONF_OVER_VOLTAGE_PROTECTION_THRESHOLD: - if ((ret = maynuo_m97_get_float(modbus, UMAX, &fvalue)) == SR_OK) + if ((ret = maynuo_m97_get_float(sdi, UMAX, &fvalue)) == SR_OK) *data = g_variant_new_double(fvalue); break; case SR_CONF_OVER_CURRENT_PROTECTION_ENABLED: *data = g_variant_new_boolean(TRUE); break; case SR_CONF_OVER_CURRENT_PROTECTION_ACTIVE: - if ((ret = maynuo_m97_get_bit(modbus, IOVER, &ivalue)) == SR_OK) + if ((ret = maynuo_m97_get_bit(sdi, IOVER, &ivalue)) == SR_OK) *data = g_variant_new_boolean(ivalue); break; case SR_CONF_OVER_CURRENT_PROTECTION_THRESHOLD: - if ((ret = maynuo_m97_get_float(modbus, IMAX, &fvalue)) == SR_OK) + if ((ret = maynuo_m97_get_float(sdi, IMAX, &fvalue)) == SR_OK) *data = g_variant_new_double(fvalue); break; case SR_CONF_OVER_TEMPERATURE_PROTECTION: *data = g_variant_new_boolean(TRUE); break; case SR_CONF_OVER_TEMPERATURE_PROTECTION_ACTIVE: - if ((ret = maynuo_m97_get_bit(modbus, HEAT, &ivalue)) == SR_OK) + if ((ret = maynuo_m97_get_bit(sdi, HEAT, &ivalue)) == SR_OK) *data = g_variant_new_boolean(ivalue); break; default: @@ -318,11 +306,9 @@ 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 sr_modbus_dev_inst *modbus; (void)cg; - modbus = sdi->conn; devc = sdi->priv; switch (key) { @@ -330,15 +316,15 @@ static int config_set(uint32_t key, GVariant *data, case SR_CONF_LIMIT_MSEC: return sr_sw_limits_config_set(&devc->limits, key, data); case SR_CONF_ENABLED: - return maynuo_m97_set_input(modbus, g_variant_get_boolean(data)); + return maynuo_m97_set_input(sdi, g_variant_get_boolean(data)); case SR_CONF_VOLTAGE_TARGET: - return maynuo_m97_set_float(modbus, UFIX, g_variant_get_double(data)); + return maynuo_m97_set_float(sdi, UFIX, g_variant_get_double(data)); case SR_CONF_CURRENT_LIMIT: - return maynuo_m97_set_float(modbus, IFIX, g_variant_get_double(data)); + return maynuo_m97_set_float(sdi, IFIX, g_variant_get_double(data)); case SR_CONF_OVER_VOLTAGE_PROTECTION_THRESHOLD: - return maynuo_m97_set_float(modbus, UMAX, g_variant_get_double(data)); + return maynuo_m97_set_float(sdi, UMAX, g_variant_get_double(data)); case SR_CONF_OVER_CURRENT_PROTECTION_THRESHOLD: - return maynuo_m97_set_float(modbus, IMAX, g_variant_get_double(data)); + return maynuo_m97_set_float(sdi, IMAX, g_variant_get_double(data)); default: return SR_ERR_NA; } @@ -394,7 +380,7 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi) sr_sw_limits_acquisition_start(&devc->limits); std_session_send_df_header(sdi); - return maynuo_m97_capture_start(sdi); + return SR_OK; } static int dev_acquisition_stop(struct sr_dev_inst *sdi) diff --git a/src/hardware/maynuo-m97/protocol.c b/src/hardware/maynuo-m97/protocol.c index 3a4c2d46..72076a45 100644 --- a/src/hardware/maynuo-m97/protocol.c +++ b/src/hardware/maynuo-m97/protocol.c @@ -20,73 +20,126 @@ #include #include "protocol.h" -SR_PRIV int maynuo_m97_get_bit(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_get_bit(const struct sr_dev_inst *sdi, enum maynuo_m97_coil address, int *value) { + struct dev_context *devc; + struct sr_modbus_dev_inst *modbus; uint8_t coil; + + devc = sdi->priv; + modbus = sdi->conn; + + g_mutex_lock(&devc->rw_mutex); int ret = sr_modbus_read_coils(modbus, address, 1, &coil); + g_mutex_unlock(&devc->rw_mutex); *value = coil & 1; return ret; } -SR_PRIV int maynuo_m97_set_bit(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_set_bit(const struct sr_dev_inst *sdi, enum maynuo_m97_coil address, int value) { - return sr_modbus_write_coil(modbus, address, value); + struct dev_context *devc; + struct sr_modbus_dev_inst *modbus; + + devc = sdi->priv; + modbus = sdi->conn; + + g_mutex_lock(&devc->rw_mutex); + int ret = sr_modbus_write_coil(modbus, address, value); + g_mutex_unlock(&devc->rw_mutex); + return ret; } -SR_PRIV int maynuo_m97_get_float(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_get_float(const struct sr_dev_inst *sdi, enum maynuo_m97_register address, float *value) { + struct dev_context *devc; + struct sr_modbus_dev_inst *modbus; uint16_t registers[2]; + + devc = sdi->priv; + modbus = sdi->conn; + + g_mutex_lock(&devc->rw_mutex); int ret = sr_modbus_read_holding_registers(modbus, address, 2, registers); + g_mutex_unlock(&devc->rw_mutex); if (ret == SR_OK) *value = RBFL(registers); return ret; } -SR_PRIV int maynuo_m97_set_float(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_set_float(const struct sr_dev_inst *sdi, enum maynuo_m97_register address, float value) { + struct dev_context *devc; + struct sr_modbus_dev_inst *modbus; uint16_t registers[2]; + int ret; + + devc = sdi->priv; + modbus = sdi->conn; + WBFL(registers, value); - return sr_modbus_write_multiple_registers(modbus, address, 2, registers); + g_mutex_lock(&devc->rw_mutex); + ret = sr_modbus_write_multiple_registers(modbus, address, 2, registers); + g_mutex_unlock(&devc->rw_mutex); + return ret; } -static int maynuo_m97_cmd(struct sr_modbus_dev_inst *modbus, +static int maynuo_m97_cmd(const struct sr_dev_inst *sdi, enum maynuo_m97_mode cmd) { + struct dev_context *devc; + struct sr_modbus_dev_inst *modbus; uint16_t registers[1]; + int ret; + + devc = sdi->priv; + modbus = sdi->conn; + WB16(registers, cmd); - return sr_modbus_write_multiple_registers(modbus, CMD, 1, registers); + g_mutex_lock(&devc->rw_mutex); + ret = sr_modbus_write_multiple_registers(modbus, CMD, 1, registers); + g_mutex_unlock(&devc->rw_mutex); + return ret; } -SR_PRIV int maynuo_m97_get_mode(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_get_mode(const struct sr_dev_inst *sdi, enum maynuo_m97_mode *mode) { + struct dev_context *devc; + struct sr_modbus_dev_inst *modbus; uint16_t registers[1]; int ret; + + devc = sdi->priv; + modbus = sdi->conn; + + g_mutex_lock(&devc->rw_mutex); ret = sr_modbus_read_holding_registers(modbus, SETMODE, 1, registers); + g_mutex_unlock(&devc->rw_mutex); *mode = RB16(registers) & 0xFF; return ret; } -SR_PRIV int maynuo_m97_set_mode(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_set_mode(const struct sr_dev_inst *sdi, enum maynuo_m97_mode mode) { - return maynuo_m97_cmd(modbus, mode); + return maynuo_m97_cmd(sdi, mode); } -SR_PRIV int maynuo_m97_set_input(struct sr_modbus_dev_inst *modbus, int enable) +SR_PRIV int maynuo_m97_set_input(const struct sr_dev_inst *sdi, int enable) { enum maynuo_m97_mode mode; int ret; - if ((ret = maynuo_m97_get_mode(modbus, &mode)) != SR_OK) + if ((ret = maynuo_m97_get_mode(sdi, &mode)) != SR_OK) return ret; - if ((ret = maynuo_m97_cmd(modbus, enable ? INPUT_ON : INPUT_OFF)) != SR_OK) + if ((ret = maynuo_m97_cmd(sdi, enable ? INPUT_ON : INPUT_OFF)) != SR_OK) return ret; - return maynuo_m97_set_mode(modbus, mode); + return maynuo_m97_set_mode(sdi, mode); } SR_PRIV int maynuo_m97_get_model_version(struct sr_modbus_dev_inst *modbus, @@ -94,6 +147,12 @@ SR_PRIV int maynuo_m97_get_model_version(struct sr_modbus_dev_inst *modbus, { uint16_t registers[2]; int ret; + + /* + * No mutex here, because there is no sr_dev_inst when this function + * is called. + */ + ret = sr_modbus_read_holding_registers(modbus, MODEL, 2, registers); *model = RB16(registers + 0); *version = RB16(registers + 1); @@ -147,26 +206,13 @@ static void maynuo_m97_session_send_value(const struct sr_dev_inst *sdi, struct g_slist_free(analog.meaning->channels); } -SR_PRIV int maynuo_m97_capture_start(const struct sr_dev_inst *sdi) -{ - struct dev_context *devc; - struct sr_modbus_dev_inst *modbus; - int ret; - - modbus = sdi->conn; - devc = sdi->priv; - - if ((ret = sr_modbus_read_holding_registers(modbus, U, 4, NULL)) == SR_OK) - devc->expecting_registers = 4; - return ret; -} - SR_PRIV int maynuo_m97_receive_data(int fd, int revents, void *cb_data) { struct sr_dev_inst *sdi; struct dev_context *devc; struct sr_modbus_dev_inst *modbus; uint16_t registers[4]; + int ret; (void)fd; (void)revents; @@ -177,8 +223,11 @@ SR_PRIV int maynuo_m97_receive_data(int fd, int revents, void *cb_data) modbus = sdi->conn; devc = sdi->priv; - devc->expecting_registers = 0; - if (sr_modbus_read_holding_registers(modbus, -1, 4, registers) == SR_OK) { + g_mutex_lock(&devc->rw_mutex); + ret = sr_modbus_read_holding_registers(modbus, -1, 4, registers); + g_mutex_unlock(&devc->rw_mutex); + + if (ret == SR_OK) { std_session_send_df_frame_begin(sdi); maynuo_m97_session_send_value(sdi, sdi->channels->data, @@ -197,6 +246,5 @@ SR_PRIV int maynuo_m97_receive_data(int fd, int revents, void *cb_data) return TRUE; } - maynuo_m97_capture_start(sdi); return TRUE; } diff --git a/src/hardware/maynuo-m97/protocol.h b/src/hardware/maynuo-m97/protocol.h index 31589d90..892f0b4b 100644 --- a/src/hardware/maynuo-m97/protocol.h +++ b/src/hardware/maynuo-m97/protocol.h @@ -37,7 +37,7 @@ struct maynuo_m97_model { struct dev_context { const struct maynuo_m97_model *model; struct sr_sw_limits limits; - int expecting_registers; + GMutex rw_mutex; }; enum maynuo_m97_coil { @@ -130,26 +130,25 @@ enum maynuo_m97_mode { INPUT_OFF = 43, }; -SR_PRIV int maynuo_m97_get_bit(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_get_bit(const struct sr_dev_inst *sdi, enum maynuo_m97_coil address, int *value); -SR_PRIV int maynuo_m97_set_bit(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_set_bit(const struct sr_dev_inst *sdi, enum maynuo_m97_coil address, int value); -SR_PRIV int maynuo_m97_get_float(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_get_float(const struct sr_dev_inst *sdi, enum maynuo_m97_register address, float *value); -SR_PRIV int maynuo_m97_set_float(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_set_float(const struct sr_dev_inst *sdi, enum maynuo_m97_register address, float value); -SR_PRIV int maynuo_m97_get_mode(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_get_mode(const struct sr_dev_inst *sdi, enum maynuo_m97_mode *mode); -SR_PRIV int maynuo_m97_set_mode(struct sr_modbus_dev_inst *modbus, +SR_PRIV int maynuo_m97_set_mode(const struct sr_dev_inst *sdi, enum maynuo_m97_mode mode); -SR_PRIV int maynuo_m97_set_input(struct sr_modbus_dev_inst *modbus, int enable); +SR_PRIV int maynuo_m97_set_input(const struct sr_dev_inst *sdi, int enable); SR_PRIV int maynuo_m97_get_model_version(struct sr_modbus_dev_inst *modbus, uint16_t *model, uint16_t *version); SR_PRIV const char *maynuo_m97_mode_to_str(enum maynuo_m97_mode mode); -SR_PRIV int maynuo_m97_capture_start(const struct sr_dev_inst *sdi); SR_PRIV int maynuo_m97_receive_data(int fd, int revents, void *cb_data); #endif