Skip to content

Commit c179e47

Browse files
committed
korad-kaxxxxp: rephrase how config setter transmits commands to device
The korad_kaxxxxp_set_value() routine was phrased in redundant and hard to read ways that were error prone during maintainance. A variables pair for "command" and "value" variables was assigned to, depending on the involved parameter of the config setter. And only later in another code section the text message got combined and was sent to the device. Error paths which handled unknown keys or non-settable parameters returned early but had to balance resources on their way out. This rephrased implementation immediately constructs the text message when dispatching parameters and looking up their values. Can use more appropriate printf(3) formats in the respective construction. Emits the same amount of diagnostics. Releases resources upon exit which were released during execution. Eliminates dynamic allocation of a 20 byte buffer. Tested-By: Pilatomic <[email protected]>
1 parent 88276d9 commit c179e47

File tree

1 file changed

+37
-40
lines changed

1 file changed

+37
-40
lines changed

src/hardware/korad-kaxxxxp/protocol.c

Lines changed: 37 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -167,84 +167,81 @@ static void give_device_time_to_process(struct dev_context *devc)
167167
SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
168168
int target, struct dev_context *devc)
169169
{
170-
char *msg;
171-
const char *cmd;
172-
float value;
170+
char msg[20];
173171
int ret;
174172

175173
g_mutex_lock(&devc->rw_mutex);
176174
give_device_time_to_process(devc);
177175

176+
msg[0] = '\0';
177+
ret = SR_OK;
178178
switch (target) {
179179
case KAXXXXP_CURRENT:
180180
case KAXXXXP_VOLTAGE:
181181
case KAXXXXP_STATUS:
182-
sr_err("Can't set measurable parameter %d.", target);
183-
g_mutex_unlock(&devc->rw_mutex);
184-
return SR_ERR;
182+
sr_err("Can't set measured value %d.", target);
183+
ret = SR_ERR;
184+
break;
185185
case KAXXXXP_CURRENT_LIMIT:
186-
cmd = "ISET1:%05.3f";
187-
value = devc->set_current_limit;
186+
sr_snprintf_ascii(msg, sizeof(msg),
187+
"ISET1:%05.3f", devc->set_current_limit);
188188
break;
189189
case KAXXXXP_VOLTAGE_TARGET:
190-
cmd = "VSET1:%05.2f";
191-
value = devc->set_voltage_target;
190+
sr_snprintf_ascii(msg, sizeof(msg),
191+
"VSET1:%05.2f", devc->set_voltage_target);
192192
break;
193193
case KAXXXXP_OUTPUT:
194-
cmd = "OUT%01.0f";
195-
value = (devc->set_output_enabled) ? 1 : 0;
194+
sr_snprintf_ascii(msg, sizeof(msg),
195+
"OUT%1d", (devc->set_output_enabled) ? 1 : 0);
196196
/* Set value back to recognize changes */
197197
devc->output_enabled = devc->set_output_enabled;
198198
break;
199199
case KAXXXXP_BEEP:
200-
cmd = "BEEP%01.0f";
201-
value = (devc->set_beep_enabled) ? 1 : 0;
200+
sr_snprintf_ascii(msg, sizeof(msg),
201+
"BEEP%1d", (devc->set_beep_enabled) ? 1 : 0);
202202
break;
203203
case KAXXXXP_OCP:
204-
cmd = "OCP%01.0f";
205-
value = (devc->set_ocp_enabled) ? 1 : 0;
204+
sr_snprintf_ascii(msg, sizeof(msg),
205+
"OCP%1d", (devc->set_ocp_enabled) ? 1 : 0);
206206
/* Set value back to recognize changes */
207207
devc->ocp_enabled = devc->set_ocp_enabled;
208208
break;
209209
case KAXXXXP_OVP:
210-
cmd = "OVP%01.0f";
211-
value = (devc->set_ovp_enabled) ? 1 : 0;
210+
sr_snprintf_ascii(msg, sizeof(msg),
211+
"OVP%1d", (devc->set_ovp_enabled) ? 1 : 0);
212212
/* Set value back to recognize changes */
213213
devc->ovp_enabled = devc->set_ovp_enabled;
214214
break;
215215
case KAXXXXP_SAVE:
216-
cmd = "SAV%01.0f";
217216
if (devc->program < 1 || devc->program > 5) {
218-
sr_err("Only programs 1-5 supported and %d isn't "
219-
"between them.", devc->program);
220-
g_mutex_unlock(&devc->rw_mutex);
221-
return SR_ERR;
217+
sr_err("Program %d is not in the supported 1-5 range.",
218+
devc->program);
219+
ret = SR_ERR;
220+
break;
222221
}
223-
value = devc->program;
222+
sr_snprintf_ascii(msg, sizeof(msg),
223+
"SAV%1d", devc->program);
224224
break;
225225
case KAXXXXP_RECALL:
226-
cmd = "RCL%01.0f";
227226
if (devc->program < 1 || devc->program > 5) {
228-
sr_err("Only programs 1-5 supported and %d isn't "
229-
"between them.", devc->program);
230-
g_mutex_unlock(&devc->rw_mutex);
231-
return SR_ERR;
227+
sr_err("Program %d is not in the supported 1-5 range.",
228+
devc->program);
229+
ret = SR_ERR;
230+
break;
232231
}
233-
value = devc->program;
232+
sr_snprintf_ascii(msg, sizeof(msg),
233+
"RCL%1d", devc->program);
234234
break;
235235
default:
236-
sr_err("Don't know how to set %d.", target);
237-
g_mutex_unlock(&devc->rw_mutex);
238-
return SR_ERR;
236+
sr_err("Don't know how to set target %d.", target);
237+
ret = SR_ERR;
238+
break;
239239
}
240240

241-
msg = g_malloc0(20 + 1);
242-
if (cmd)
243-
sr_snprintf_ascii(msg, 20, cmd, value);
244-
245-
ret = korad_kaxxxxp_send_cmd(serial, msg);
246-
devc->req_sent_at = g_get_monotonic_time();
247-
g_free(msg);
241+
if (ret == SR_OK && msg[0]) {
242+
ret = korad_kaxxxxp_send_cmd(serial, msg);
243+
devc->req_sent_at = g_get_monotonic_time();
244+
}
248245

249246
g_mutex_unlock(&devc->rw_mutex);
250247

0 commit comments

Comments
 (0)