rdtech-dps: research to reduce serial comm transfer volume again

The DPS devices default to a rather low UART communication bitrate. The
generous retrieval of more Modbus registers than strictly necessary for
the periodic acquisition of analog channel values and state tracking may
or may not be an issue.

Instrument all call sites in api.c to the device state gathering calls
such that the protocol.c side transparently can reduce the transfer
volume depending on which level of detail the caller may expect given
its current context. Under optimal circumstances this might cut the
traffic in half, which DPS devices may benefit from.

This commit adds the infrastructure to potentially reduce transfer
volume, but does not change behaviour. The "config" reasons may need
further partitioning if Modbus register access really turns out to be
a severe bottleneck. The API lends itself to that extension easily. The
approach which gets prepared in this commit needs to get runtime tested
by those who got access to the hardware.

diff --git a/src/hardware/rdtech-dps/api.c b/src/hardware/rdtech-dps/api.c
index 568ba575f4c5b6701a841b16732388f6425ea301..1c67ea49d1715d5b32cb3b18b445288623438cdc 100644 (file)
--- a/src/hardware/rdtech-dps/api.c
+++ b/src/hardware/rdtech-dps/api.c
@@ -279,7 +279,7 @@ static int config_get(uint32_t key, GVariant **data,
                ret = sr_sw_limits_config_get(&devc->limits, key, data);
                break;
        case SR_CONF_ENABLED:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_OUTPUT_ENABLED))
@@ -287,7 +287,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_boolean(state.output_enabled);
                break;
        case SR_CONF_REGULATION:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_REGULATION_CC))
@@ -296,7 +296,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_string(cc_text);
                break;
        case SR_CONF_VOLTAGE:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_VOLTAGE))
@@ -304,7 +304,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_double(state.voltage);
                break;
        case SR_CONF_VOLTAGE_TARGET:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_VOLTAGE_TARGET))
@@ -312,7 +312,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_double(state.voltage_target);
                break;
        case SR_CONF_CURRENT:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_CURRENT))
@@ -320,7 +320,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_double(state.current);
                break;
        case SR_CONF_CURRENT_LIMIT:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_CURRENT_LIMIT))
@@ -328,7 +328,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_double(state.current_limit);
                break;
        case SR_CONF_OVER_VOLTAGE_PROTECTION_ENABLED:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_PROTECT_ENABLED))
@@ -336,7 +336,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_boolean(state.protect_enabled);
                break;
        case SR_CONF_OVER_VOLTAGE_PROTECTION_ACTIVE:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_PROTECT_OVP))
@@ -344,7 +344,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_boolean(state.protect_ovp);
                break;
        case SR_CONF_OVER_VOLTAGE_PROTECTION_THRESHOLD:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_OUTPUT_ENABLED))
@@ -352,7 +352,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_double(state.ovp_threshold);
                break;
        case SR_CONF_OVER_CURRENT_PROTECTION_ENABLED:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_PROTECT_ENABLED))
@@ -360,7 +360,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_boolean(state.protect_enabled);
                break;
        case SR_CONF_OVER_CURRENT_PROTECTION_ACTIVE:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_PROTECT_OCP))
@@ -368,7 +368,7 @@ static int config_get(uint32_t key, GVariant **data,
                *data = g_variant_new_boolean(state.protect_ocp);
                break;
        case SR_CONF_OVER_CURRENT_PROTECTION_THRESHOLD:
-               ret = rdtech_dps_get_state(sdi, &state);
+               ret = rdtech_dps_get_state(sdi, &state, ST_CTX_CONFIG);
                if (ret != SR_OK)
                        return ret;
                if (!(state.mask & STATE_OCP_THRESHOLD))

diff --git a/src/hardware/rdtech-dps/protocol.c b/src/hardware/rdtech-dps/protocol.c
index 9f45fc4f6d8fa974f37b78b807859faabd5f3e9b..7005a7a114fdfec65764e56b1c164159b1c51f54 100644 (file)
--- a/src/hardware/rdtech-dps/protocol.c
+++ b/src/hardware/rdtech-dps/protocol.c
@@ -254,10 +254,11 @@ static int send_value(const struct sr_dev_inst *sdi,
  * when the UART bitrate is only 9600bps?
  */
 SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
-       struct rdtech_dps_state *state)
+       struct rdtech_dps_state *state, enum rdtech_dps_state_context reason)
 {
        struct dev_context *devc;
        struct sr_modbus_dev_inst *modbus;
+       gboolean get_config, get_init_state, get_curr_meas;
        uint16_t registers[12];
        int ret;
        const uint8_t *rdptr;
@@ -276,6 +277,37 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
        if (!state)
                return SR_ERR_ARG;
 
+       /* Determine the requested level of response detail. */
+       get_config = FALSE;
+       get_init_state = FALSE;
+       get_curr_meas = FALSE;
+       switch (reason) {
+       case ST_CTX_CONFIG:
+               get_config = TRUE;
+               get_init_state = TRUE;
+               get_curr_meas = TRUE;
+               break;
+       case ST_CTX_PRE_ACQ:
+               get_init_state = TRUE;
+               get_curr_meas = TRUE;
+               break;
+       case ST_CTX_IN_ACQ:
+               get_curr_meas = TRUE;
+               break;
+       default:
+               /* EMPTY */
+               break;
+       }
+       /*
+        * TODO Make use of this information to reduce the transfer
+        * volume, especially on low bitrate serial connections. Though
+        * the device firmware's samplerate is probably more limiting
+        * than communication bandwidth is.
+        */
+       (void)get_config;
+       (void)get_init_state;
+       (void)get_curr_meas;
+
        switch (devc->model->model_type) {
        case MODEL_DPS:
                /*
@@ -285,9 +317,6 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
                 * and the sequence of the registers and how to interpret
                 * their bit fields. But then this is not too unusual for
                 * a hardware specific device driver ...
-                *
-                * TODO Optionally reduce the transfer volume depending
-                * on the caller specified state query context.
                 */
                g_mutex_lock(&devc->rw_mutex);
                ret = rdtech_dps_read_holding_registers(modbus,
@@ -393,7 +422,13 @@ SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
                return SR_ERR_ARG;
        }
 
-       /* Store gathered details in the high level container. */
+       /*
+        * Store gathered details in the high level container.
+        *
+        * TODO Make use of the caller's context. The register access
+        * code path above need not have gathered every detail in every
+        * invocation.
+        */
        memset(state, 0, sizeof(*state));
        state->lock = is_lock;
        state->mask |= STATE_LOCK;
@@ -551,7 +586,7 @@ SR_PRIV int rdtech_dps_seed_receive(const struct sr_dev_inst *sdi)
        struct rdtech_dps_state state;
        int ret;
 
-       ret = rdtech_dps_get_state(sdi, &state);
+       ret = rdtech_dps_get_state(sdi, &state, ST_CTX_PRE_ACQ);
        if (ret != SR_OK)
                return ret;
 
@@ -586,7 +621,7 @@ SR_PRIV int rdtech_dps_receive_data(int fd, int revents, void *cb_data)
        devc = sdi->priv;
 
        /* Get the device's current state. */
-       ret = rdtech_dps_get_state(sdi, &state);
+       ret = rdtech_dps_get_state(sdi, &state, ST_CTX_IN_ACQ);
        if (ret != SR_OK)
                return ret;
 

diff --git a/src/hardware/rdtech-dps/protocol.h b/src/hardware/rdtech-dps/protocol.h
index 26c196ff43755d1585206c0e60df30f6bcf54e03..456e20419bf6f785de425588111ccaa195ebe920 100644 (file)
--- a/src/hardware/rdtech-dps/protocol.h
+++ b/src/hardware/rdtech-dps/protocol.h
@@ -86,8 +86,14 @@ struct rdtech_dps_state {
        float voltage, current, power;
 };
 
+enum rdtech_dps_state_context {
+       ST_CTX_NONE,
+       ST_CTX_CONFIG,
+       ST_CTX_PRE_ACQ,
+       ST_CTX_IN_ACQ,
+};
 SR_PRIV int rdtech_dps_get_state(const struct sr_dev_inst *sdi,
-       struct rdtech_dps_state *state);
+       struct rdtech_dps_state *state, enum rdtech_dps_state_context reason);
 SR_PRIV int rdtech_dps_set_state(const struct sr_dev_inst *sdi,
        struct rdtech_dps_state *state);