Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support for Korad KKG305P #219

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support for Korad KKG305P #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a1c0d2
Support for Korad KKG305P
May 30, 2023
d4726c1
Fix un-conditional double read
May 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/hardware/korad-kaxxxxp/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ static const struct korad_kaxxxxp_model models[] = {
{"Korad", "KA3005P", "", 1, volts_30, amps_5,
KORAD_QUIRK_ID_TRAILING},
{"Korad", "KD3005P", "", 1, volts_30, amps_5, 0},
{"Korad", "KKG305P", "", 1, volts_30, amps_5,
KORAD_QUIRK_KKG_FAMILY},
{"Korad", "KD6005P", "", 1, volts_60, amps_5, 0},
{"RND", "KA3005P", "RND 320-KA3005P", 1, volts_30, amps_5,
KORAD_QUIRK_ID_OPT_VERSION},
Expand Down Expand Up @@ -276,11 +278,21 @@ static GSList *scan(struct sr_dev_driver *di, GSList *options)
len = sizeof(reply) - 1;
sr_dbg("Want max %zu bytes.", len);

ret = korad_kaxxxxp_send_cmd(serial, "*IDN?");
ret = korad_kaxxxxp_send_cmd(serial, "*IDN?", FALSE);
if (ret < 0)
return NULL;

ret = korad_kaxxxxp_read_chars(serial, len, reply);
if (ret == 0) {
/*
* Make an attemp to send line termination
*/
sr_dbg("Make an attempt to send line termination");
ret = korad_kaxxxxp_send_cmd(serial, "\n", FALSE);
if (ret < 0)
return NULL;
ret = korad_kaxxxxp_read_chars(serial, len, reply);
}
if (ret < 0)
return NULL;
sr_dbg("Received: %d, %s", ret, reply);
Expand Down
96 changes: 74 additions & 22 deletions src/hardware/korad-kaxxxxp/protocol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,29 @@
#define DEVICE_PROCESSING_TIME_MS 80

SR_PRIV int korad_kaxxxxp_send_cmd(struct sr_serial_dev_inst *serial,
const char *cmd)
const char *cmd, const gboolean nl_termination)
{
int ret;
uint32_t cmd_len;
char *_cmd;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a fan of leading underscores for var names


cmd_len = strlen(cmd);
// '\n' + '\0'
_cmd = g_malloc0(cmd_len+2);
strcpy(_cmd, cmd);

if( nl_termination ) {
_cmd[cmd_len] = '\n';
_cmd[++cmd_len] = '\0';

sr_dbg("Sending '%s'.", cmd);
if ((ret = serial_write_blocking(serial, cmd, strlen(cmd), 0)) < 0) {
sr_err("Error sending command: %d.", ret);
return ret;
}

sr_dbg("Sending '%s'.", _cmd);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest moving this before you append the newline, so the debug message doesn't end up with sometimes a stray \n ?

if ((ret = serial_write_blocking(serial, _cmd, cmd_len, 0)) < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the assignment outside the conditional

sr_err("Error sending command: %d.", ret);
}
g_free(_cmd);

return ret;
}

Expand Down Expand Up @@ -171,10 +184,13 @@ SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
const char *cmd;
float value;
int ret;
gboolean nl_termination;

g_mutex_lock(&devc->rw_mutex);
give_device_time_to_process(devc);

nl_termination = devc->model->quirks & KORAD_QUIRK_KKG_FAMILY;

switch (target) {
case KAXXXXP_CURRENT:
case KAXXXXP_VOLTAGE:
Expand Down Expand Up @@ -242,7 +258,7 @@ SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
if (cmd)
sr_snprintf_ascii(msg, 20, cmd, value);

ret = korad_kaxxxxp_send_cmd(serial, msg);
ret = korad_kaxxxxp_send_cmd(serial, msg, nl_termination);
devc->req_sent_at = g_get_monotonic_time();
g_free(msg);

Expand All @@ -260,40 +276,42 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial,
char status_byte;
gboolean needs_ovp_quirk;
gboolean prev_status;
gboolean nl_termination;

g_mutex_lock(&devc->rw_mutex);
give_device_time_to_process(devc);
nl_termination = devc->model->quirks & KORAD_QUIRK_KKG_FAMILY;

value = NULL;
count = 5;

switch (target) {
case KAXXXXP_CURRENT:
/* Read current from device. */
ret = korad_kaxxxxp_send_cmd(serial, "IOUT1?");
ret = korad_kaxxxxp_send_cmd(serial, "IOUT1?", nl_termination);
value = &(devc->current);
break;
case KAXXXXP_CURRENT_LIMIT:
/* Read set current from device. */
ret = korad_kaxxxxp_send_cmd(serial, "ISET1?");
ret = korad_kaxxxxp_send_cmd(serial, "ISET1?", nl_termination);
value = &(devc->current_limit);
break;
case KAXXXXP_VOLTAGE:
/* Read voltage from device. */
ret = korad_kaxxxxp_send_cmd(serial, "VOUT1?");
ret = korad_kaxxxxp_send_cmd(serial, "VOUT1?", nl_termination);
value = &(devc->voltage);
break;
case KAXXXXP_VOLTAGE_TARGET:
/* Read set voltage from device. */
ret = korad_kaxxxxp_send_cmd(serial, "VSET1?");
ret = korad_kaxxxxp_send_cmd(serial, "VSET1?", nl_termination);
value = &(devc->voltage_target);
break;
case KAXXXXP_STATUS:
case KAXXXXP_OUTPUT:
case KAXXXXP_OCP:
case KAXXXXP_OVP:
/* Read status from device. */
ret = korad_kaxxxxp_send_cmd(serial, "STATUS?");
ret = korad_kaxxxxp_send_cmd(serial, "STATUS?", nl_termination);
count = 1;
break;
default:
Expand Down Expand Up @@ -323,16 +341,35 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial,
prev_status = devc->cc_mode[0];
devc->cc_mode[0] = !(status_byte & (1 << 0));
devc->cc_mode_1_changed = devc->cc_mode[0] != prev_status;

if( devc->model->quirks & KORAD_QUIRK_KKG_FAMILY) {
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing indent inside block, bad whitespace in if(

* KKG family status bits are different
* BIT1 - remote compensation status
* BIT2 - external interface (trigger, switch) status
*/
/* Remote compensation */
prev_status = devc->rmt_comp_enabled;
devc->rmt_comp_enabled = !(status_byte & (1 << 1));
devc->rmt_comp_changed = devc->rmt_comp_enabled != prev_status;

/* External interface */
prev_status = devc->ext_int_enabled;
devc->ext_int_enabled = !(status_byte & (1 << 2));
devc->ext_int_changed = devc->ext_int_enabled != prev_status;
} else {
/* Constant current channel two. */
prev_status = devc->cc_mode[1];
devc->cc_mode[1] = !(status_byte & (1 << 1));
devc->cc_mode_2_changed = devc->cc_mode[1] != prev_status;
}

/*
* Tracking:
* status_byte & ((1 << 2) | (1 << 3))
* 00 independent 01 series 11 parallel
*/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated whitespace

devc->beep_enabled = status_byte & (1 << 4);

/* OCP enabled. */
Expand All @@ -354,17 +391,32 @@ SR_PRIV int korad_kaxxxxp_get_value(struct sr_serial_dev_inst *serial,
}

sr_dbg("Status: 0x%02x", status_byte);
sr_spew("Status: CH1: constant %s CH2: constant %s. "
"Tracking would be %s and %s. Output is %s. "
"OCP is %s, OVP is %s. Device is %s.",
(status_byte & (1 << 0)) ? "voltage" : "current",
(status_byte & (1 << 1)) ? "voltage" : "current",
(status_byte & (1 << 2)) ? "parallel" : "series",
(status_byte & (1 << 3)) ? "tracking" : "independent",
(status_byte & (1 << 6)) ? "enabled" : "disabled",
(status_byte & (1 << 5)) ? "enabled" : "disabled",
(status_byte & (1 << 7)) ? "enabled" : "disabled",
(status_byte & (1 << 4)) ? "beeping" : "silent");
if( devc->model->quirks & KORAD_QUIRK_KKG_FAMILY) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another if(

sr_spew("Status: CH1: constant %s. "
"Remote compensation: %s. External interface: %s "
"Reserved bit %s. Output is %s. "
"OCP is %s, OVP is %s. Device is %s.",
(status_byte & (1 << 0)) ? "voltage" : "current",
(status_byte & (1 << 1)) ? "enabled" : "disabled",
(status_byte & (1 << 2)) ? "enabled" : "disabled",
(status_byte & (1 << 3)) ? "1" : "0",
(status_byte & (1 << 6)) ? "enabled" : "disabled",
(status_byte & (1 << 5)) ? "enabled" : "disabled",
(status_byte & (1 << 7)) ? "enabled" : "disabled",
(status_byte & (1 << 4)) ? "beeping" : "silent");
} else {
sr_spew("Status: CH1: constant %s CH2: constant %s. "
"Tracking would be %s and %s. Output is %s. "
"OCP is %s, OVP is %s. Device is %s.",
(status_byte & (1 << 0)) ? "voltage" : "current",
(status_byte & (1 << 1)) ? "voltage" : "current",
(status_byte & (1 << 2)) ? "parallel" : "series",
(status_byte & (1 << 3)) ? "tracking" : "independent",
(status_byte & (1 << 6)) ? "enabled" : "disabled",
(status_byte & (1 << 5)) ? "enabled" : "disabled",
(status_byte & (1 << 7)) ? "enabled" : "disabled",
(status_byte & (1 << 4)) ? "beeping" : "silent");
}
}

/* Read the sixth byte from ISET? BUG workaround. */
Expand Down
17 changes: 11 additions & 6 deletions src/hardware/korad-kaxxxxp/protocol.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ enum korad_quirks_flag {
KORAD_QUIRK_ID_NO_VENDOR = 1UL << 1,
KORAD_QUIRK_ID_TRAILING = 1UL << 2,
KORAD_QUIRK_ID_OPT_VERSION = 1UL << 3,
KORAD_QUIRK_ALL = (1UL << 4) - 1,
KORAD_QUIRK_KKG_FAMILY = 1UL << 4,
KORAD_QUIRK_ALL = (1UL << 5) - 1,
};

/* Information on single model */
Expand Down Expand Up @@ -79,16 +80,20 @@ struct dev_context {
float voltage_target; /**< Output voltage set. */
gboolean cc_mode[2]; /**< Device is in CC mode (otherwise CV). */

gboolean output_enabled; /**< Is the output enabled? */
gboolean beep_enabled; /**< Enable beeper. */
gboolean ocp_enabled; /**< Output current protection enabled. */
gboolean ovp_enabled; /**< Output voltage protection enabled. */
gboolean output_enabled; /**< Is the output enabled? */
gboolean beep_enabled; /**< Enable beeper. */
gboolean ocp_enabled; /**< Output current protection enabled. */
gboolean ovp_enabled; /**< Output voltage protection enabled. */
gboolean rmt_comp_enabled; /**< Is remote compensation enabled. */
gboolean ext_int_enabled; /**< Is external interface enabled. */

gboolean cc_mode_1_changed; /**< CC mode of channel 1 has changed. */
gboolean cc_mode_2_changed; /**< CC mode of channel 2 has changed. */
gboolean output_enabled_changed; /**< Output enabled state has changed. */
gboolean ocp_enabled_changed; /**< OCP enabled state has changed. */
gboolean ovp_enabled_changed; /**< OVP enabled state has changed. */
gboolean rmt_comp_changed; /**< Is remote compensation enabled. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong copypasted comment here and below, enabled-> changed

gboolean ext_int_changed; /**< Is external interface enabled. */

int acquisition_target; /**< What reply to expect. */
int program; /**< Program to store or recall. */
Expand All @@ -102,7 +107,7 @@ struct dev_context {
};

SR_PRIV int korad_kaxxxxp_send_cmd(struct sr_serial_dev_inst *serial,
const char *cmd);
const char *cmd, const gboolean nl_termination);
SR_PRIV int korad_kaxxxxp_read_chars(struct sr_serial_dev_inst *serial,
size_t count, char *buf);
SR_PRIV int korad_kaxxxxp_set_value(struct sr_serial_dev_inst *serial,
Expand Down