From: Gerhard Sittig <redacted>
Date: Thu, 28 Sep 2023 10:30:05 +0000 (+0200)
Subject: rdtech-dps: touch up comments
X-Git-Url: https://sigrok.org/gitweb/?a=commitdiff_plain;h=62f3228bc94224e54a1474365a84b7157791dc34;p=libsigrok.git

rdtech-dps: touch up comments

Address style issues in comments. Extend and rephrase some for improved
readability and awareness during maintenance. Move comments to before
the code which they relate to.
---

diff --git a/src/hardware/rdtech-dps/api.c b/src/hardware/rdtech-dps/api.c
index 79dac48b..91a2aac4 100644
--- a/src/hardware/rdtech-dps/api.c
+++ b/src/hardware/rdtech-dps/api.c
@@ -68,7 +68,7 @@ static const uint32_t devopts_w_range[] = {
 	SR_CONF_RANGE | SR_CONF_GET | SR_CONF_SET | SR_CONF_LIST,
 };
 
-/* range name, max current/voltage/power, current/voltage digits */
+/* Range name, max current/voltage/power, current/voltage digits. */
 static struct rdtech_dps_range ranges_dps3005[] = {
 	{  "5A",  5, 30,  160, 3, 2 }
 };
@@ -102,8 +102,9 @@ static struct rdtech_dps_range ranges_rd6012[] = {
 };
 
 /*
- * Current digits for RD6012P is 4 for the 6A range (RTU reg  20 = 0) and
- * 3 for the 12A range (RTU reg 20 = 1)
+ * RD6012P supports multiple current ranges with differing resolution.
+ * Up to 6A with 4 digits (when RTU reg 20 == 0), up to 12A with 3 digits
+ * (when RTU reg 20 == 1).
  */
 static struct rdtech_dps_range ranges_rd6012p[] = {
 	{  "6A",  6, 60,  360, 4, 3 },
@@ -118,7 +119,7 @@ static struct rdtech_dps_range ranges_rd6024[] = {
 	{ "24A", 24, 60, 1440, 2, 2 }
 };
 
-/* model ID, model name, range */
+/* Model ID, model name, model dependent ranges. */
 static const struct rdtech_dps_model supported_models[] = {
 	{ MODEL_DPS, 3005, "DPS3005", ARRAY_AND_SIZE(ranges_dps3005) },
 	{ MODEL_DPS, 5005, "DPS5005", ARRAY_AND_SIZE(ranges_dps5005) },
diff --git a/src/hardware/rdtech-dps/protocol.c b/src/hardware/rdtech-dps/protocol.c
index 236cc90a..d5e63bf8 100644
--- 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 */
@@ -649,25 +650,30 @@ 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:
+			/*
+			 * Need not set the range when the device only
+			 * supports a single fixed range.
+			 */
 			if (devc->model->n_ranges == 1)
-				/* No need to set. */
 				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;
 				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;