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 2dc8f09b3a6fee4330d5c7c9e2a65b9cd0d4bee6..1b3daa939a8b9eb58758e536e2078ac95107a504 100644 (file)
--- 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 3a4c2d4631473ce4692e774232257dcdb37a8107..72076a4563ee1adb1bb83f97d4ecee7d953067ab 100644 (file)
--- a/src/hardware/maynuo-m97/protocol.c
+++ b/src/hardware/maynuo-m97/protocol.c
@@ -20,73 +20,126 @@
 #include <config.h>
 #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 31589d901d745e19312e51b1df260efa32fa378d..892f0b4b5fcf4c9929da1ad326ad6c5d5349c11b 100644 (file)
--- 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