Skip to content

Add support for Siglent's SDS2000X Plus series #176

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

Open
gralco wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for Siglent's SDS2000X Plus series #176

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
58 changes: 49 additions & 9 deletions src/hardware/siglent-sds/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ enum series {
SDS1000XP,
SDS1000XE,
SDS2000X,
SDS2000XP,
};

/* short name, full name */
Expand All @@ -183,19 +184,21 @@ static const struct siglent_sds_vendor supported_vendors[] = {
* number of vertical divs, live waveform samples, memory buffer samples */
static const struct siglent_sds_series supported_series[] = {
[SDS1000CML] = {VENDOR(SIGLENT), "SDS1000CML", NON_SPO_MODEL,
{ 50, 1 }, { 2, 1000 }, 18, 8, 1400363},
{ 50, 1 }, { 2, 1000 }, 18, 8, 25, 1400363},
[SDS1000CNL] = {VENDOR(SIGLENT), "SDS1000CNL", NON_SPO_MODEL,
{ 50, 1 }, { 2, 1000 }, 18, 8, 1400363},
{ 50, 1 }, { 2, 1000 }, 18, 8, 25, 1400363},
[SDS1000DL] = {VENDOR(SIGLENT), "SDS1000DL", NON_SPO_MODEL,
{ 50, 1 }, { 2, 1000 }, 18, 8, 1400363},
{ 50, 1 }, { 2, 1000 }, 18, 8, 25, 1400363},
[SDS1000X] = {VENDOR(SIGLENT), "SDS1000X", SPO_MODEL,
{ 50, 1 }, { 500, 100000 }, 14, 8, 14000363},
{ 50, 1 }, { 500, 100000 }, 14, 8, 25, 14000363},
[SDS1000XP] = {VENDOR(SIGLENT), "SDS1000X+", SPO_MODEL,
{ 50, 1 }, { 500, 100000 }, 14, 8, 14000363},
{ 50, 1 }, { 500, 100000 }, 14, 8, 25, 14000363},
[SDS1000XE] = {VENDOR(SIGLENT), "SDS1000XE", ESERIES,
{ 50, 1 }, { 500, 100000 }, 14, 8, 14000363},
{ 50, 1 }, { 500, 100000 }, 14, 8, 25, 14000363},
[SDS2000X] = {VENDOR(SIGLENT), "SDS2000X", SPO_MODEL,
{ 50, 1 }, { 500, 100000 }, 14, 8, 14000363},
{ 50, 1 }, { 500, 100000 }, 14, 8, 25, 14000363},
[SDS2000XP] = {VENDOR(SIGLENT), "SDS2000X+", SPO_MODEL,
{ 100, 1 }, { 500, 100000 }, 10, 8, 30, 14000363},
};

#define SERIES(x) &supported_series[x]
Expand Down Expand Up @@ -227,6 +230,11 @@ static const struct siglent_sds_model supported_models[] = {
{ SERIES(SDS2000X), "SDS2204X", { 2, 1000000000 }, 4, FALSE, 0 },
{ SERIES(SDS2000X), "SDS2302X", { 2, 1000000000 }, 2, FALSE, 0 },
{ SERIES(SDS2000X), "SDS2304X", { 2, 1000000000 }, 4, FALSE, 0 },
{ SERIES(SDS2000XP), "SDS2102X Plus", { 1, 1000000000 }, 2, TRUE, 16 },
{ SERIES(SDS2000XP), "SDS2104X Plus", { 1, 1000000000 }, 4, TRUE, 16 },
{ SERIES(SDS2000XP), "SDS2204X Plus", { 1, 1000000000 }, 4, TRUE, 16 },
{ SERIES(SDS2000XP), "SDS2354X Plus", { 1, 1000000000 }, 4, TRUE, 16 },
{ SERIES(SDS2000XP), "SDS2504X Plus", { 1, 1000000000 }, 4, TRUE, 16 },
};

static struct sr_dev_driver siglent_sds_driver_info;
Expand Down Expand Up @@ -429,6 +437,36 @@ static int config_get(uint32_t key, GVariant **data,
*data = g_variant_new_string("History");
break;
case SR_CONF_SAMPLERATE:
/* SDS2000X+: The channels need to be enabled/disabled prior to
* obtaining the sample rate when the data source is set to display.
* If a channel is turned on/off on the oscilloscope then use Sigrok's settings instead. */
if (!strcmp(devc->model->series->name, "SDS2000X+") && devc->data_source == DATA_SOURCE_SCREEN) {
Copy link
Contributor

@fenugrec fenugrec Feb 4, 2024

Choose a reason for hiding this comment

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

I prefer avoiding to test things like model name strings for feature support... adding a bool flag (or bit-masked enums, I would look at other more mature drivers to see if there is a more common pattern) would be preferable esp. if future models also need this

GSList *l;
for (l = sdi->channels; l; l = l->next) {
ch = l->data;
if (ch->type == SR_CHANNEL_ANALOG) {
char *channel_state;
char *cmd = g_strdup_printf("C%d:TRA?", ch->index + 1);
sr_scpi_get_string(sdi->conn, cmd, &channel_state);
if (!strcmp(channel_state, "ON") && !ch->enabled)
Copy link
Contributor

@fenugrec fenugrec Feb 4, 2024

Choose a reason for hiding this comment

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

not checking if sr_scpi_get_string() failed before strcmp (i didnt check if it sets channel_state to null or to an empty string if it fails)

if (siglent_sds_config_set(sdi, "C%d:TRA %s", ch->index + 1,
ch->enabled ? "ON" : "OFF") != SR_OK)
return SR_ERR;
if (!strcmp(channel_state, "OFF") && ch->enabled)
if (siglent_sds_config_set(sdi, "C%d:TRA %s", ch->index + 1,
ch->enabled ? "ON" : "OFF") != SR_OK)
return SR_ERR;
Copy link
Contributor

@fenugrec fenugrec Feb 4, 2024

Choose a reason for hiding this comment

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

this is getting into some pretty deep indent levels, maybe time to split some functionality into a separate func ?

if (ch->enabled != devc->analog_channels[ch->index]) {
/* Enabled channel is currently disabled, or vice versa. */
if (siglent_sds_config_set(sdi, "C%d:TRA %s", ch->index + 1,
ch->enabled ? "ON" : "OFF") != SR_OK)
return SR_ERR;
devc->analog_channels[ch->index] = ch->enabled;
devc->channels_switched = TRUE;
}
}
}
}
siglent_sds_get_dev_cfg_horizontal(sdi);
*data = g_variant_new_uint64(devc->samplerate);
break;
Expand Down Expand Up @@ -857,10 +895,12 @@ static int dev_acquisition_start(const struct sr_dev_inst *sdi)
// devc->analog_frame_size = devc->model->series->buffer_samples;
// devc->digital_frame_size = devc->model->series->buffer_samples;

siglent_sds_get_dev_cfg_horizontal(sdi);
if (strcmp(devc->model->series->name, "SDS2000X+"))
siglent_sds_get_dev_cfg_horizontal(sdi);
siglent_sds_get_dev_cfg_vertical(sdi);
switch (devc->model->series->protocol) {
case SPO_MODEL:
if (siglent_sds_config_set(sdi, "WFSU SP,0,TYPE,1") != SR_OK)
if (siglent_sds_config_set(sdi, "WFSU SP,0,NP,0,FP,0") != SR_OK)
Copy link
Contributor

@fenugrec fenugrec Feb 4, 2024

Choose a reason for hiding this comment

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

you are changing this for all models , is this intended ?

return SR_ERR;
if (devc->average_enabled) {
if (siglent_sds_config_set(sdi, "ACQW AVERAGE,%i", devc->average_samples) != SR_OK)
Expand Down
Loading