rdtech-dps: rephrase how model specific ranges are handled

[libsigrok.git] / src / hardware / rdtech-dps / protocol.c

diff --git a/src/hardware/rdtech-dps/protocol.c b/src/hardware/rdtech-dps/protocol.c
index 236cc90a4e5e281e8242ae9fa78962c0ab60be43..dfcffbb16c2f8582924f77a37f49569f5ba66582 100644 (file)
--- a/src/hardware/rdtech-dps/protocol.c
+++ b/src/hardware/rdtech-dps/protocol.c
@@ -26,7 +26,7 @@
 
 #include "protocol.h"
 
-/* These are the Modbus RTU registers for the family of rdtech-dps devices. */
+/* These are the Modbus RTU registers for the DPS family of devices. */
 enum rdtech_dps_register {
        REG_DPS_USET       = 0x00, /* Mirror of 0x50 */
        REG_DPS_ISET       = 0x01, /* Mirror of 0x51 */
@@ -72,8 +72,9 @@ enum rdtech_dps_regulation_mode {
 };
 
 /*
- * Some registers are specific to a certain device. For example,
- * REG_RD_RANGE is specific to RD6012P.
+ * These are the Modbus RTU registers for the RD family of devices.
+ * Some registers are device specific, like REG_RD_RANGE of RD6012P
+ * which could be battery related in other devices.
  */
 enum rdtech_rd_register {
        REG_RD_MODEL = 0, /* u16 */
@@ -116,12 +117,12 @@ static int rdtech_dps_read_holding_registers(struct sr_modbus_dev_inst *modbus,
        int ret;
 
        retries = 3;
-       do {
+       while (retries--) {
                ret = sr_modbus_read_holding_registers(modbus,
                        address, nb_registers, registers);
                if (ret == SR_OK)
                        return ret;
-       } while (--retries);
+       }
 
        return ret;
 }
@@ -224,7 +225,7 @@ SR_PRIV int rdtech_dps_get_model_version(struct sr_modbus_dev_inst *modbus,
 SR_PRIV void rdtech_dps_update_multipliers(const struct sr_dev_inst *sdi)
 {
        struct dev_context *devc;
-       struct rdtech_dps_range *range;
+       const struct rdtech_dps_range *range;
 
        devc = sdi->priv;
        range = &devc->model->ranges[devc->curr_range];
@@ -248,7 +249,7 @@ SR_PRIV int rdtech_dps_update_range(const struct sr_dev_inst *sdi)
         * Only update range if there are multiple ranges and data
         * acquisition hasn't started.
         */
-       if (devc->model->n_ranges == 1 || devc->acquisition_started)
+       if (devc->model->n_ranges <= 1 || devc->acquisition_started)
                return SR_OK;
        if (devc->model->model_type != MODEL_RD)
                return SR_ERR;
@@ -257,7 +258,8 @@ SR_PRIV int rdtech_dps_update_range(const struct sr_dev_inst *sdi)
                REG_RD_RANGE, 1, &range);
        if (ret != SR_OK)
                return ret;
-       devc->curr_range = range ? 1 : 0;
+       range = range ? 1 : 0;
+       devc->curr_range = range;
        rdtech_dps_update_multipliers(sdi);
 
        return SR_OK;
@@ -316,6 +318,7 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
        uint16_t reg_val, reg_state, out_state, ovpset_raw, ocpset_raw;
        gboolean is_lock, is_out_enabled, is_reg_cc;
        gboolean uses_ovp, uses_ocp;
+       gboolean have_range;
        uint16_t range;
        float volt_target, curr_limit;
        float ovp_threshold, ocp_threshold;
@@ -359,12 +362,9 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
        (void)get_init_state;
        (void)get_curr_meas;
 
-       /*
-        * The model RD6012P has two voltage/current ranges. We set a
-        * default value here such that the compiler doesn't generate
-        * an uninitialized variable warning.
-        */
-       range = 0;
+       have_range = devc->model->n_ranges > 1;
+       if (!have_range)
+               range = 0;
 
        switch (devc->model->model_type) {
        case MODEL_DPS:
@@ -377,8 +377,9 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
                 * a hardware specific device driver ...
                 */
                g_mutex_lock(&devc->rw_mutex);
-               ret = rdtech_dps_read_holding_registers(modbus, REG_DPS_USET,
-                       REG_DPS_ENABLE - REG_DPS_USET + 1, registers);
+               ret = rdtech_dps_read_holding_registers(modbus,
+                       REG_DPS_USET, REG_DPS_ENABLE - REG_DPS_USET + 1,
+                       registers);
                g_mutex_unlock(&devc->rw_mutex);
                if (ret != SR_OK)
                        return ret;
@@ -458,8 +459,8 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
                is_reg_cc = reg_state == MODE_CC;
                out_state = read_u16be_inc(&rdptr); /* ENABLE */
                is_out_enabled = out_state != 0;
-               if (devc->model->n_ranges > 1) {
-                       rdptr += sizeof (uint16_t); /* PRESET */
+               if (have_range) {
+                       (void)read_u16be_inc(&rdptr); /* PRESET */
                        range = read_u16be_inc(&rdptr) ? 1 : 0; /* RANGE */
                }
 
@@ -522,7 +523,7 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
        state->mask |= STATE_CURRENT;
        state->power = curr_power;
        state->mask |= STATE_POWER;
-       if (devc->model->n_ranges > 1) {
+       if (have_range) {
                state->range = range;
                state->mask |= STATE_RANGE;
        }
@@ -649,25 +650,33 @@ SR_PRIV int rdtech_dps_set_state(const struct sr_dev_inst *sdi,
                reg_value = state->range;
                switch (devc->model->model_type) {
                case MODEL_DPS:
+                       /* DPS models don't support current ranges at all. */
                        if (reg_value > 0)
                                return SR_ERR_ARG;
                        break;
                case MODEL_RD:
-                       if (devc->model->n_ranges == 1)
-                               /* No need to set. */
+                       /*
+                        * Reject unsupported range indices.
+                        * Need not set the range when the device only
+                        * supports a single fixed range.
+                        */
+                       if (reg_value >= devc->model->n_ranges)
+                               return SR_ERR_NA;
+                       if (devc->model->n_ranges <= 1)
                                return SR_OK;
                        ret = rdtech_rd_set_reg(sdi, REG_RD_RANGE, reg_value);
                        if (ret != SR_OK)
                                return ret;
+                       /*
+                        * Immediately update internal state outside of
+                        * an acquisition. Assume that in-acquisition
+                        * activity will update internal state. This is
+                        * essential for meta package emission.
+                        */
                        if (!devc->acquisition_started) {
-                               devc->curr_range = reg_value ? 1 : 0;
+                               devc->curr_range = reg_value;
                                rdtech_dps_update_multipliers(sdi);
                        }
-                       /*
-                        * We rely on the data acquisition to update
-                        * devc->curr_range. If we do it here, there
-                        * will be no range meta package.
-                        */
                        break;
                default:
                        return SR_ERR_ARG;
@@ -687,7 +696,6 @@ SR_PRIV int rdtech_dps_seed_receive(const struct sr_dev_inst *sdi)
        if (!sdi || !sdi->priv)
                return SR_ERR_ARG;
        devc = sdi->priv;
-       devc->acquisition_started = TRUE;
 
        ret = rdtech_dps_get_state(sdi, &state, ST_CTX_PRE_ACQ);
        if (ret != SR_OK)
@@ -717,7 +725,7 @@ SR_PRIV int rdtech_dps_receive_data(int fd, int revents, void *cb_data)
        struct rdtech_dps_state state;
        int ret;
        struct sr_channel *ch;
-       const char *regulation_text;
+       const char *regulation_text, *range_text;
 
        (void)fd;
        (void)revents;
@@ -773,9 +781,9 @@ SR_PRIV int rdtech_dps_receive_data(int fd, int revents, void *cb_data)
                devc->curr_out_state = state.output_enabled;
        }
        if (devc->curr_range != state.range) {
+               range_text = devc->model->ranges[state.range].range_str;
                (void)sr_session_send_meta(sdi, SR_CONF_RANGE,
-                       g_variant_new_string(
-                               devc->model->ranges[state.range].range_str));
+                       g_variant_new_string(range_text));
                devc->curr_range = state.range;
                rdtech_dps_update_multipliers(sdi);
        }